RFC: Implementing new mechanism for hard register operands to inline asm as a constraint. #85846
+664
−58
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-backend-systemz @llvm/pr-subscribers-backend-aarch64 Author: None (tltao) ChangesRecreate the hard register PR created on phabricator with some minor additions: https://reviews.llvm.org/D105142 The main idea is to allow Clang to support the ability to specify specific hardware registers in inline assembly constraints via the use of curly braces Relevant RFCs posted here: https://lists.llvm.org/pipermail/llvm-dev/2021-June/151370.html Copying the Summary from the phabricator patch:
The following design decisions were taken for this patch: For the Sema side:
For the Clang CodeGen side:
Tests:
Patch is 68.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85846.diff 19 Files Affected:
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 13d7261d83d7f1..39d1981a068992 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1760,6 +1760,49 @@ references can be used instead of numeric references.
return -1;
}
+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:
+
+* function calls might clobber register variables
+* 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
====================
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e97902564af08..09672eb3865742 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9235,6 +9235,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: can't 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">;
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 374595edd2ce4a..01d43b838414b7 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1043,9 +1043,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 {
@@ -1187,6 +1195,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;
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 5d9055174c089a..2190a8b8bb246d 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -770,6 +770,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++;
@@ -785,6 +797,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 {
@@ -917,6 +959,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++;
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 2dd6b2181e87df..c40ef2a3c13e94 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -188,11 +188,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;
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index 71322a094f5edb..9ed452163c6048 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -213,11 +213,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;
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index bfbdafb682c851..89071da7a42776 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/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 {
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index d2232c7d5275ab..e7fb2706d02590 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -307,7 +307,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')))
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 8898e3f22a7df6..97f61eea620b2a 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2183,9 +2183,17 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) {
CaseRangeBlock = SavedCRBlock;
}
-static std::string
-SimplifyConstraint(const char *Constraint, const TargetInfo &Target,
- SmallVectorImpl<TargetInfo::ConstraintInfo> *OutCons=nullptr) {
+static std::string SimplifyConstraint(
+ const char *Constraint, const TargetInfo &Target,
+ SmallVectorImpl<TargetInfo::ConstraintInfo> *OutCons = nullptr) {
+ // 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;
while (*Constraint) {
@@ -2232,37 +2240,94 @@ SimplifyConstraint(const char *Constraint, const TargetInfo &Target,
return Result;
}
+/// 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) {
-/// 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) {
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)
diff --git a/clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c b/clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c
index 754d7e66f04b24..60237c81fd7298 100644
--- a/clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c
+++ b/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;
}
diff --git a/clang/test/CodeGen/SystemZ/systemz-inline-asm.c b/clang/test/CodeGen/SystemZ/systemz-inline-asm.c
index e38d37cd345e26..a3b47700dc30bb 100644
--- a/clang/test/CodeGen/SystemZ/systemz-inline-asm.c
+++ b/clang/test/CodeGen/SystemZ/systemz-inline-asm.c
@@ -134,12 +134,25 @@ long double test_f128(long double f, long double g) {
int test_physregs(void) {
// CHECK-LABEL: define{{.*}} signext i32 @test_physregs()
register int l __asm__("r7") = 0;
+ int m = 0;
// CHECK: call i32 asm "lr $0, $1", "={r7},{r7}"
- __asm__("lr %0, %1" : "+r"(l));
+ __asm__("lr %0, %1"
+ : "+r"(l));
// CHECK: call i32 asm "$0 $1 $2", "={r7},{r7},{r7}"
- __asm__("%0 %1 %2" : "+r"(l) : "r"(l));
+ __asm__("%0 %1 %2"
+ : "+r"(l)
+ : "r"(l));
- return l;
+ // CHECK: call i32 asm "lr $0, $1", "={r6},{r6}"
+ __asm__("lr %0, %1"
+ : "+{r6}"(m));
+
+ // CHECK: call i32 asm "$0 $1 $2", "={r6},{r6},{r6}"
+ __asm__("%0 %1 %2"
+ : "+{r6}"(m)
+ : "{r6}"(m));
+
+ return l + m;
}
diff --git a/clang/test/CodeGen/aarch64-inline-asm.c b/clang/test/CodeGen/aarch64-inline-asm.c
index 8ddee560b11da4..860cc858275ea6 100644
--- a/clang/test/CodeGen/aarch64-inline-asm.c
+++ b/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}"()
-}
\ No newline at end of file
+}
diff --git a/clang/test/CodeGen/asm-goto.c b/clang/test/CodeGen/asm-goto.c
index 4037c1b2a3d7a2..77bd77615f2998 100644
--- a/clang/test/CodeGen/asm-goto.c
+++ b/clang/test/CodeGen/asm-goto.c
@@ -55,14 +55,14 @@ int test3(int out1, int out2) {
int test4(int out1, int out2) {
// CHECK-LABEL: define{{.*}} i32 @test4(
- // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", "={si},={di},r,0,1,!i,!i
+ // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", "={si},={di},r,{si},{di},!i,!i
// CHECK: to label %asm.fallthrough [label %label_true.split, label %loop.split]
// CHECK-LABEL: asm.fallthrough:
if (out1 < out2)
asm volatile goto("jne %l5" : "+S"(out1), "+D"(out2) : "r"(out1) :: label_true, loop);
else
asm volatile goto("jne %l7" : "+S"(out1), "+D"(out2) : "r"(out1), "r"(out2) :: label_true, loop);
- // CHECK: callbr { i32, i32 } asm sideeffect "jne ${7:l}", "={si},={di},r,r,0,1,!i,!i
+ // CHECK: callbr { i32, i32 } asm sideeffect "jne ${7:l}", "={si},={di},r,r,{si},{di},!i,!i
// CHECK: to label %asm.fallthrough6 [label %label_true.split11, label %loop.split14]
// CHECK-LABEL: asm.fallthrough6:
return out1 + out2;
@@ -92,7 +92,7 @@ int test5(int addr, int size, int limit) {
int test6(int out1) {
// CHECK-LABEL: define{{.*}} i32 @test6(
- // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne ${3:l}", "={si},r,0,!i,!i,{{.*}}
+ // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne ${3:l}", "={si},r,{si},!i,!i,{{.*}}
// CHECK: to label %asm.fallthrough [label %label_true.split, label %landing.split]
// CHECK-LABEL: asm.fallthrough:
// CHECK-LABEL: landing:
diff --git a/clang/test/CodeGen/ms-intrinsics.c b/clang/test/CodeGen/ms-intrinsics.c
index 5bb003d1f91fc0..375258ca609675 100644
--- a/clang/test/CodeGen/ms-intrinsics.c
+++ b/clang/test/CodeGen/ms-intrinsics.c
@@ -36,12 +36,12 @@ void test__movsb(unsigned char *Dest, unsigned char *Src, size_t Count) {
return __movsb(Dest, Src, Count);
}
// CHECK-I386-LABEL: define{{.*}} void @test__movsb
-// CHECK-I386: tail call { ptr, ptr, i32 } asm sideeffect "xchg $(%esi, $1$|$1, esi$)\0Arep movsb\0Axchg ...
[truncated]
|
Member
|
@llvm/pr-subscribers-backend-arm Author: None (tltao) ChangesRecreate the hard register PR created on phabricator with some minor additions: https://reviews.llvm.org/D105142 The main idea is to allow Clang to support the ability to specify specific hardware registers in inline assembly constraints via the use of curly braces Relevant RFCs posted here: https://lists.llvm.org/pipermail/llvm-dev/2021-June/151370.html Copying the Summary from the phabricator patch:
The following design decisions were taken for this patch: For the Sema side:
For the Clang CodeGen side:
Tests:
Patch is 68.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85846.diff 19 Files Affected:
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 13d7261d83d7f1..39d1981a068992 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1760,6 +1760,49 @@ references can be used instead of numeric references.
return -1;
}
+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:
+
+* function calls might clobber register variables
+* 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
====================
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e97902564af08..09672eb3865742 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9235,6 +9235,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: can't 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">;
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 374595edd2ce4a..01d43b838414b7 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1043,9 +1043,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 {
@@ -1187,6 +1195,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;
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 5d9055174c089a..2190a8b8bb246d 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -770,6 +770,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++;
@@ -785,6 +797,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 {
@@ -917,6 +959,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++;
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 2dd6b2181e87df..c40ef2a3c13e94 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -188,11 +188,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;
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index 71322a094f5edb..9ed452163c6048 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -213,11 +213,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;
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index bfbdafb682c851..89071da7a42776 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/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 {
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index d2232c7d5275ab..e7fb2706d02590 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -307,7 +307,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')))
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 8898e3f22a7df6..97f61eea620b2a 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2183,9 +2183,17 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) {
CaseRangeBlock = SavedCRBlock;
}
-static std::string
-SimplifyConstraint(const char *Constraint, const TargetInfo &Target,
- SmallVectorImpl<TargetInfo::ConstraintInfo> *OutCons=nullptr) {
+static std::string SimplifyConstraint(
+ const char *Constraint, const TargetInfo &Target,
+ SmallVectorImpl<TargetInfo::ConstraintInfo> *OutCons = nullptr) {
+ // 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;
while (*Constraint) {
@@ -2232,37 +2240,94 @@ SimplifyConstraint(const char *Constraint, const TargetInfo &Target,
return Result;
}
+/// 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) {
-/// 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) {
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)
diff --git a/clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c b/clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c
index 754d7e66f04b24..60237c81fd7298 100644
--- a/clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c
+++ b/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;
}
diff --git a/clang/test/CodeGen/SystemZ/systemz-inline-asm.c b/clang/test/CodeGen/SystemZ/systemz-inline-asm.c
index e38d37cd345e26..a3b47700dc30bb 100644
--- a/clang/test/CodeGen/SystemZ/systemz-inline-asm.c
+++ b/clang/test/CodeGen/SystemZ/systemz-inline-asm.c
@@ -134,12 +134,25 @@ long double test_f128(long double f, long double g) {
int test_physregs(void) {
// CHECK-LABEL: define{{.*}} signext i32 @test_physregs()
register int l __asm__("r7") = 0;
+ int m = 0;
// CHECK: call i32 asm "lr $0, $1", "={r7},{r7}"
- __asm__("lr %0, %1" : "+r"(l));
+ __asm__("lr %0, %1"
+ : "+r"(l));
// CHECK: call i32 asm "$0 $1 $2", "={r7},{r7},{r7}"
- __asm__("%0 %1 %2" : "+r"(l) : "r"(l));
+ __asm__("%0 %1 %2"
+ : "+r"(l)
+ : "r"(l));
- return l;
+ // CHECK: call i32 asm "lr $0, $1", "={r6},{r6}"
+ __asm__("lr %0, %1"
+ : "+{r6}"(m));
+
+ // CHECK: call i32 asm "$0 $1 $2", "={r6},{r6},{r6}"
+ __asm__("%0 %1 %2"
+ : "+{r6}"(m)
+ : "{r6}"(m));
+
+ return l + m;
}
diff --git a/clang/test/CodeGen/aarch64-inline-asm.c b/clang/test/CodeGen/aarch64-inline-asm.c
index 8ddee560b11da4..860cc858275ea6 100644
--- a/clang/test/CodeGen/aarch64-inline-asm.c
+++ b/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}"()
-}
\ No newline at end of file
+}
diff --git a/clang/test/CodeGen/asm-goto.c b/clang/test/CodeGen/asm-goto.c
index 4037c1b2a3d7a2..77bd77615f2998 100644
--- a/clang/test/CodeGen/asm-goto.c
+++ b/clang/test/CodeGen/asm-goto.c
@@ -55,14 +55,14 @@ int test3(int out1, int out2) {
int test4(int out1, int out2) {
// CHECK-LABEL: define{{.*}} i32 @test4(
- // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", "={si},={di},r,0,1,!i,!i
+ // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", "={si},={di},r,{si},{di},!i,!i
// CHECK: to label %asm.fallthrough [label %label_true.split, label %loop.split]
// CHECK-LABEL: asm.fallthrough:
if (out1 < out2)
asm volatile goto("jne %l5" : "+S"(out1), "+D"(out2) : "r"(out1) :: label_true, loop);
else
asm volatile goto("jne %l7" : "+S"(out1), "+D"(out2) : "r"(out1), "r"(out2) :: label_true, loop);
- // CHECK: callbr { i32, i32 } asm sideeffect "jne ${7:l}", "={si},={di},r,r,0,1,!i,!i
+ // CHECK: callbr { i32, i32 } asm sideeffect "jne ${7:l}", "={si},={di},r,r,{si},{di},!i,!i
// CHECK: to label %asm.fallthrough6 [label %label_true.split11, label %loop.split14]
// CHECK-LABEL: asm.fallthrough6:
return out1 + out2;
@@ -92,7 +92,7 @@ int test5(int addr, int size, int limit) {
int test6(int out1) {
// CHECK-LABEL: define{{.*}} i32 @test6(
- // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne ${3:l}", "={si},r,0,!i,!i,{{.*}}
+ // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne ${3:l}", "={si},r,{si},!i,!i,{{.*}}
// CHECK: to label %asm.fallthrough [label %label_true.split, label %landing.split]
// CHECK-LABEL: asm.fallthrough:
// CHECK-LABEL: landing:
diff --git a/clang/test/CodeGen/ms-intrinsics.c b/clang/test/CodeGen/ms-intrinsics.c
index 5bb003d1f91fc0..375258ca609675 100644
--- a/clang/test/CodeGen/ms-intrinsics.c
+++ b/clang/test/CodeGen/ms-intrinsics.c
@@ -36,12 +36,12 @@ void test__movsb(unsigned char *Dest, unsigned char *Src, size_t Count) {
return __movsb(Dest, Src, Count);
}
// CHECK-I386-LABEL: define{{.*}} void @test__movsb
-// CHECK-I386: tail call { ptr, ptr, i32 } asm sideeffect "xchg $(%esi, $1$|$1, esi$)\0Arep movsb\0Axchg ...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
Author
Member
|
Thanks for reviving the change! I'm definitely in favor of this, but since it adds no new functionality (only better usability), the value proposition is significantly lessened (IMO, to the point where it may not be be worthwhile to do) if it gets implemented ONLY in clang. So I'd love to see some movement on the GCC side, too. Maybe revive the existing discussion thread? |
jyknight
reviewed
Mar 28, 2024
clang/docs/LanguageExtensions.rst
Outdated
| statement which improves readability and solves a few other issues experienced | ||
| by local register variables, such as: | ||
|
|
||
| * function calls might clobber register variables |
Member
There was a problem hiding this comment.
This isn't actually an issue (at least in LLVM), since local register variables are not actually assigned to the named register except when passed/returned from inline-asm statements.
| case 'E': | ||
| case 'F': | ||
| break; // Pass them. | ||
| case '{': { |
Member
There was a problem hiding this comment.
It's unclear to me whether this uses the same register parsing logic as the existing named-register asm on variables. It looks like it's going to do something different, which worries me.
I think we ought to be accepting the exact same names in both syntaxes, on all platforms. Can you confirm if that's actually the case with this PR?
Contributor
Author
There was a problem hiding this comment.
Yes, we should be using the same names. The register asm variable is parsed in clang/lib/Sema/SemaDecl.cpp using:
case SC_Register:
// Local Named register
if (!Context.getTargetInfo().isValidGCCRegisterName(Label) &&
DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl()))
Diag(E->getExprLoc(), diag::err_asm_unknown_register_name) << Label;
break;
added 6 commits
December 3, 2025 15:05
https://reviews.llvm.org/D105142 The main idea is to allow Clang to support the ability to specify specific hardware registers in inline assembly constraints via the use of curly braces ``{}``. As such, this is mainly a Clang change. Relevant RFCs posted here https://lists.llvm.org/pipermail/llvm-dev/2021-June/151370.html https://gcc.gnu.org/pipermail/gcc/2021-June/236269.html
tltao
requested review from
JonPsson1,
el-ev,
erichkeane,
jyknight,
nikic,
redstar and
uweigand
Contributor
Author
|
Ping. Restarting this topic as GCC have implemented the same feature. https://gcc.gnu.org/onlinedocs/gcc/Hard-Register-Constraints.html |
Collaborator
efriedma-quic
left a comment
efriedma-quic
left a comment
There was a problem hiding this comment.
Please add tests for multiple constraints, like void f() { asm(""::"i{x0}"(0)); }.
Contributor
Author
I think this raises an interesting question of how we want to resolve this pattern. If a variable named in this constraint is also set via register asm. E.g. Clang currently overwrites IMO, this goes against the intended use of hard registers, so I think we should disallow this in the front end, and error out if it detects |
Collaborator
If we can get away with rejecting it, sure, fine. gcc accepts it, but not sure if anyone is actually using it that way. If you do want to reject, please make sure we reject gracefully at both the clang level and the LLVM IR level. |
Contributor
Author
Is there a reason we also need to check it in the LLVM IR level if we are already checking for it in clang? This PR only affects the Clang output so I don't know for sure if there's another path to generate the LLVM IR pattern. |
Collaborator
We want to be friendly to people writing non-clang frontends. |
Member
Not sure why we have to fully reject this. In GCC, using a register asm in an operand with hard register constraints is allowed if the register asm is compatible with the constraint. When there are multiple constraints, I think it could work the same way - if the register asm is compatible with at least one of the constraints, it is OK (and we effectively force the register from the register asm), otherwise we should error out. @stefan-sf-ibm , the GCC docs do not specifically go into this case. Can you confirm how GCC handles register asm in combination with multiple constraints including hard register constraints? |
Contributor
|
Currently I've implemented/documented constraints consisting of single hard register constraints only since this is the most basic feature and I'm still running into RA problems in certain cases for those. Once those are fixed I would like to move on and have a look at how to implement multiple constraints involving hard register constraints. I haven't fully wrapped my head around those. There are certainly cases like constraint This brings me to register asm and hard register constraints. If the constraint is a single hard register constraint and the operand a register asm object and both coincide in the register, then I don't see a reason to reject this. That being said, currently I accept this in GCC and as you said, if multiple constraints are involved and at least one is compatible with the register asm, then this could work, too. Though, this is where it becomes a bit more complicated since GCC and Clang do not always coincide here. Assume we have that GCC assigns to the operand some register which is compatible with constraint |
Contributor
Author
Is there an example of a use case that looks something like To me, the constraint With hard register however, we are able to fine tune inline asm much more precisely, so to me, allowing something like |
Collaborator
|
An example that might be helpful (using x86 asm): Here, you want one of two choices: either an immediate, or the register "cl". I can't think of any cases where you'd want to specify both a specific register and a generic register class, though. |
Member
|
I can image cases where multiple register constraints may still be useful; e.g. to express that we can use any one of several hard-coded registers, or we can use a hard-coded register or some register class that doesn't contain that hard register. It is of course superfluous to specific both a hard register and a register class containing this register. But that's really the same issue as specifying two classes where one is a superclass of the other - and we don't explicitly forbid this either. |
Contributor
Author
|
Thanks for all the comments. I think from @stefan-sf-ibm's response, it sounds like the GCC side is not fully decided on what to do with multiple constraints involving hard registers, so we can wait for the details to be hashed out further there. In the meantime, I plan on making a few changes:
Does this sound reasonable? |
Collaborator
I'm not sure I follow what you mean here; does this mean that if you have a register asm variable, it might not end up in that register? That seems like a regression for existing code. Other bits seem fine. |
Contributor
Author
Ah, good point. I was thinking more in line of mismatching register classes, but you're right, with our current TargetLowering logic, it's not possible to combine register asm with any other constraints. |
Member
|
A register-asm variable declaration passed to an asm statement should definitely override the constraints listed on the asm statement itself, forcing the use of the specified register. Doing otherwise is extremely surprising behavior! That said, it'd (probably) be OK to report a compiler error if the constraints listed on the asm statement would not otherwise permit the listed register. (But such a behavior change is pretty much unrelated to this PR) Agreed that we should support multiple hard register constraints, or mixed hard and other register constraints specified on the asm statement. And, yes, we should support that at the same level as we support multiple constraints today (which is to say "we adhere to the specified constraints, but don't necessarily make GOOD choices"). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recreate the hard register PR created on phabricator with some minor additions: https://reviews.llvm.org/D105142
The main idea is to allow Clang to support the ability to specify specific hardware registers in inline assembly constraints via the use of curly braces
{}. As such, this is mainly a Clang change.Relevant RFCs posted here:
https://lists.llvm.org/pipermail/llvm-dev/2021-June/151370.html
https://gcc.gnu.org/pipermail/gcc/2021-June/236269.html (implemented - see https://gcc.gnu.org/onlinedocs/gcc/Hard-Register-Constraints.html)
Copying the Summary from the phabricator patch:
The following design decisions were taken for this patch:
For the Sema side:
For the Clang CodeGen side:
Tests:
Differences with GCC:
For the most part, the Clang implementation follows the GCC implementation. However, one difference between the Clang implementation and the GCC implementation is in the following example:
In the GCC implementation, both statements above emits the error
matching constraint not valid in output operand. In LLVM, statement(1)emits the errorcouldn't allocate output register for constraint 'x'fromInlineAsmLowering/selectionDAGwhereas statement(2)emits no errors.