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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >> wrote: >>> On Fri, Dec 6, 2013 at 5:36 PM, Aaron Ballman <[email protected]> >>> wrote: >>>> On Fri, Dec 6, 2013 at 4:05 AM, Alexander Potapenko <[email protected]> >>>> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
