r196415 - This attribute somehow remained nameless in the attribute tablegen, until now.

[Aaron Ballman]

You actually have a bug in your code, but it's a good bug to have in the end.

__has_attribute(force_align_arg_pointer) always returns 0, so your
attribute is never being applied to the function, regardless of target
architecture.

This happens because force_align_arg_pointer is an attribute without a
name, so the preprocessor doesn't believe it exists. When I gave the
attribute a name, it meant that force_align_arg_pointer would always
return 1, regardless of target, because __has_attribute doesn't care
about targets.

After discussing it on IRC, it sounds like we do want __has_attribute
to support target-specific attributes the way you were originally
thinking it does. So ultimately, your code will do what you expect it
to do. But for right now, the code is broken if you're expecting the
attribute to be applied to that function -- it won't be.

For right now, I'm leaving my patch reverted because it means you can
build cleanly. But I think you may need to revisit your code to
determine whether the attribute needs to be applied or not; it may be
that you don't require the attribute in the first place if everything
is working without it being applied. Or you may need to update your
test suit if the code isn't working due to the attribute not being
applied.

~Aaron

On Fri, Dec 6, 2013 at 11:04 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> After a good chat on IRC, I've figured out that we were talking past
> one another. This is not a problem for x86 as it is working as
> expected there. It's an issue for non-x86 because the __has_attribute
> implementation does not care about target-specific attributes, just
> the spelling.
>
> Reverted this change in r196583 to unblock the chrome tsan builds.
> Will reapply once I've figured out what to do with __has_attribute.
>
> ~Aaron
>
> On Fri, Dec 6, 2013 at 10:02 AM, Alexander Potapenko <glider at google.com> wrote:
>> On Fri, Dec 6, 2013 at 5:36 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> On Fri, Dec 6, 2013 at 4:05 AM, Alexander Potapenko <glider at google.com> wrote:
>>>> Maybe MSVC doesn't define __GNUC__? I guess it's better to remove the
>>>> #ifdef clutter so that the attribute is applied unconditionally.
>>>
>>> Even when I did that, I was unable to reproduce. A very basic test of:
>>>
>>> __attribute__((__force_align_arg_pointer__))
>>> void f(void);
>>>
>>> Would not demonstrate the issue. When I stepped through the code, it
>>> would get to getKind, strip off the leading and trailing underscores,
>>> and determine the proper attribute id.
>> The above test also reproduces the problem for me with -m64. This
>> isn't the case with -m32, (obviously because the attribute is an
>> x86-only)
>>
>>>
>>> I don't have access to a Linux box right now. Give me a bit to set one
>>> up; any particular distro you are seeing this with? Also, x86 or x64
>>> host environment?
>> This is Ubuntu Precise, but since no headers are involved I doubt the
>> distribution version should affect the output.
>> The host environment is x86_64.
>>
>>> In the meantime, does the following code work for you:
>>>
>>> __attribute__((force_align_arg_pointer))
>>> void f(void);
>> It doesn't, with the same symptoms (only when targeting x86_64)
>>
>> If the code in QCMS is correct,
>> __has_attribute(__force_align_arg_pointer__) should return 1 on x86
>> and 0 on x86_64.
>> Looks like this is not the case for the ToT Clang on Linux:
>>
>> $ cat p.cc
>> #include <stdio.h>
>> int main() {
>>   printf("%d\n", __has_attribute(__force_align_arg_pointer__));
>>   printf("%d\n", __has_attribute(force_align_arg_pointer));
>>   return 0;
>> }
>>
>> $ bin/clang++ p.cc -o p -m32 && ./p
>> 1
>> 1
>> $ bin/clang++ p.cc -o p -m64 && ./p
>> 1
>> 1