Skip to content

[X86] Support EGPR for inline assembly. #92338

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

Merged
merged 15 commits into from
May 30, 2024
Merged

Conversation

FreddyLeaf
Copy link
Contributor

@FreddyLeaf FreddyLeaf commented May 16, 2024

"jR": explicitly enables EGPR
"r", "l", "q": enables/disables EGPR w/wo -mapx-inline-asm-use-gpr32
"jr": explicitly enables GPR with -mapx-inline-asm-use-gpr32
-mapx-inline-asm-use-gpr32 will also define a new macro:
__APX_INLINE_ASM_USE_GPR32__

GCC patches:
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631183.html
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631186.html
[PATCH v2] x86: Define APX_INLINE_ASM_USE_GPR32 (gnu.org)

Reference: https://gcc.godbolt.org/z/nPPvbY6r4

"jR": explictly enables EGPR
"r": enables/disables EGPR w/wo -mapx-inline-asm-use-gpr32
-mapx-inline-asm-use-gpr32 will also define a new Macro:
__APX_INLINE_ASM_USE_GPR32__
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 16, 2024
@FreddyLeaf FreddyLeaf requested a review from RKSimon May 16, 2024 03:13
@llvmbot
Copy link
Member

llvmbot commented May 16, 2024

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Freddy Ye (FreddyLeaf)

Changes

"jR": explictly enables EGPR
"r": enables/disables EGPR w/wo -mapx-inline-asm-use-gpr32
-mapx-inline-asm-use-gpr32 will also define a new Macro:
APX_INLINE_ASM_USE_GPR32


Full diff: https://github.com/llvm/llvm-project/pull/92338.diff

10 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+2)
  • (modified) clang/lib/Basic/Targets/X86.cpp (+26)
  • (modified) clang/lib/Basic/Targets/X86.h (+1)
  • (modified) clang/lib/Driver/ToolChains/Arch/X86.cpp (+2)
  • (added) clang/test/Driver/x86-apx-inline-asm-use-gpr32.cpp (+3)
  • (modified) clang/test/Preprocessor/x86_target_features.c (+3)
  • (modified) llvm/lib/Target/X86/X86.td (+3)
  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+52-5)
  • (added) llvm/test/CodeGen/X86/inline-asm-jR-constraint.ll (+19)
  • (added) llvm/test/CodeGen/X86/inline-asm-r-constraint.ll (+16)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 73a2518480e9b..20a7c482bbf06 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6281,6 +6281,8 @@ def mno_apx_features_EQ : CommaJoined<["-"], "mno-apx-features=">, Group<m_x86_F
 // we will add it to -mapxf.
 def mapxf : Flag<["-"], "mapxf">, Alias<mapx_features_EQ>, AliasArgs<["egpr","push2pop2","ppx", "ndd"]>;
 def mno_apxf : Flag<["-"], "mno-apxf">, Alias<mno_apx_features_EQ>, AliasArgs<["egpr","push2pop2","ppx","ndd"]>;
+def mapx_inline_asm_use_gpr32 : Flag<["-"], "mapx-inline-asm-use-gpr32">, Group<m_Group>,
+                                HelpText<"Enable use of GPR32 in inline assembly for APX">;
 } // let Flags = [TargetSpecific]
 
 // VE feature flags
diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 67e2126cf766b..9e61b6e6d6441 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -450,6 +450,8 @@ bool X86TargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
       HasFullBFloat16 = true;
     } else if (Feature == "+egpr") {
       HasEGPR = true;
+    } else if (Feature == "+inline-asm-use-gpr32") {
+      HasInlineAsmUseGPR32 = true;
     } else if (Feature == "+push2pop2") {
       HasPush2Pop2 = true;
     } else if (Feature == "+ppx") {
@@ -974,6 +976,8 @@ void X86TargetInfo::getTargetDefines(const LangOptions &Opts,
   // Condition here is aligned with the feature set of mapxf in Options.td
   if (HasEGPR && HasPush2Pop2 && HasPPX && HasNDD)
     Builder.defineMacro("__APX_F__");
+  if (HasInlineAsmUseGPR32)
+    Builder.defineMacro("__APX_INLINE_ASM_USE_GPR32__");
 
   // Each case falls through to the previous one here.
   switch (SSELevel) {
@@ -1493,6 +1497,15 @@ bool X86TargetInfo::validateAsmConstraint(
   case 'C': // SSE floating point constant.
   case 'G': // x87 floating point constant.
     return true;
+  case 'j':
+    Name++;
+    switch (*Name) {
+    default:
+      return false;
+    case 'R':
+      Info.setAllowsRegister();
+      return true;
+    }
   case '@':
     // CC condition changes.
     if (auto Len = matchAsmCCConstraint(Name)) {
@@ -1764,6 +1777,19 @@ std::string X86TargetInfo::convertConstraint(const char *&Constraint) const {
       // to the next constraint.
       return std::string("^") + std::string(Constraint++, 2);
     }
+  case 'j':
+    switch (Constraint[1]) {
+    default:
+      // Break from inner switch and fall through (copy single char),
+      // continue parsing after copying the current constraint into
+      // the return string.
+      break;
+    case 'R':
+      // "^" hints llvm that this is a 2 letter constraint.
+      // "Constraint++" is used to promote the string iterator
+      // to the next constraint.
+      return std::string("^") + std::string(Constraint++, 2);
+    }
     [[fallthrough]];
   default:
     return std::string(1, *Constraint);
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index c14e4d5f433d8..69c68ee80f3ba 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -174,6 +174,7 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
   bool HasNDD = false;
   bool HasCCMP = false;
   bool HasCF = false;
+  bool HasInlineAsmUseGPR32 = false;
 
 protected:
   llvm::X86::CPUKind CPU = llvm::X86::CK_None;
diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index 53e26a9f8e229..085ff4824a9b0 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -309,4 +309,6 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,
     Features.push_back("+prefer-no-gather");
   if (Args.hasArg(options::OPT_mno_scatter))
     Features.push_back("+prefer-no-scatter");
+  if (Args.hasArg(options::OPT_mapx_inline_asm_use_gpr32))
+    Features.push_back("+inline-asm-use-gpr32");
 }
diff --git a/clang/test/Driver/x86-apx-inline-asm-use-gpr32.cpp b/clang/test/Driver/x86-apx-inline-asm-use-gpr32.cpp
new file mode 100644
index 0000000000000..a45140d96e66c
--- /dev/null
+++ b/clang/test/Driver/x86-apx-inline-asm-use-gpr32.cpp
@@ -0,0 +1,3 @@
+/// Tests -mapx-inline-asm-use-gpr32
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -mapx-inline-asm-use-gpr32 -### %s 2>&1 | FileCheck --check-prefix=GPR32 %s
+// GPR32: "-target-feature" "+inline-asm-use-gpr32"
diff --git a/clang/test/Preprocessor/x86_target_features.c b/clang/test/Preprocessor/x86_target_features.c
index 5602c59158fe5..7fe19598fbd4e 100644
--- a/clang/test/Preprocessor/x86_target_features.c
+++ b/clang/test/Preprocessor/x86_target_features.c
@@ -811,3 +811,6 @@
 // NDD: #define __NDD__ 1
 // PPX: #define __PPX__ 1
 // PUSH2POP2: #define __PUSH2POP2__ 1
+
+// RUN: %clang -target x86_64-unknown-unknown -march=x86-64 -mapx-inline-asm-use-gpr32 -x c -E -dM -o - %s | FileCheck --check-prefixes=USEGPR32 %s
+// USEGPR32: #define __APX_INLINE_ASM_USE_GPR32__ 1
diff --git a/llvm/lib/Target/X86/X86.td b/llvm/lib/Target/X86/X86.td
index 9f5b58d78fcce..90a0c878a1101 100644
--- a/llvm/lib/Target/X86/X86.td
+++ b/llvm/lib/Target/X86/X86.td
@@ -353,6 +353,9 @@ def FeatureCCMP : SubtargetFeature<"ccmp", "HasCCMP", "true",
                                    "Support conditional cmp & test instructions">;
 def FeatureCF : SubtargetFeature<"cf", "HasCF", "true",
                                  "Support conditional faulting">;
+def FeatureUseGPR32InInlineAsm
+    : SubtargetFeature<"inline-asm-use-gpr32", "UseInlineAsmGPR32", "false",
+                       "Enable use of GPR32 in inline assembly for APX">;
 
 // Ivy Bridge and newer processors have enhanced REP MOVSB and STOSB (aka
 // "string operations"). See "REP String Enhancement" in the Intel Software
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 0410cc33ca337..2f381a3fc117c 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -57574,6 +57574,13 @@ X86TargetLowering::getConstraintType(StringRef Constraint) const {
       case '2':
         return C_RegisterClass;
       }
+    case 'j':
+      switch (Constraint[1]) {
+      default:
+        break;
+      case 'R':
+        return C_RegisterClass;
+      }
     }
   } else if (parseConstraintCode(Constraint) != X86::COND_INVALID)
     return C_Other;
@@ -57653,6 +57660,18 @@ X86TargetLowering::getSingleConstraintMatchWeight(
       break;
     }
     break;
+  case 'j':
+    if (StringRef(Constraint).size() != 2)
+      break;
+    switch (Constraint[1]) {
+    default:
+      return CW_Invalid;
+    case 'R':
+      if (CallOperandVal->getType()->isIntegerTy())
+        Wt = CW_SpecificReg;
+      break;
+    }
+    break;
   case 'v':
     if ((Ty->getPrimitiveSizeInBits() == 512) && Subtarget.hasAVX512())
       Wt = CW_Register;
@@ -58015,16 +58034,28 @@ X86TargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
         return std::make_pair(0U, &X86::GR64_ABCDRegClass);
       break;
     case 'r':   // GENERAL_REGS
-    case 'l':   // INDEX_REGS
+    case 'l':   // INDEX_REGS{}
+      if (Subtarget.useInlineAsmGPR32()) {
+        if (VT == MVT::i8 || VT == MVT::i1)
+          return std::make_pair(0U, &X86::GR8_NOREX2RegClass);
+        if (VT == MVT::i16)
+          return std::make_pair(0U, &X86::GR16_NOREX2RegClass);
+        if (VT == MVT::i32 || VT == MVT::f32 ||
+            (!VT.isVector() && !Subtarget.is64Bit()))
+          return std::make_pair(0U, &X86::GR32_NOREX2RegClass);
+        if (VT != MVT::f80 && !VT.isVector())
+          return std::make_pair(0U, &X86::GR64_NOREX2RegClass);
+        break;
+      }
       if (VT == MVT::i8 || VT == MVT::i1)
-        return std::make_pair(0U, &X86::GR8_NOREX2RegClass);
+        return std::make_pair(0U, &X86::GR8RegClass);
       if (VT == MVT::i16)
-        return std::make_pair(0U, &X86::GR16_NOREX2RegClass);
+        return std::make_pair(0U, &X86::GR16RegClass);
       if (VT == MVT::i32 || VT == MVT::f32 ||
           (!VT.isVector() && !Subtarget.is64Bit()))
-        return std::make_pair(0U, &X86::GR32_NOREX2RegClass);
+        return std::make_pair(0U, &X86::GR32RegClass);
       if (VT != MVT::f80 && !VT.isVector())
-        return std::make_pair(0U, &X86::GR64_NOREX2RegClass);
+        return std::make_pair(0U, &X86::GR64RegClass);
       break;
     case 'R':   // LEGACY_REGS
       if (VT == MVT::i8 || VT == MVT::i1)
@@ -58248,6 +58279,22 @@ X86TargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
       }
       break;
     }
+  } else if (Constraint.size() == 2 && Constraint[0] == 'j') {
+    switch (Constraint[1]) {
+      default:
+        break;
+      case 'R':
+        if (VT == MVT::i8 || VT == MVT::i1)
+          return std::make_pair(0U, &X86::GR8RegClass);
+        if (VT == MVT::i16)
+          return std::make_pair(0U, &X86::GR16RegClass);
+        if (VT == MVT::i32 || VT == MVT::f32 ||
+            (!VT.isVector() && !Subtarget.is64Bit()))
+          return std::make_pair(0U, &X86::GR32RegClass);
+        if (VT != MVT::f80 && !VT.isVector())
+          return std::make_pair(0U, &X86::GR64RegClass);
+        break;
+    }
   }
 
   if (parseConstraintCode(Constraint) != X86::COND_INVALID)
diff --git a/llvm/test/CodeGen/X86/inline-asm-jR-constraint.ll b/llvm/test/CodeGen/X86/inline-asm-jR-constraint.ll
new file mode 100644
index 0000000000000..2348e75433798
--- /dev/null
+++ b/llvm/test/CodeGen/X86/inline-asm-jR-constraint.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: not llc -mtriple=x86_64 %s 2>&1 | FileCheck %s --check-prefix=ERR
+; RUN: llc -mtriple=x86_64 -mattr=+egpr  < %s | FileCheck %s --check-prefix=EGPR
+; RUN: llc -mtriple=x86_64 -mattr=+egpr,+inline-asm-use-gpr32  < %s | FileCheck %s --check-prefix=EGPRUSEGPR32
+
+; ERR: error: inline assembly requires more registers than available
+
+define void @constraint_jR_test() nounwind "frame-pointer"="all" {
+; EGPR-LABEL: constraint_jR_test:
+; EGPR:    addq %r16, %rax
+;
+; EGPRUSEGPR32-LABEL: constraint_jR_test:
+; EGPRUSEGPR32:    addq %r16, %rax
+entry:
+  %reg = alloca i64, align 8
+  %0 = load i64, ptr %reg, align 8
+  call void asm sideeffect "add $0, %rax", "^jR,~{rax},~{rbx},~{rcx},~{rdx},~{rdi},~{rsi},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{dirflag},~{fpsr},~{flags}"(i64 %0)
+  ret void
+}
diff --git a/llvm/test/CodeGen/X86/inline-asm-r-constraint.ll b/llvm/test/CodeGen/X86/inline-asm-r-constraint.ll
new file mode 100644
index 0000000000000..158dcdf9be272
--- /dev/null
+++ b/llvm/test/CodeGen/X86/inline-asm-r-constraint.ll
@@ -0,0 +1,16 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: not llc -mtriple=x86_64 < %s 2>&1 | FileCheck %s --check-prefix=ERR
+; RUN: not llc -mtriple=x86_64 -mattr=+egpr < %s 2>&1 | FileCheck %s --check-prefix=ERR
+; RUN: llc -mtriple=x86_64 -mattr=+egpr,+inline-asm-use-gpr32 < %s | FileCheck %s --check-prefix=USEGPR32
+
+; ERR: error: inline assembly requires more registers than available
+
+define void @constraint_r_test() nounwind "frame-pointer"="all" {
+; USEGPR32-LABEL: constraint_r_test:
+; USEGPR32:    addq %r16, %rax
+entry:
+  %reg = alloca i64, align 8
+  %0 = load i64, ptr %reg, align 8
+  call void asm sideeffect "add $0, %rax", "r,~{rax},~{rbx},~{rcx},~{rdx},~{rdi},~{rsi},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{dirflag},~{fpsr},~{flags}"(i64 %0)
+  ret void
+}

Copy link

github-actions bot commented May 16, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@KanRobert
Copy link
Contributor

Please put the corresponding GCC links for your description

@FreddyLeaf
Copy link
Contributor Author

Please put the corresponding GCC links for your description

done.

@FreddyLeaf
Copy link
Contributor Author

ping for review

@jyknight
Copy link
Member

Please update the constraint code list https://llvm.org/docs/LangRef.html#supported-constraint-code-list with any new codes.

@FreddyLeaf
Copy link
Contributor Author

FreddyLeaf commented May 21, 2024

Please update the constraint code list https://llvm.org/docs/LangRef.html#supported-constraint-code-list with any new codes.

Thanks reminding. Added in 4087704, pls help review.

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

someone with more knowledge about EGPR should approve this as well, but I'm happy with the code style for the additions.

Copy link
Contributor

@KanRobert KanRobert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@FreddyLeaf FreddyLeaf merged commit 73f4c25 into llvm:main May 30, 2024
7 of 8 checks passed
@FreddyLeaf FreddyLeaf deleted the egpr_inline_ass branch May 30, 2024 06:47
FreddyLeaf added a commit to FreddyLeaf/llvm-project that referenced this pull request May 30, 2024
@banach-space
Copy link
Contributor

banach-space commented May 30, 2024

Hi, thanks for this contribution. Sadly, it messes up my local checkout on MacOS (which is insensitive when it comes to files names). These files are problematic:

  • "asm-constraint-jR.ll" and "asm-constraint-jr.ll"

Please, could you rename them so that they are not identical on case-insensitive file systems?

@RKSimon
Copy link
Collaborator

RKSimon commented May 30, 2024

@FreddyLeaf This is corrupting git checkouts on windows - please can you revert ?

banach-space added a commit to banach-space/llvm-project that referenced this pull request May 30, 2024
Renames asm-constraint-jR.ll and asm-constraint-jR.ll - on
case-insensitive files systems those are treated as one file. Originally
introduced in llvm#92338.
@banach-space
Copy link
Contributor

Should be fixed by #93794

banach-space added a commit that referenced this pull request May 30, 2024
Renames asm-constraint-jR.ll and asm-constraint-jR.ll - on
case-insensitive files systems those are treated as one file. Originally
introduced in #92338.
@FreddyLeaf
Copy link
Contributor Author

Sorry, I was OOO, thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category llvm:ir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants