Richard Biener wrote:
> On Mon, Sep 2, 2019 at 10:35 PM Ulrich Weigand <uweig...@de.ibm.com> wrote:
> > Now one question might be, why does get_pointer_alignment not take
> > type alignment into account by itself?  This appears to be deliberate
> > to avoid considering numeric pointer values to be aligned when they
> > are not for target-specific reasons (e.g. the low bit that indicates
> > Thumb on ARM).
> 
> It's simply because we throw away type casting on GIMPLE so
> when you see an aligned __int128 * as argument to
> __builtin_sync* then there's no guarantee it's actually the type
> the user used here.   The user might have written
> 
> __buitlin_sync_* ((__int128 *) ptr /* cast away bogus over-alignedness */, 
> ...);
> 
> and GIMPLE happily throws away the cast.

Huh, I was simply going after the comment in front of get_object_alignment_2:
   Note that the address (and thus the alignment) computed here is based
   on the address to which a symbol resolves, whereas DECL_ALIGN is based
   on the address at which an object is actually located.  These two
   addresses are not always the same.  For example, on ARM targets,
   the address &foo of a Thumb function foo() has the lowest bit set,
   whereas foo() itself starts on an even address.

combined with the fact that get_object_alignment_2 actually itself
uses type alignment if we have an actual memory object:
      /* When EXP is an actual memory reference then we can use
         TYPE_ALIGN of a pointer indirection to derive alignment.
         Do so only if get_pointer_alignment_1 did not reveal absolute
         alignment knowledge and if using that alignment would
         improve the situation.  */

Is this not correct either, or is the situation there somehow different?

In any case, I'm not sure I understand your example.  Is this supposed
to cover the case where "ptr" is a type with 16 byte alignment, while
__int128 only has 8 byte alignment?  But in that case I'd assume that
every possible value of "ptr" at runtime must be 16 byte aligned, or
else it wouldn't be a valid value of a variable of that type, right?
So in that case assuming a 16-byte alignment would be correct?

> The proper way to carry the information from source to RTL expansion
> (or earlier GIMPLE) is to add another argument to the builtins
> specifying alignment which the FEs can derive from the argument type
> given appropriate semantics of the builtin.

I guess I can have a look to do it this way, if necessary.
Is there already some example of a builtin that does that?

> Alternatively you could make it simply undefined behavior passing
> not appropriately aligned memory to those builtins.  But I guess
> we support misaligned cases so that's not an option.

We definitely must handle the misaligned case (since the **default**
alignment of __int128 on s390x is misaligned for atomic access).

> also looks fishy.  So maybe it _is_ an option to simply require
> larger alignment?  Maybe add targetm.mode_alignment_for_atomics ()?

That's why this wouldn't work either -- a default __int128 is not
correctly aligned, we can only assume alignment if it was specifically
specified by the user via an attribute or the like.  (But since the
latter is a very common case, it is also important to handle.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

Reply via email to