RFC: Implementing new mechanism for hard register operands to inline asm as a constraint.

clang/docs/LanguageExtensions.rst

@@ -2250,6 +2250,48 @@ Query for this feature with ``__has_extension(gnu_asm_constexpr_strings)``.
       asm((std::string_view("nop")) ::: (std::string_view("memory")));
    }
 
+Hard Register Operands for ASM Constraints
+==========================================
+
+Clang supports the ability to specify specific hardware registers in inline
+assembly constraints via the use of curly braces ``{}``.
+
+Prior to clang-19, the only way to associate an inline assembly constraint
+with a specific register is via the local register variable feature (`GCC
+Specifying Registers for Local Variables <https://gcc.gnu.org/onlinedocs/gcc-6.5.0/gcc/Local-Register-Variables.html>`_).
+However, the local register variable association lasts for the entire
+scope of the variable.
+
+Hard register operands will instead only apply to the specific inline ASM
+statement which improves readability and solves a few other issues experienced
+by local register variables, such as:
+
+* the constraints for the register operands are superfluous
+* one register variable cannot be used for 2 different inline
+  assemblies if the value is expected in different hard regs
+
+The code below is an example of an inline assembly statement using local
+register variables:
+
+.. code-block:: c++
+
+  void foo() {
+    register int *p1 asm ("r0") = bar();
+    register int *p2 asm ("r1") = bar();
+    register int *result asm ("r0");
+    asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
+  }
+
+Below is the same code but using hard register operands.
+
+.. code-block:: c++
+
+  void foo() {
+    int *p1 = bar();
+    int *p2 = bar();
+    int *result;
+    asm ("sysint" : "={r0}" (result) : "0" (p1), "{r1}" (p2));
+  }
 
 Objective-C Features
 ====================

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -9906,6 +9906,8 @@ let CategoryName = "Inline Assembly Issue" in {
     "more than one input constraint matches the same output '%0'">;
   def err_store_value_to_reg : Error<
     "impossible constraint in asm: cannot store value into a register">;
+  def err_asm_hard_reg_variable_duplicate : Error<
+    "hard register operand already defined as register variable">;
 
   def warn_asm_label_on_auto_decl : Warning<
     "ignored asm label '%0' on automatic variable">;

clang/include/clang/Basic/TargetInfo.h

@@ -1115,9 +1115,17 @@ class TargetInfo : public TransferrableTargetInfo,
   ///
   /// This function is used by Sema in order to diagnose conflicts between
   /// the clobber list and the input/output lists.
+  /// The constraint should already by validated in
+  /// validateHardRegisterAsmConstraint so just do some basic checking
   virtual StringRef getConstraintRegister(StringRef Constraint,
                                           StringRef Expression) const {
-    return "";
+    StringRef Reg = Expression;
+    size_t Start = Constraint.find('{');
+    size_t End = Constraint.find('}');
+    if (Start != StringRef::npos && End != StringRef::npos && End > Start)
+      Reg = Constraint.substr(Start + 1, End - Start - 1);
+
+    return Reg;
   }
 
   struct ConstraintInfo {
@@ -1277,6 +1285,14 @@ class TargetInfo : public TransferrableTargetInfo,
   validateAsmConstraint(const char *&Name,
                         TargetInfo::ConstraintInfo &info) const = 0;
 
+  // Validate the "hard register" inline asm constraint. This constraint is
+  // of the form {<reg-name>}. This constraint is meant to be used
+  // as an alternative for the "register asm" construct to put inline
+  // asm operands into specific registers.
+  bool
+  validateHardRegisterAsmConstraint(const char *&Name,
+                                    TargetInfo::ConstraintInfo &info) const;
+
   bool resolveSymbolicName(const char *&Name,
                            ArrayRef<ConstraintInfo> OutputConstraints,
                            unsigned &Index) const;

clang/lib/Basic/TargetInfo.cpp

@@ -831,6 +831,18 @@ bool TargetInfo::validateOutputConstraint(ConstraintInfo &Info) const {
     case 'E':
     case 'F':
       break;  // Pass them.
+    case '{': {
+      // First, check the target parser in case it validates
+      // the {...} constraint differently.
+      if (validateAsmConstraint(Name, Info))
+        return true;
+
+      // If not, that's okay, we will try to validate it
+      // using a target agnostic implementation.
+      if (!validateHardRegisterAsmConstraint(Name, Info))
+        return false;
+      break;
+    }
     }
 
     Name++;
@@ -846,6 +858,36 @@ bool TargetInfo::validateOutputConstraint(ConstraintInfo &Info) const {
   return Info.allowsMemory() || Info.allowsRegister();
 }
 
+bool TargetInfo::validateHardRegisterAsmConstraint(
+    const char *&Name, TargetInfo::ConstraintInfo &Info) const {
+  // First, swallow the '{'.
+  Name++;
+
+  // Mark the start of the possible register name.
+  const char *Start = Name;
+
+  // Loop through rest of "Name".
+  // In this loop, we check whether we have a closing curly brace which
+  // validates the constraint. Also, this allows us to get the correct bounds to
+  // set our register name.
+  while (*Name && *Name != '}')
+    Name++;
+
+  // Missing '}' or if there is anything after '}', return false.
+  if (!*Name || *(Name + 1))
+    return false;
+
+  // Now we set the register name.
+  std::string Register(Start, Name - Start);
+
+  // We validate whether its a valid register to be used.
+  if (!isValidGCCRegisterName(Register))
+    return false;
+
+  Info.setAllowsRegister();
+  return true;
+}
+
 bool TargetInfo::resolveSymbolicName(const char *&Name,
                                      ArrayRef<ConstraintInfo> OutputConstraints,
                                      unsigned &Index) const {
@@ -978,6 +1020,18 @@ bool TargetInfo::validateInputConstraint(
     case '!': // Disparage severely.
     case '*': // Ignore for choosing register preferences.
       break;  // Pass them.
+    case '{': {
+      // First, check the target parser in case it validates
+      // the {...} constraint differently.
+      if (validateAsmConstraint(Name, Info))
+        return true;
+
+      // If not, that's okay, we will try to validate it
+      // using a target agnostic implementation.
+      if (!validateHardRegisterAsmConstraint(Name, Info))
+        return false;
+      break;
+    }
     }
 
     Name++;
@@ -1059,6 +1113,14 @@ void TargetInfo::copyAuxTarget(const TargetInfo *Aux) {
 std::string
 TargetInfo::simplifyConstraint(StringRef Constraint,
                                SmallVectorImpl<ConstraintInfo> *OutCons) const {
+  // If we have only the {...} constraint, do not do any simplifications. This
+  // already maps to the lower level LLVM inline assembly IR that tells the
+  // backend to allocate a specific register. Any validations would have already
+  // been done in the Sema stage or will be done in the AddVariableConstraints
+  // function.
+  if (Constraint[0] == '{' || (Constraint[0] == '&' && Constraint[1] == '{'))
+    return std::string(Constraint);
+
   std::string Result;
 
   for (const char *I = Constraint.begin(), *E = Constraint.end(); I < E; I++) {

clang/lib/Basic/Targets/AArch64.h

@@ -232,11 +232,6 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
                              std::string &SuggestedModifier) const override;
   std::string_view getClobbers() const override;
 
-  StringRef getConstraintRegister(StringRef Constraint,
-                                  StringRef Expression) const override {
-    return Expression;
-  }
-
   int getEHDataRegisterNumber(unsigned RegNo) const override;
 
   bool validatePointerAuthKey(const llvm::APSInt &value) const override;

clang/lib/Basic/Targets/ARM.h

@@ -205,11 +205,6 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
                              std::string &SuggestedModifier) const override;
   std::string_view 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;

clang/lib/Basic/Targets/RISCV.h

@@ -70,11 +70,6 @@ class RISCVTargetInfo : public TargetInfo {
 
   std::string_view getClobbers() const override { return ""; }
 
-  StringRef getConstraintRegister(StringRef Constraint,
-                                  StringRef Expression) const override {
-    return Expression;
-  }
-
   ArrayRef<const char *> getGCCRegNames() const override;
 
   int getEHDataRegisterNumber(unsigned RegNo) const override {

clang/lib/Basic/Targets/X86.h

@@ -312,7 +312,7 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
       return "di";
     // In case the constraint is 'r' we need to return Expression
     case 'r':
-      return Expression;
+      return TargetInfo::getConstraintRegister(Constraint, Expression);
     // Double letters Y<x> constraints
     case 'Y':
       if ((++I != E) && ((*I == '0') || (*I == 'z')))

clang/lib/CodeGen/CGStmt.cpp

@@ -2471,36 +2471,94 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) {
   CaseRangeBlock = SavedCRBlock;
 }
 
-/// AddVariableConstraints - Look at AsmExpr and if it is a variable declared
-/// as using a particular register add that as a constraint that will be used
-/// in this asm stmt.
-static std::string
-AddVariableConstraints(const std::string &Constraint, const Expr &AsmExpr,
-                       const TargetInfo &Target, CodeGenModule &CGM,
-                       const AsmStmt &Stmt, const bool EarlyClobber,
-                       std::string *GCCReg = nullptr) {
+/// Is it valid to apply a register constraint for a variable marked with
+/// the "register asm" construct?
+/// Optionally, if it is determined that we can, we set "Register" to the
+/// regiser name.
+static bool
+ShouldApplyRegisterVariableConstraint(const Expr &AsmExpr,
+                                      std::string *Register = nullptr) {
+
   const DeclRefExpr *AsmDeclRef = dyn_cast<DeclRefExpr>(&AsmExpr);
   if (!AsmDeclRef)
-    return Constraint;
+    return false;
   const ValueDecl &Value = *AsmDeclRef->getDecl();
   const VarDecl *Variable = dyn_cast<VarDecl>(&Value);
   if (!Variable)
-    return Constraint;
+    return false;
   if (Variable->getStorageClass() != SC_Register)
-    return Constraint;
+    return false;
   AsmLabelAttr *Attr = Variable->getAttr<AsmLabelAttr>();
   if (!Attr)
+    return false;
+
+  if (Register != nullptr)
+    // Set the register to return from Attr.
+    *Register = Attr->getLabel().str();
+  return true;
+}
+
+/// AddVariableConstraints:
+/// Look at AsmExpr and if it is a variable declared as using a particular
+/// register add that as a constraint that will be used in this asm stmt.
+/// Whether it can be used or not is dependent on querying
+/// ShouldApplyRegisterVariableConstraint() Also check whether the "hard
+/// register" inline asm constraint (i.e. "{reg-name}") is specified. If so, add
+/// that as a constraint that will be used in this asm stmt.
+static std::string
+AddVariableConstraints(const std::string &Constraint, const Expr &AsmExpr,
+                       const TargetInfo &Target, CodeGenModule &CGM,
+                       const AsmStmt &Stmt, const bool EarlyClobber,
+                       std::string *GCCReg = nullptr) {
+
+  // Do we have the "hard register" inline asm constraint.
+  bool ApplyHardRegisterConstraint =
+      Constraint[0] == '{' || (EarlyClobber && Constraint[1] == '{');
+
+  // Do we have "register asm" on a variable.
+  std::string Reg = "";
+  bool ApplyRegisterVariableConstraint =
+      ShouldApplyRegisterVariableConstraint(AsmExpr, &Reg);
+
+  // Diagnose the scenario where we apply both the register variable constraint
+  // and a hard register variable constraint as an unsupported error.
+  // Why? Because we could have a situation where the register passed in through
+  // {...} and the register passed in through the "register asm" construct could
+  // be different, and in this case, there's no way for the compiler to know
+  // which one to emit.
+  if (ApplyHardRegisterConstraint && ApplyRegisterVariableConstraint) {
+    CGM.getDiags().Report(AsmExpr.getExprLoc(),
+                          diag::err_asm_hard_reg_variable_duplicate);
     return Constraint;
-  StringRef Register = Attr->getLabel();
-  assert(Target.isValidGCCRegisterName(Register));
+  }
+
+  if (!ApplyHardRegisterConstraint && !ApplyRegisterVariableConstraint)
+    return Constraint;
+
   // We're using validateOutputConstraint here because we only care if
   // this is a register constraint.
   TargetInfo::ConstraintInfo Info(Constraint, "");
-  if (Target.validateOutputConstraint(Info) &&
-      !Info.allowsRegister()) {
+  if (Target.validateOutputConstraint(Info) && !Info.allowsRegister()) {
     CGM.ErrorUnsupported(&Stmt, "__asm__");
     return Constraint;
   }
+
+  if (ApplyHardRegisterConstraint) {
+    int Start = EarlyClobber ? 2 : 1;
+    int End = Constraint.find('}');
+    Reg = Constraint.substr(Start, End - Start);
+    // If we don't have a valid register name, simply return the constraint.
+    // For example: There are some targets like X86 that use a constraint such
+    // as "@cca", which is validated and then converted into {@cca}. Now this
+    // isn't necessarily a "GCC Register", but in terms of emission, it is
+    // valid since it lowered appropriately in the X86 backend. For the {..}
+    // constraint, we shouldn't be too strict and error out if the register
+    // itself isn't a valid "GCC register".
+    if (!Target.isValidGCCRegisterName(Reg))
+      return Constraint;
+  }
+
+  StringRef Register(Reg);
   // Canonicalize the register here before returning it.
   Register = Target.getNormalizedGCCRegisterName(Register);
   if (GCCReg != nullptr)

clang/test/CodeGen/AArch64/inline-asm.c

@@ -77,7 +77,15 @@ void test_gcc_registers(void) {
 
 void test_tied_earlyclobber(void) {
   register int a asm("x1");
-  asm("" : "+&r"(a));
+  asm(""
+      : "+&r"(a));
+  // CHECK: call i32 asm "", "=&{x1},0"(i32 %0)
+}
+
+void test_tied_earlyclobber2(void) {
+  int a;
+  asm(""
+      : "+&{x1}"(a));
   // CHECK: call i32 asm "", "=&{x1},0"(i32 %0)
 }
 
@@ -102,4 +110,4 @@ void test_sme_constraints(){
 
   asm("movt zt0[3, mul vl], z0" : : : "zt0");
 // CHECK: call void asm sideeffect "movt zt0[3, mul vl], z0", "~{zt0}"()
-}
+}

clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c

@@ -5,9 +5,15 @@
 // Test that an error is given if a physreg is defined by multiple operands.
 int test_physreg_defs(void) {
   register int l __asm__("r7") = 0;
+  int m;
 
   // CHECK: error: multiple outputs to hard register: r7
-  __asm__("" : "+r"(l), "=r"(l));
+  __asm__(""
+          : "+r"(l), "=r"(l));
 
-  return l;
+  // CHECK: error: multiple outputs to hard register: r6
+  __asm__(""
+          : "+{r6}"(m), "={r6}"(m));
+
+  return l + m;
 }