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

Aaron Ballman aaron at aaronballman.com
Thu Jan 9 15:03:39 PST 2014


This attribute is now named in Attr.td again, and I have fixed
__has_attribute to be intelligent about target architectures. So this
should also fix the bug in your code.

HTH!

~Aaron

On Fri, Dec 6, 2013 at 11:52 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> 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



More information about the cfe-commits mailing list