Hi Nico, Yes, it should. I’d like the followup commit (adding a fixit) to be merged too, if it gets through code review.
Cheers, James From: [email protected] [mailto:[email protected]] On Behalf Of Nico Weber Sent: 25 July 2014 15:21 To: James Molloy Cc: [email protected] Subject: Re: r213935 - Revert part of r206963 Should this be merged to 3.5? 3.4 had this warning too, right? On Fri, Jul 25, 2014 at 3:19 AM, James Molloy <[email protected]<mailto:[email protected]>> wrote: Author: jamesm Date: Fri Jul 25 05:19:47 2014 New Revision: 213935 URL: http://llvm.org/viewvc/llvm-project?rev=213935&view=rev Log: Revert part of r206963 Specifically the part where we removed a warning to be compatible with GCC, which has been widely regarded as a bad idea. I'm not quite happy with how obtuse this warning is, especially in the fairly common case of a 32-bit integer literal, so I've got another patch awaiting review that adds a fixit to reduce confusion. Modified: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/Sema/arm64-inline-asm.c cfe/trunk/test/Sema/inline-asm-validate.c Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=213935&r1=213934&r2=213935&view=diff ============================================================================== --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Fri Jul 25 05:19:47 2014 @@ -4503,6 +4503,33 @@ public: return false; } + virtual bool validateConstraintModifier(StringRef Constraint, + const char Modifier, + unsigned Size) const { + // Strip off constraint modifiers. + while (Constraint[0] == '=' || Constraint[0] == '+' || Constraint[0] == '&') + Constraint = Constraint.substr(1); + + switch (Constraint[0]) { + default: + return true; + case 'z': + case 'r': { + switch (Modifier) { + case 'x': + case 'w': + // For now assume that the person knows what they're + // doing with the modifier. + return true; + default: + // By default an 'r' constraint will be in the 'x' + // registers. + return Size == 64; + } + } + } + } + virtual const char *getClobbers() const { return ""; } int getEHDataRegisterNumber(unsigned RegNo) const { Modified: cfe/trunk/test/Sema/arm64-inline-asm.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/arm64-inline-asm.c?rev=213935&r1=213934&r2=213935&view=diff ============================================================================== --- cfe/trunk/test/Sema/arm64-inline-asm.c (original) +++ cfe/trunk/test/Sema/arm64-inline-asm.c Fri Jul 25 05:19:47 2014 @@ -1,9 +1,9 @@ // RUN: %clang_cc1 -triple arm64-apple-ios7.1 -fsyntax-only -verify %s -// expected-no-diagnostics void foo() { asm volatile("USE(%0)" :: "z"(0LL)); asm volatile("USE(%x0)" :: "z"(0LL)); asm volatile("USE(%w0)" :: "z"(0)); + asm volatile("USE(%0)" :: "z"(0)); // expected-warning {{value size does not match register size specified by the constraint and modifier}} } Modified: cfe/trunk/test/Sema/inline-asm-validate.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/inline-asm-validate.c?rev=213935&r1=213934&r2=213935&view=diff ============================================================================== --- cfe/trunk/test/Sema/inline-asm-validate.c (original) +++ cfe/trunk/test/Sema/inline-asm-validate.c Fri Jul 25 05:19:47 2014 @@ -1,9 +1,8 @@ // RUN: %clang_cc1 -triple arm64-apple-macosx10.8.0 -fsyntax-only -verify %s -// expected-no-diagnostics unsigned t, r, *p; int foo (void) { - __asm__ __volatile__( "stxr %w[_t], %[_r], [%[_p]]" : [_t] "=&r" (t) : [_p] "p" (p), [_r] "r" (r) : "memory"); + __asm__ __volatile__( "stxr %w[_t], %[_r], [%[_p]]" : [_t] "=&r" (t) : [_p] "p" (p), [_r] "r" (r) : "memory"); // expected-warning{{value size does not match register size specified by the constraint and modifier}} return 1; } _______________________________________________ cfe-commits mailing list [email protected]<mailto:[email protected]> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
