-
Notifications
You must be signed in to change notification settings - Fork 11.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Implementing new mechanism for hard register operands to inline asm as a constraint. #85846
base: main
Are you sure you want to change the base?
Conversation
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
@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]
|
@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. |
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? |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -770,6 +770,18 @@ bool TargetInfo::validateOutputConstraint(ConstraintInfo &Info) const { | |||
case 'E': | |||
case 'F': | |||
break; // Pass them. | |||
case '{': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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;
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
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: