Skip to content

[TargetLowering] Deduplicate choosing InlineAsm constraint between ISels #67057

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.

Sign up for GitHub

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 6 commits into from
Sep 25, 2023
Merged

[TargetLowering] Deduplicate choosing InlineAsm constraint between ISels #67057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d40003
[TargetLowering] Deduplicate choosing InlineAsm constraint between ISels
nickdesaulniers Sep 21, 2023
3ac190b
[TargetLowering] use less std::string
nickdesaulniers Sep 21, 2023
5d1d8ab
remove const
nickdesaulniers Sep 21, 2023
2d16f27
fix typo
nickdesaulniers Sep 21, 2023
a542e81
improve fn comment for getConstraintPiority, reorder case statements
nickdesaulniers Sep 21, 2023
a0cb4cb
bound lowerImmediateIfPossible loop
nickdesaulniers Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion llvm/include/llvm/CodeGen/TargetLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 contain 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.
///
Expand Down Expand Up @@ -4853,7 +4862,7 @@ 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, StringRef Constraint,
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const;

Expand Down
78 changes: 12 additions & 66 deletions llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -207,7 +142,18 @@ 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;
for (const unsigned E = G.size();
BestIdx < E && (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.
Expand Down
111 changes: 62 additions & 49 deletions llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -5701,20 +5702,27 @@ 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). A higher return value means a
/// stronger preference for one constraint type relative to another.
/// 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_Register:
return 1;
case TargetLowering::C_RegisterClass:
return 2;
return 4;
case TargetLowering::C_Memory:
case TargetLowering::C_Address:
return 3;
case TargetLowering::C_RegisterClass:
return 2;
case TargetLowering::C_Register:
return 1;
case TargetLowering::C_Unknown:
return 0;
}
llvm_unreachable("Invalid constraint type");
}
Expand Down Expand Up @@ -5812,59 +5820,51 @@ 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 ||
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) && 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) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, thank you @sam-mccall !

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,
/// false otherwise.
static bool lowerImmediateIfPossible(TargetLowering::ConstraintPair &P,
SDValue Op, SelectionDAG *DAG,
const TargetLowering &TLI) {

assert((P.second == TargetLowering::C_Other ||
P.second == TargetLowering::C_Immediate) &&
"need immediate or other");

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
Expand All @@ -5879,7 +5879,20 @@ 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;
for (const unsigned E = G.size();
BestIdx < E && (G[BestIdx].second == TargetLowering::C_Other ||
G[BestIdx].second == TargetLowering::C_Immediate);
++BestIdx)
if (lowerImmediateIfPossible(G[BestIdx], Op, DAG, *this))
break;

OpInfo.ConstraintCode = G[BestIdx].first;
OpInfo.ConstraintType = G[BestIdx].second;
}

// 'X' matches anything.
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AArch64/AArch64ISelLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
5 changes: 2 additions & 3 deletions llvm/lib/Target/AMDGPU/SIISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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]) {
Expand Down
6 changes: 2 additions & 4 deletions llvm/lib/Target/AMDGPU/SIISelLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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,
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/Target/ARM/ARMISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@
#include <iterator>
#include <limits>
#include <optional>
#include <string>
#include <tuple>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -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,");

Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/ARM/ARMISelLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading