r213935 - Revert part of r206963

[James Molloy]

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: thakis at google.com [mailto:thakis at google.com] On Behalf Of Nico Weber
Sent: 25 July 2014 15:21
To: James Molloy
Cc: cfe-commits at cs.uiuc.edu
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 <james.molloy at arm.com<mailto:james.molloy at arm.com>> 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
cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140725/5916c27e/attachment.html>