[TargetLowering] Deduplicate choosing InlineAsm constraint between ISels #67057
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
Given a list of constraints for InlineAsm (ex. "imr") I'm looking to modify the order in which is chosen. Before doing so, I noticed a fair amount of logic is duplicated between SelectionDAGISel and GlobalISel for this. That is because SelectionDAGISel is also trying to lower immediates during selection. If we detangle these concerns into: 1. choose the preferred constraint 2. attempt to lower that constraint Then we can slide down the list of constraints until we find one that can be lowered. That allows the implementation to be shared between instruction selection frameworks. This makes it so that later I might only need to adjust the priority of constraints in one place, and have both selectors behave the same.
I want to use StringRef, let me replace one use of std::string.
llvmbot
added
backend:ARM
backend:AArch64
backend:AMDGPU
backend:RISC-V
backend:Sparc
backend:X86
llvm:globalisel
backend:m68k
backend:loongarch
llvm:SelectionDAG
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-backend-sparc ChangesGiven a list of constraints for InlineAsm (ex. "imr") I'm looking to That is because SelectionDAGISel is also trying to lower immediates
This makes it so that later I might only need to adjust the priority of Patch is 36.94 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67057.diff 31 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index dabf1bedafe6389..fd41d6a5ab751dc 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -4820,6 +4820,15 @@ class TargetLowering : public TargetLoweringBase {
/// Given a constraint, return the type of constraint it is for this target.
virtual ConstraintType getConstraintType(StringRef Constraint) const;
+ using ConstraintPair = std::pair<StringRef, TargetLowering::ConstraintType>;
+ using ConstraintGroup = SmallVector<ConstraintPair>;
+ /// Given an OpInfo with list of constraints codes as strings, return a
+ /// sorted Vector of pairs of constraint codes and their types in priority of
+ /// what we'd prefer to lower them as. This may contains immediates that
+ /// cannot be lowered, but it is meant to be a machine agnostic order of
+ /// preferences.
+ ConstraintGroup getConstraintPreferences(AsmOperandInfo &OpInfo) const;
+
/// Given a physical register constraint (e.g. {edx}), return the register
/// number and the register class for the register.
///
@@ -4853,7 +4862,8 @@ class TargetLowering : public TargetLoweringBase {
/// Lower the specified operand into the Ops vector. If it is invalid, don't
/// add anything to Ops.
- virtual void LowerAsmOperandForConstraint(SDValue Op, std::string &Constraint,
+ virtual void LowerAsmOperandForConstraint(SDValue Op,
+ const StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const;
diff --git a/llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp b/llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
index 00dba57fcb80227..a6d802c91f309c1 100644
--- a/llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
@@ -133,71 +133,6 @@ static void getRegistersForValue(MachineFunction &MF,
}
}
-/// Return an integer indicating how general CT is.
-static unsigned getConstraintGenerality(TargetLowering::ConstraintType CT) {
- switch (CT) {
- case TargetLowering::C_Immediate:
- case TargetLowering::C_Other:
- case TargetLowering::C_Unknown:
- return 0;
- case TargetLowering::C_Register:
- return 1;
- case TargetLowering::C_RegisterClass:
- return 2;
- case TargetLowering::C_Memory:
- case TargetLowering::C_Address:
- return 3;
- }
- llvm_unreachable("Invalid constraint type");
-}
-
-static void chooseConstraint(TargetLowering::AsmOperandInfo &OpInfo,
- const TargetLowering *TLI) {
- assert(OpInfo.Codes.size() > 1 && "Doesn't have multiple constraint options");
- unsigned BestIdx = 0;
- TargetLowering::ConstraintType BestType = TargetLowering::C_Unknown;
- int BestGenerality = -1;
-
- // Loop over the options, keeping track of the most general one.
- for (unsigned i = 0, e = OpInfo.Codes.size(); i != e; ++i) {
- TargetLowering::ConstraintType CType =
- TLI->getConstraintType(OpInfo.Codes[i]);
-
- // Indirect 'other' or 'immediate' constraints are not allowed.
- if (OpInfo.isIndirect && !(CType == TargetLowering::C_Memory ||
- CType == TargetLowering::C_Register ||
- CType == TargetLowering::C_RegisterClass))
- continue;
-
- // If this is an 'other' or 'immediate' constraint, see if the operand is
- // valid for it. For example, on X86 we might have an 'rI' constraint. If
- // the operand is an integer in the range [0..31] we want to use I (saving a
- // load of a register), otherwise we must use 'r'.
- if (CType == TargetLowering::C_Other ||
- CType == TargetLowering::C_Immediate) {
- assert(OpInfo.Codes[i].size() == 1 &&
- "Unhandled multi-letter 'other' constraint");
- // FIXME: prefer immediate constraints if the target allows it
- }
-
- // Things with matching constraints can only be registers, per gcc
- // documentation. This mainly affects "g" constraints.
- if (CType == TargetLowering::C_Memory && OpInfo.hasMatchingInput())
- continue;
-
- // This constraint letter is more general than the previous one, use it.
- int Generality = getConstraintGenerality(CType);
- if (Generality > BestGenerality) {
- BestType = CType;
- BestIdx = i;
- BestGenerality = Generality;
- }
- }
-
- OpInfo.ConstraintCode = OpInfo.Codes[BestIdx];
- OpInfo.ConstraintType = BestType;
-}
-
static void computeConstraintToUse(const TargetLowering *TLI,
TargetLowering::AsmOperandInfo &OpInfo) {
assert(!OpInfo.Codes.empty() && "Must have at least one constraint");
@@ -207,7 +142,16 @@ static void computeConstraintToUse(const TargetLowering *TLI,
OpInfo.ConstraintCode = OpInfo.Codes[0];
OpInfo.ConstraintType = TLI->getConstraintType(OpInfo.ConstraintCode);
} else {
- chooseConstraint(OpInfo, TLI);
+ TargetLowering::ConstraintGroup G = TLI->getConstraintPreferences(OpInfo);
+ if (G.empty())
+ return;
+ // FIXME: prefer immediate constraints if the target allows it
+ unsigned BestIdx = 0;
+ while (G[BestIdx].second == TargetLowering::C_Other ||
+ G[BestIdx].second == TargetLowering::C_Immediate)
+ ++BestIdx;
+ OpInfo.ConstraintCode = G[BestIdx].first;
+ OpInfo.ConstraintType = G[BestIdx].second;
}
// 'X' matches anything.
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index bd1940994a87f0f..a6ac340937fc0d5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -5378,11 +5378,12 @@ SDValue TargetLowering::LowerAsmOutputForConstraint(
/// Lower the specified operand into the Ops vector.
/// If it is invalid, don't add anything to Ops.
void TargetLowering::LowerAsmOperandForConstraint(SDValue Op,
- std::string &Constraint,
+ StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const {
- if (Constraint.length() > 1) return;
+ if (Constraint.size() > 1)
+ return;
char ConstraintLetter = Constraint[0];
switch (ConstraintLetter) {
@@ -5701,13 +5702,19 @@ TargetLowering::ParseConstraints(const DataLayout &DL,
return ConstraintOperands;
}
-/// Return an integer indicating how general CT is.
-static unsigned getConstraintGenerality(TargetLowering::ConstraintType CT) {
+/// Return a number indicating our preference for chosing a type of constraint
+/// over another, for the purpose of sorting them. Immediates are almost always
+/// preferrable (when they can be emitted).
+/// FIXME: We should prefer registers over memory but doing so may lead to
+/// unrecoverable register exhaustion later.
+/// https://github.com/llvm/llvm-project/issues/20571
+static unsigned getConstraintPiority(TargetLowering::ConstraintType CT) {
switch (CT) {
- case TargetLowering::C_Immediate:
- case TargetLowering::C_Other:
case TargetLowering::C_Unknown:
return 0;
+ case TargetLowering::C_Immediate:
+ case TargetLowering::C_Other:
+ return 4;
case TargetLowering::C_Register:
return 1;
case TargetLowering::C_RegisterClass:
@@ -5812,18 +5819,13 @@ TargetLowering::ConstraintWeight
/// 2) Otherwise, pick the most general constraint present. This prefers
/// 'm' over 'r', for example.
///
-static void ChooseConstraint(TargetLowering::AsmOperandInfo &OpInfo,
- const TargetLowering &TLI,
- SDValue Op, SelectionDAG *DAG) {
- assert(OpInfo.Codes.size() > 1 && "Doesn't have multiple constraint options");
- unsigned BestIdx = 0;
- TargetLowering::ConstraintType BestType = TargetLowering::C_Unknown;
- int BestGenerality = -1;
+TargetLowering::ConstraintGroup TargetLowering::getConstraintPreferences(
+ TargetLowering::AsmOperandInfo &OpInfo) const {
+ ConstraintGroup Ret;
- // Loop over the options, keeping track of the most general one.
- for (unsigned i = 0, e = OpInfo.Codes.size(); i != e; ++i) {
- TargetLowering::ConstraintType CType =
- TLI.getConstraintType(OpInfo.Codes[i]);
+ Ret.reserve(OpInfo.Codes.size());
+ for (StringRef Code : OpInfo.Codes) {
+ TargetLowering::ConstraintType CType = getConstraintType(Code);
// Indirect 'other' or 'immediate' constraints are not allowed.
if (OpInfo.isIndirect && !(CType == TargetLowering::C_Memory ||
@@ -5831,40 +5833,36 @@ static void ChooseConstraint(TargetLowering::AsmOperandInfo &OpInfo,
CType == TargetLowering::C_RegisterClass))
continue;
- // If this is an 'other' or 'immediate' constraint, see if the operand is
- // valid for it. For example, on X86 we might have an 'rI' constraint. If
- // the operand is an integer in the range [0..31] we want to use I (saving a
- // load of a register), otherwise we must use 'r'.
- if ((CType == TargetLowering::C_Other ||
- CType == TargetLowering::C_Immediate) && Op.getNode()) {
- assert(OpInfo.Codes[i].size() == 1 &&
- "Unhandled multi-letter 'other' constraint");
- std::vector<SDValue> ResultOps;
- TLI.LowerAsmOperandForConstraint(Op, OpInfo.Codes[i],
- ResultOps, *DAG);
- if (!ResultOps.empty()) {
- BestType = CType;
- BestIdx = i;
- break;
- }
- }
-
// Things with matching constraints can only be registers, per gcc
// documentation. This mainly affects "g" constraints.
if (CType == TargetLowering::C_Memory && OpInfo.hasMatchingInput())
continue;
- // This constraint letter is more general than the previous one, use it.
- int Generality = getConstraintGenerality(CType);
- if (Generality > BestGenerality) {
- BestType = CType;
- BestIdx = i;
- BestGenerality = Generality;
- }
+ Ret.emplace_back(Code, CType);
}
- OpInfo.ConstraintCode = OpInfo.Codes[BestIdx];
- OpInfo.ConstraintType = BestType;
+ std::sort(Ret.begin(), Ret.end(), [](ConstraintPair a, ConstraintPair b) {
+ return getConstraintPiority(a.second) > getConstraintPiority(b.second);
+ });
+
+ return Ret;
+}
+
+// If we have an immediate, see if we can lower it. Return true if we can, or
+// don't have an immediate, false otherwise.
+static bool lowerImmediateIfPossible(TargetLowering::ConstraintPair &P,
+ SDValue Op, SelectionDAG *DAG,
+ const TargetLowering &TLI) {
+
+ if (P.second != TargetLowering::C_Other &&
+ P.second != TargetLowering::C_Immediate)
+ return true;
+ if (!Op.getNode())
+ return false;
+
+ std::vector<SDValue> ResultOps;
+ TLI.LowerAsmOperandForConstraint(Op, P.first, ResultOps, *DAG);
+ return !ResultOps.empty();
}
/// Determines the constraint code and constraint type to use for the specific
@@ -5879,7 +5877,19 @@ void TargetLowering::ComputeConstraintToUse(AsmOperandInfo &OpInfo,
OpInfo.ConstraintCode = OpInfo.Codes[0];
OpInfo.ConstraintType = getConstraintType(OpInfo.ConstraintCode);
} else {
- ChooseConstraint(OpInfo, *this, Op, DAG);
+ ConstraintGroup G = getConstraintPreferences(OpInfo);
+ if (G.empty())
+ return;
+
+ unsigned BestIdx = 0;
+ while (!lowerImmediateIfPossible(G[BestIdx], Op, DAG, *this)) {
+ if (BestIdx + 1 > G.size() - 1)
+ return;
+ ++BestIdx;
+ }
+
+ OpInfo.ConstraintCode = G[BestIdx].first;
+ OpInfo.ConstraintType = G[BestIdx].second;
}
// 'X' matches anything.
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index fd6068c9787e6c5..0f11fd96e6bf3f8 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -10325,12 +10325,12 @@ EVT AArch64TargetLowering::getAsmOperandValueType(const DataLayout &DL,
/// LowerAsmOperandForConstraint - Lower the specified operand into the Ops
/// vector. If it is invalid, don't add anything to Ops.
void AArch64TargetLowering::LowerAsmOperandForConstraint(
- SDValue Op, std::string &Constraint, std::vector<SDValue> &Ops,
+ SDValue Op, StringRef Constraint, std::vector<SDValue> &Ops,
SelectionDAG &DAG) const {
SDValue Result;
// Currently only support length 1 constraints.
- if (Constraint.length() != 1)
+ if (Constraint.size() != 1)
return;
char ConstraintLetter = Constraint[0];
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.h b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
index 48b5bc16b36537b..822d926e77b48cd 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -1165,7 +1165,7 @@ class AArch64TargetLowering : public TargetLowering {
const char *LowerXConstraint(EVT ConstraintVT) const override;
- void LowerAsmOperandForConstraint(SDValue Op, std::string &Constraint,
+ void LowerAsmOperandForConstraint(SDValue Op, StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const override;
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 1c85ec3f9f5212f..7d4812e20811aba 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -13817,7 +13817,7 @@ static uint64_t clearUnusedBits(uint64_t Val, unsigned Size) {
}
void SITargetLowering::LowerAsmOperandForConstraint(SDValue Op,
- std::string &Constraint,
+ StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const {
if (isImmConstraint(Constraint)) {
@@ -13866,8 +13866,7 @@ bool SITargetLowering::getAsmOperandConstVal(SDValue Op, uint64_t &Val) const {
return false;
}
-bool SITargetLowering::checkAsmConstraintVal(SDValue Op,
- const std::string &Constraint,
+bool SITargetLowering::checkAsmConstraintVal(SDValue Op, StringRef Constraint,
uint64_t Val) const {
if (Constraint.size() == 1) {
switch (Constraint[0]) {
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.h b/llvm/lib/Target/AMDGPU/SIISelLowering.h
index ffe49f651fdb307..1a3f829d787bcf6 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.h
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.h
@@ -468,13 +468,11 @@ class SITargetLowering final : public AMDGPUTargetLowering {
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
ConstraintType getConstraintType(StringRef Constraint) const override;
- void LowerAsmOperandForConstraint(SDValue Op,
- std::string &Constraint,
+ void LowerAsmOperandForConstraint(SDValue Op, const StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const override;
bool getAsmOperandConstVal(SDValue Op, uint64_t &Val) const;
- bool checkAsmConstraintVal(SDValue Op,
- const std::string &Constraint,
+ bool checkAsmConstraintVal(SDValue Op, StringRef Constraint,
uint64_t Val) const;
bool checkAsmConstraintValA(SDValue Op,
uint64_t Val,
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 21489a61e576f5d..b9b118f821038b5 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -111,7 +111,6 @@
#include <iterator>
#include <limits>
#include <optional>
-#include <string>
#include <tuple>
#include <utility>
#include <vector>
@@ -20252,14 +20251,14 @@ bool ARMTargetLowering::ExpandInlineAsm(CallInst *CI) const {
return false;
InlineAsm *IA = cast<InlineAsm>(CI->getCalledOperand());
- std::string AsmStr = IA->getAsmString();
+ StringRef AsmStr = IA->getAsmString();
SmallVector<StringRef, 4> AsmPieces;
SplitString(AsmStr, AsmPieces, ";\n");
switch (AsmPieces.size()) {
default: return false;
case 1:
- AsmStr = std::string(AsmPieces[0]);
+ AsmStr = AsmPieces[0];
AsmPieces.clear();
SplitString(AsmStr, AsmPieces, " \t,");
@@ -20439,13 +20438,14 @@ RCPair ARMTargetLowering::getRegForInlineAsmConstraint(
/// LowerAsmOperandForConstraint - Lower the specified operand into the Ops
/// vector. If it is invalid, don't add anything to Ops.
void ARMTargetLowering::LowerAsmOperandForConstraint(SDValue Op,
- std::string &Constraint,
- std::vector<SDValue>&Ops,
+ StringRef Constraint,
+ std::vector<SDValue> &Ops,
SelectionDAG &DAG) const {
SDValue Result;
// Currently only support length 1 constraints.
- if (Constraint.length() != 1) return;
+ if (Constraint.size() != 1)
+ return;
char ConstraintLetter = Constraint[0];
switch (ConstraintLetter) {
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.h b/llvm/lib/Target/ARM/ARMISelLowering.h
index efa82386768605d..6c2b92de7a1df2f 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.h
+++ b/llvm/lib/Target/ARM/ARMISelLowering.h
@@ -530,7 +530,7 @@ class VectorType;
/// vector. If it is invalid, don't add anything to Ops. If hasMemory is
/// true it means one of the asm constraint of the inline asm instruction
/// being processed is 'm'.
- void LowerAsmOperandForConstraint(SDValue Op, std::string &Constraint,
+ void LowerAsmOperandForConstraint(SDValue Op, StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const override;
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.cpp b/llvm/lib/Target/AVR/AVRISelLowering.cpp
index 4f5a48bd05a4e86..cd1dcfaea0eb178 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRISelLowering.cpp
@@ -2722,7 +2722,7 @@ AVRTargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
}
void AVRTargetLowering::LowerAsmOperandForConstraint(SDValue Op,
- std::string &Constraint,
+ StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const {
SDValue Result;
@@ -2730,7 +2730,7 @@ void AVRTargetLowering::LowerAsmOperandForConstraint(SDValue Op,
EVT Ty = Op.getValueType();
// Currently only support length 1 constraints.
- if (Constraint.length() != 1) {
+ if (Constraint.size() != 1) {
return;
}
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.h b/llvm/lib/Target/AVR/AVRISelLowering.h
index 6815b519bebf74d..f60579593453258 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.h
+++ b/llvm/lib/Target/AVR/AVRISelLowering.h
@@ -136,7 +136,7 @@ class AVRTargetLowering : public TargetLowering {
InlineAsm::ConstraintCode
getInlineAsmMemConstraint(StringRef ConstraintCode) const override;
- void LowerAsmOperandForConstraint(SDValue Op, std::string &Constraint,
+ void LowerAsmOperandForConstraint(SDValue Op, StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const override;
diff --git a/llvm/lib/Target/Lanai/LanaiISelLowering.cpp b/llvm/lib/Target/Lanai/LanaiISelLowering.cpp
index 157f8602743390a..cbb5c2b998e27aa 100644
--- a/llvm/lib/Target/Lanai/LanaiISelLowering.cpp
+++ b/llvm/lib/Target/Lana...
[truncated]
|
nickdesaulniers
requested review from
echristo,
MaskRay,
jyknight,
qcolombet,
efriedma-quic and
bwendling
bwendling
reviewed
Sep 21, 2023
bwendling
approved these changes
Sep 21, 2023
qcolombet
approved these changes
Sep 22, 2023
"in which is chosen" - are you missing some words here? |
maybe |
As a native speaker (sorry I don't know if you are too) I would probably say "the order in which they are chosen". |
sam-mccall
reviewed
Sep 26, 2023
| } | ||
|
|
||
| OpInfo.ConstraintCode = OpInfo.Codes[BestIdx]; | ||
| OpInfo.ConstraintType = BestType; | ||
| std::sort(Ret.begin(), Ret.end(), [](ConstraintPair a, ConstraintPair b) { |
There was a problem hiding this comment.
this sort() seems to have introduced nondeterminism in some configs
I've replaced it with stable_sort in 679c3a1 - maybe you have a better fix, but this seemed like a safe way to avoid reverting.
(My reading is the old code picks the first best option & stable_sort restores this)
There was a problem hiding this comment.
yes, thank you @sam-mccall !
nickdesaulniers
added a commit
to nickdesaulniers/llvm-project
that referenced
this pull request
Sep 26, 2023
I accidentally introduced this in commit 330fa7d ("[TargetLowering] Deduplicate choosing InlineAsm constraint between ISels (llvm#67057)") Fix forward.
|
Another follow up fix: #67494 |
nickdesaulniers
added a commit
that referenced
this pull request
Sep 26, 2023
legrosbuffle
pushed a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
I accidentally introduced this in commit 330fa7d ("[TargetLowering] Deduplicate choosing InlineAsm constraint between ISels (llvm#67057)") Fix forward.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Given a list of constraints for InlineAsm (ex. "imr") I'm looking to
modify the order in which they are chosen. Before doing so, I noticed a fair
amount of logic is duplicated between SelectionDAGISel and GlobalISel
for this.
That is because SelectionDAGISel is also trying to lower immediates
during selection. If we detangle these concerns into:
Then we can slide down the list of constraints until we find one that
can be lowered. That allows the implementation to be shared between
instruction selection frameworks.
This makes it so that later I might only need to adjust the priority of
constraints in one place, and have both selectors behave the same.