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