Skip to content

Conversation

el-ev
Copy link
Member

@el-ev el-ev commented Aug 22, 2025

The function is shared between traditional CodeGen and CIR CodeGen.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Aug 22, 2025
@el-ev el-ev requested review from jmorse and SLTozer and removed request for jmorse August 22, 2025 07:50
@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Iris Shi (el-ev)

Changes

The function is shared between traditional CodeGen and CIR CodeGen.


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

3 Files Affected:

  • (modified) clang/include/clang/Basic/TargetInfo.h (+4)
  • (modified) clang/lib/Basic/TargetInfo.cpp (+49)
  • (modified) clang/lib/CodeGen/CGStmt.cpp (+3-53)
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 25b68622656fa..05c717c6d91f3 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1259,6 +1259,10 @@ class TargetInfo : public TransferrableTargetInfo,
                            ArrayRef<ConstraintInfo> OutputConstraints,
                            unsigned &Index) const;
 
+  std::string
+  simplifyConstraint(StringRef Constraint,
+                     SmallVectorImpl<ConstraintInfo> *OutCons = nullptr) const;
+
   // Constraint parm will be left pointing at the last character of
   // the constraint.  In practice, it won't be changed unless the
   // constraint is longer than one character.
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 2fbf1ee39b789..5c305900e6dcd 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/TargetParser/TargetParser.h"
 #include <cstdlib>
@@ -1034,3 +1035,51 @@ void TargetInfo::copyAuxTarget(const TargetInfo *Aux) {
   auto *Src = static_cast<const TransferrableTargetInfo*>(Aux);
   *Target = *Src;
 }
+
+std::string
+TargetInfo::simplifyConstraint(StringRef Constraint,
+                               SmallVectorImpl<ConstraintInfo> *OutCons) const {
+  std::string Result;
+
+  for (const char *I = Constraint.begin(), *E = Constraint.end(); I < E; I++) {
+    switch (*I) {
+    default:
+      Result += convertConstraint(I);
+      break;
+    // Ignore these
+    case '*':
+    case '?':
+    case '!':
+    case '=': // Will see this and the following in mult-alt constraints.
+    case '+':
+      break;
+    case '#': // Ignore the rest of the constraint alternative.
+      while (I + 1 != E && I[1] != ',')
+        I++;
+      break;
+    case '&':
+    case '%':
+      Result += *I;
+      while (I + 1 != E && I[1] == *I)
+        I++;
+      break;
+    case ',':
+      Result += "|";
+      break;
+    case 'g':
+      Result += "imr";
+      break;
+    case '[': {
+      assert(OutCons &&
+             "Must pass output names to constraints with a symbolic name");
+      unsigned Index;
+      bool ResolveResult = resolveSymbolicName(I, *OutCons, Index);
+      assert(ResolveResult && "Could not resolve symbolic name");
+      (void)ResolveResult;
+      Result += llvm::utostr(Index);
+      break;
+    }
+    }
+  }
+  return Result;
+}
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 031ef73214e76..e2184d953b49d 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2454,56 +2454,6 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) {
   CaseRangeBlock = SavedCRBlock;
 }
 
-static std::string
-SimplifyConstraint(const char *Constraint, const TargetInfo &Target,
-                 SmallVectorImpl<TargetInfo::ConstraintInfo> *OutCons=nullptr) {
-  std::string Result;
-
-  while (*Constraint) {
-    switch (*Constraint) {
-    default:
-      Result += Target.convertConstraint(Constraint);
-      break;
-    // Ignore these
-    case '*':
-    case '?':
-    case '!':
-    case '=': // Will see this and the following in mult-alt constraints.
-    case '+':
-      break;
-    case '#': // Ignore the rest of the constraint alternative.
-      while (Constraint[1] && Constraint[1] != ',')
-        Constraint++;
-      break;
-    case '&':
-    case '%':
-      Result += *Constraint;
-      while (Constraint[1] && Constraint[1] == *Constraint)
-        Constraint++;
-      break;
-    case ',':
-      Result += "|";
-      break;
-    case 'g':
-      Result += "imr";
-      break;
-    case '[': {
-      assert(OutCons &&
-             "Must pass output names to constraints with a symbolic name");
-      unsigned Index;
-      bool result = Target.resolveSymbolicName(Constraint, *OutCons, Index);
-      assert(result && "Could not resolve symbolic name"); (void)result;
-      Result += llvm::utostr(Index);
-      break;
-    }
-    }
-
-    Constraint++;
-  }
-
-  return Result;
-}
-
 /// 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.
@@ -2882,8 +2832,8 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
 
     // Simplify the output constraint.
     std::string OutputConstraint(S.getOutputConstraint(i));
-    OutputConstraint = SimplifyConstraint(OutputConstraint.c_str() + 1,
-                                          getTarget(), &OutputConstraintInfos);
+    OutputConstraint = getTarget().simplifyConstraint(
+        OutputConstraint.c_str() + 1, &OutputConstraintInfos);
 
     const Expr *OutExpr = S.getOutputExpr(i);
     OutExpr = OutExpr->IgnoreParenNoopCasts(getContext());
@@ -3045,7 +2995,7 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
 
     // Simplify the input constraint.
     std::string InputConstraint(S.getInputConstraint(i));
-    InputConstraint = SimplifyConstraint(InputConstraint.c_str(), getTarget(),
+    InputConstraint = getTarget().simplifyConstraint(InputConstraint.c_str(),
                                          &OutputConstraintInfos);
 
     InputConstraint = AddVariableConstraints(

Copy link

github-actions bot commented Aug 22, 2025

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

@el-ev el-ev force-pushed the users/el-ev/move_simplify_constraints branch from 865a3eb to 2da6e65 Compare August 22, 2025 08:00
Copy link
Member

@jmorse jmorse left a comment

Choose a reason for hiding this comment

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

This seems fine; note that while your change to use StringRef as a parameter type is cleaner, both call sites you change are passing char *'s and I believe this means we'll run strlen every time this function is called? Better to construct directly from the std::string being fed in, and add a case to break on a zero-byte to the switch IMO.

I'm cautious to give an immediate LGTM as I'm not a frequent clang contributor or familiar with the CIR work; someone elses approval would be appreciated.

Copy link
Contributor

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

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

This looks good to me. I wasn't sure about moving this to TargetInfo, as opposed to starting some new CodeGenShared component for utility functions that can be shared between LLVM IR and CIR codegen, but given the use of TargetInfo within the function, I think it makes sense in this case to move it there.

I'd like to give the LLVM IR codegen maintainers time to look at this before we merge it.

OutputConstraint = SimplifyConstraint(OutputConstraint.c_str() + 1,
getTarget(), &OutputConstraintInfos);
OutputConstraint = getTarget().simplifyConstraint(
OutputConstraint.c_str() + 1, &OutputConstraintInfos);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OutputConstraint.c_str() + 1, &OutputConstraintInfos);
StringRef(OutputConstraint).substr(1), &OutputConstraintInfos);

@@ -3045,8 +2995,8 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {

// Simplify the input constraint.
std::string InputConstraint(S.getInputConstraint(i));
InputConstraint = SimplifyConstraint(InputConstraint.c_str(), getTarget(),
&OutputConstraintInfos);
InputConstraint = getTarget().simplifyConstraint(InputConstraint.c_str(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
InputConstraint = getTarget().simplifyConstraint(InputConstraint.c_str(),
InputConstraint = getTarget().simplifyConstraint(InputConstraint,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants