[Targets] Implement getConstraintRegister for ARM and AArch64

[miyuki]

Summary

The getConstraintRegister method is used by semantic checking of
inline assembly statements in order to diagnose conflicts between
clobber list and input/output lists. Currently ARM and AArch64 don't
override getConstraintRegister, so conflicts between registers
assigned to variables in asm labels and clobber lists are not
diagnosed. Such conflicts can cause assertion failures in the back end
and even miscompilations.

This patch implements getConstraintRegister for ARM and AArch64
targets. Since these targets don't have single-register constraints,
the implementation is trivial and just returns the register specified
in an asm label (if any).

[thopre]

lib/Basic/Targets/AArch64.h

85–89

From what I understood of the original patch, getConstraintRegister is a sort of a more comprehensive version of convertRegister. On ARM convertRegister handles U and p constraint specially, should this do the same?

[miyuki]

lib/Basic/Targets/AArch64.h

85–89

Did you mean convertConstraint? As I understand, this function does canonicalization of constraints. If a constraint happens to be single-register, it is converted into {register} form (but this does not happen on ARM). On the other hand, getConstraintRegister takes a constraint and an asm label and converts them into a register (and in our case the register always comes from the asm label, never from the constraint), so I don't think we need to handle U and p specially.

[miyuki]

Added a comment for getConstraintRegister

[thopre]

lib/Basic/Targets/AArch64.h

85–89

Forgot to send this:

Fair enough. Since you seems to have the expected behavior of this function well understood, would you mind adding a comment here and in clang/Basic/TargetInfo.h?

Luckily we had a similar chat offline. Thanks for the change

[thopre]

FWIW LGTM.

[thopre]

As I said, LGTM. Please wait a few days (say next Monday) in case anyone else has any objection.

include/clang/Basic/TargetInfo.h

public:
   /// \brief Returns the "normalized" GCC register name.
   ///
   /// ReturnCannonical true will return the register name without any additions
   /// such as "{}" or "%" in it's canonical form, for example:
   /// ReturnCanonical = true and Name = "rax", will return "ax".
   StringRef getNormalizedGCCRegisterName(StringRef Name,
                                          bool ReturnCanonical = false) const;
 
+  /// \brief Extracts a register from the passed constraint (if it is a
+  /// single-register constraint) and the asm label expression related to a
+  /// variable in the input or output list of an inline asm statement.
+  ///
+  /// This function is used by Sema in order to diagnose conflicts between
+  /// the clobber list and the input/output lists.
   virtual StringRef getConstraintRegister(StringRef Constraint,
                                           StringRef Expression) const {
     return "";
   }
 
   struct ConstraintInfo {
     enum {
       CI_None = 0x00,

lib/Basic/Targets/AArch64.h

public:
   ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override;
   bool validateAsmConstraint(const char *&Name,
                              TargetInfo::ConstraintInfo &Info) const override;
   bool
   validateConstraintModifier(StringRef Constraint, char Modifier, unsigned Size,
                              std::string &SuggestedModifier) const override;
   const char *getClobbers() const override;
 
+  StringRef getConstraintRegister(StringRef Constraint,
+                                  StringRef Expression) const override {
+    return Expression;
+  }
+
   int getEHDataRegisterNumber(unsigned RegNo) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY AArch64leTargetInfo : public AArch64TargetInfo {
 public:
   AArch64leTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts);
 
   void getTargetDefines(const LangOptions &Opts,

lib/Basic/Targets/ARM.h

public:
   bool validateAsmConstraint(const char *&Name,
                              TargetInfo::ConstraintInfo &Info) const override;
   std::string convertConstraint(const char *&Constraint) const override;
   bool
   validateConstraintModifier(StringRef Constraint, char Modifier, unsigned Size,
                              std::string &SuggestedModifier) const override;
   const char *getClobbers() const override;
 
+  StringRef getConstraintRegister(StringRef Constraint,
+                                  StringRef Expression) const override {
+    return Expression;
+  }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override;
 
   int getEHDataRegisterNumber(unsigned RegNo) const override;
 
   bool hasSjLjLowering() const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY ARMleTargetInfo : public ARMTargetInfo {

test/Sema/arm-asm.c

 // RUN: %clang_cc1 %s -triple armv7-apple-darwin -verify -fsyntax-only
 
 void f (void) {
   int Val;
   asm volatile ("lw (r1), %0[val]": "=&b"(Val)); // expected-error {{invalid output constraint '=&b' in asm}}
   return;
 }
 
 void test_64bit_r(void) {
   long long foo = 0, bar = 0;
   asm volatile("INST %0, %1" : "=r"(foo) : "r"(bar));
 }
+
+void test_clobber_conflict(void) {
+  register int x asm("r1");
+  asm volatile("nop" :: "r"(x) : "%r1"); // expected-error {{conflicts with asm clobber list}}
+  asm volatile("nop" :: "l"(x) : "%r1"); // expected-error {{conflicts with asm clobber list}}
+  asm volatile("nop" : "=r"(x) :: "%r1"); // expected-error {{conflicts with asm clobber list}}
+}

test/Sema/arm64-inline-asm.c

 // RUN: %clang_cc1 -triple arm64-apple-ios7.1 -fsyntax-only -verify %s
 
 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}} expected-note {{use constraint modifier "w"}}
 }
+
+void test_clobber_conflict(void) {
+  register long x asm("x1");
+  asm volatile("nop" :: "r"(x) : "%x1"); // expected-error {{conflicts with asm clobber list}}
+  asm volatile("nop" : "=r"(x) :: "%x1"); // expected-error {{conflicts with asm clobber list}}
+}