Skip to content

reland [InlineAsm] wrap ConstraintCode in enum class NFC #66264

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 3 commits into from
Sep 13, 2023
Merged

reland [InlineAsm] wrap ConstraintCode in enum class NFC #66264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab68742
reland [InlineAsm] wrap ConstraintCode in enum class NFC (#66003)
nickdesaulniers Sep 13, 2023
0662f8d
fix all the targets I missed in #66003
nickdesaulniers Sep 13, 2023
fd0fa2b
fix off by one found by llvm/test/CodeGen/SystemZ/inline-asm-addr.ll
nickdesaulniers Sep 13, 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
7 changes: 4 additions & 3 deletions llvm/include/llvm/CodeGen/SelectionDAGISel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ class SelectionDAGISel : public MachineFunctionPass {
/// not match or is not implemented, return true. The resultant operands
/// (which will appear in the machine instruction) should be added to the
/// OutOps vector.
virtual bool SelectInlineAsmMemoryOperand(const SDValue &Op,
unsigned ConstraintID,
std::vector<SDValue> &OutOps) {
virtual bool
SelectInlineAsmMemoryOperand(const SDValue &Op,
InlineAsm::ConstraintCode ConstraintID,
std::vector<SDValue> &OutOps) {
return true;
}

Expand Down
13 changes: 7 additions & 6 deletions llvm/include/llvm/CodeGen/TargetLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4833,16 +4833,17 @@ class TargetLowering : public TargetLoweringBase {
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const;

virtual unsigned getInlineAsmMemConstraint(StringRef ConstraintCode) const {
virtual InlineAsm::ConstraintCode
getInlineAsmMemConstraint(StringRef ConstraintCode) const {
if (ConstraintCode == "m")
return InlineAsm::Constraint_m;
return InlineAsm::ConstraintCode::m;
if (ConstraintCode == "o")
return InlineAsm::Constraint_o;
return InlineAsm::ConstraintCode::o;
if (ConstraintCode == "X")
return InlineAsm::Constraint_X;
return InlineAsm::ConstraintCode::X;
if (ConstraintCode == "p")
return InlineAsm::Constraint_p;
return InlineAsm::Constraint_Unknown;
return InlineAsm::ConstraintCode::p;
return InlineAsm::ConstraintCode::Unknown;
}

/// Try to replace an X constraint, which matches anything, with another that
Expand Down
156 changes: 78 additions & 78 deletions llvm/include/llvm/IR/InlineAsm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,48 +217,6 @@ class InlineAsm final : public Value {
Extra_MayLoad = 8,
Extra_MayStore = 16,
Extra_IsConvergent = 32,

// Memory constraint codes.
// These could be tablegenerated but there's little need to do that since
// there's plenty of space in the encoding to support the union of all
// constraint codes for all targets.
// Addresses are included here as they need to be treated the same by the
// backend, the only difference is that they are not used to actaully
// access memory by the instruction.
// TODO: convert to enum?
Constraint_Unknown = 0,
Constraint_es,
Constraint_i,
Constraint_k,
Constraint_m,
Constraint_o,
Constraint_v,
Constraint_A,
Constraint_Q,
Constraint_R,
Constraint_S,
Constraint_T,
Constraint_Um,
Constraint_Un,
Constraint_Uq,
Constraint_Us,
Constraint_Ut,
Constraint_Uv,
Constraint_Uy,
Constraint_X,
Constraint_Z,
Constraint_ZB,
Constraint_ZC,
Constraint_Zy,

// Address constraints
Constraint_p,
Constraint_ZQ,
Constraint_ZR,
Constraint_ZS,
Constraint_ZT,

Constraints_Max = Constraint_ZT,
};

// Inline asm operands map to multiple SDNode / MachineInstr operands.
Expand All @@ -274,6 +232,46 @@ class InlineAsm final : public Value {
Func = 7, // Address operand of function call
};

// Memory constraint codes.
// Addresses are included here as they need to be treated the same by the
// backend, the only difference is that they are not used to actaully
// access memory by the instruction.
enum class ConstraintCode : uint32_t {
Unknown = 0,
es,
i,
k,
m,
o,
v,
A,
Q,
R,
S,
T,
Um,
Un,
Uq,
Us,
Ut,
Uv,
Uy,
X,
Z,
ZB,
ZC,
Zy,

// Address constraints
p,
ZQ,
ZR,
ZS,
ZT,

Max = ZT,
};

// These are helper methods for dealing with flags in the INLINEASM SDNode
// in the backend.
//
Expand Down Expand Up @@ -375,11 +373,14 @@ class InlineAsm final : public Value {
return true;
}

// TODO: convert to enum?
unsigned getMemoryConstraintID() const {
ConstraintCode getMemoryConstraintID() const {
assert((isMemKind() || isFuncKind()) &&
"Not expected mem or function flag!");
return getData();
uint32_t D = getData();
assert(D <= static_cast<uint32_t>(ConstraintCode::Max) &&
D >= static_cast<uint32_t>(ConstraintCode::Unknown) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that this triggers type-limits warning (if it's enabled) on GCC 7 and 8.

error: comparison of unsigned expression >= 0 is always true

Don't know if there is a nice way to silence it and if we want to keep this check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the report.

I'm about to delete this newly added code in #66297.

"unexpected value for memory constraint");
return static_cast<ConstraintCode>(D);
}

/// setMatchingOp - Augment an existing flag with information indicating
Expand All @@ -403,12 +404,11 @@ class InlineAsm final : public Value {

/// setMemConstraint - Augment an existing flag with the constraint code for
/// a memory constraint.
void setMemConstraint(unsigned Constraint) {
void setMemConstraint(ConstraintCode C) {
assert((isMemKind() || isFuncKind()) &&
"Flag is not a memory or function constraint!");
assert(Constraint <= Constraints_Max && "Unknown constraint ID");
assert(getData() == 0 && "Mem constraint already set");
setData(Constraint);
setData(static_cast<uint32_t>(C));
}
/// clearMemConstraint - Similar to setMemConstraint(0), but without the
/// assertion checking that the constraint has not been set previously.
Expand Down Expand Up @@ -443,63 +443,63 @@ class InlineAsm final : public Value {
return Result;
}

static StringRef getMemConstraintName(unsigned Constraint) {
switch (Constraint) {
case InlineAsm::Constraint_es:
static StringRef getMemConstraintName(ConstraintCode C) {
switch (C) {
case ConstraintCode::es:
return "es";
case InlineAsm::Constraint_i:
case ConstraintCode::i:
return "i";
case InlineAsm::Constraint_k:
case ConstraintCode::k:
return "k";
case InlineAsm::Constraint_m:
case ConstraintCode::m:
return "m";
case InlineAsm::Constraint_o:
case ConstraintCode::o:
return "o";
case InlineAsm::Constraint_v:
case ConstraintCode::v:
return "v";
case InlineAsm::Constraint_A:
case ConstraintCode::A:
return "A";
case InlineAsm::Constraint_Q:
case ConstraintCode::Q:
return "Q";
case InlineAsm::Constraint_R:
case ConstraintCode::R:
return "R";
case InlineAsm::Constraint_S:
case ConstraintCode::S:
return "S";
case InlineAsm::Constraint_T:
case ConstraintCode::T:
return "T";
case InlineAsm::Constraint_Um:
case ConstraintCode::Um:
return "Um";
case InlineAsm::Constraint_Un:
case ConstraintCode::Un:
return "Un";
case InlineAsm::Constraint_Uq:
case ConstraintCode::Uq:
return "Uq";
case InlineAsm::Constraint_Us:
case ConstraintCode::Us:
return "Us";
case InlineAsm::Constraint_Ut:
case ConstraintCode::Ut:
return "Ut";
case InlineAsm::Constraint_Uv:
case ConstraintCode::Uv:
return "Uv";
case InlineAsm::Constraint_Uy:
case ConstraintCode::Uy:
return "Uy";
case InlineAsm::Constraint_X:
case ConstraintCode::X:
return "X";
case InlineAsm::Constraint_Z:
case ConstraintCode::Z:
return "Z";
case InlineAsm::Constraint_ZB:
case ConstraintCode::ZB:
return "ZB";
case InlineAsm::Constraint_ZC:
case ConstraintCode::ZC:
return "ZC";
case InlineAsm::Constraint_Zy:
case ConstraintCode::Zy:
return "Zy";
case InlineAsm::Constraint_p:
case ConstraintCode::p:
return "p";
case InlineAsm::Constraint_ZQ:
case ConstraintCode::ZQ:
return "ZQ";
case InlineAsm::Constraint_ZR:
case ConstraintCode::ZR:
return "ZR";
case InlineAsm::Constraint_ZS:
case ConstraintCode::ZS:
return "ZS";
case InlineAsm::Constraint_ZT:
case ConstraintCode::ZT:
return "ZT";
default:
llvm_unreachable("Unknown memory constraint");
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,9 @@ bool InlineAsmLowering::lowerInlineAsm(
switch (OpInfo.Type) {
case InlineAsm::isOutput:
if (OpInfo.ConstraintType == TargetLowering::C_Memory) {
unsigned ConstraintID =
const InlineAsm::ConstraintCode ConstraintID =
TLI->getInlineAsmMemConstraint(OpInfo.ConstraintCode);
assert(ConstraintID != InlineAsm::Constraint_Unknown &&
assert(ConstraintID != InlineAsm::ConstraintCode::Unknown &&
"Failed to convert memory constraint code to constraint id.");

// Add information to the INLINEASM instruction to know about this
Expand Down Expand Up @@ -517,7 +517,7 @@ bool InlineAsmLowering::lowerInlineAsm(

assert(OpInfo.isIndirect && "Operand must be indirect to be a mem!");

unsigned ConstraintID =
const InlineAsm::ConstraintCode ConstraintID =
TLI->getInlineAsmMemConstraint(OpInfo.ConstraintCode);
InlineAsm::Flag OpFlags(InlineAsm::Kind::Mem, 1);
OpFlags.setMemConstraint(ConstraintID);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/MachineInstr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1778,7 +1778,7 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
}

if (F.isMemKind()) {
const unsigned MCID = F.getMemoryConstraintID();
const InlineAsm::ConstraintCode MCID = F.getMemoryConstraintID();
OS << ":" << InlineAsm::getMemConstraintName(MCID);
}

Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9281,9 +9281,9 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
switch (OpInfo.Type) {
case InlineAsm::isOutput:
if (OpInfo.ConstraintType == TargetLowering::C_Memory) {
unsigned ConstraintID =
const InlineAsm::ConstraintCode ConstraintID =
TLI.getInlineAsmMemConstraint(OpInfo.ConstraintCode);
assert(ConstraintID != InlineAsm::Constraint_Unknown &&
assert(ConstraintID != InlineAsm::ConstraintCode::Unknown &&
"Failed to convert memory constraint code to constraint id.");

// Add information to the INLINEASM node to know about this output.
Expand Down Expand Up @@ -9413,9 +9413,9 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
TLI.getPointerTy(DAG.getDataLayout()) &&
"Memory operands expect pointer values");

unsigned ConstraintID =
const InlineAsm::ConstraintCode ConstraintID =
TLI.getInlineAsmMemConstraint(OpInfo.ConstraintCode);
assert(ConstraintID != InlineAsm::Constraint_Unknown &&
assert(ConstraintID != InlineAsm::ConstraintCode::Unknown &&
"Failed to convert memory constraint code to constraint id.");

// Add information to the INLINEASM node to know about this input.
Expand All @@ -9429,9 +9429,9 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
}

if (OpInfo.ConstraintType == TargetLowering::C_Address) {
unsigned ConstraintID =
const InlineAsm::ConstraintCode ConstraintID =
TLI.getInlineAsmMemConstraint(OpInfo.ConstraintCode);
assert(ConstraintID != InlineAsm::Constraint_Unknown &&
assert(ConstraintID != InlineAsm::ConstraintCode::Unknown &&
"Failed to convert memory constraint code to constraint id.");

InlineAsm::Flag ResOpType(InlineAsm::Kind::Mem, 1);
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2101,7 +2101,8 @@ void SelectionDAGISel::SelectInlineAsmMemoryOperands(std::vector<SDValue> &Ops,

// Otherwise, this is a memory operand. Ask the target to select it.
std::vector<SDValue> SelOps;
unsigned ConstraintID = Flags.getMemoryConstraintID();
const InlineAsm::ConstraintCode ConstraintID =
Flags.getMemoryConstraintID();
if (SelectInlineAsmMemoryOperand(InOps[i+1], ConstraintID, SelOps))
report_fatal_error("Could not match memory address. Inline asm"
" failure!");
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/TargetInstrInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1622,7 +1622,7 @@ std::string TargetInstrInfo::createMIROperandComment(
}

if (F.isMemKind()) {
const unsigned MCID = F.getMemoryConstraintID();
InlineAsm::ConstraintCode MCID = F.getMemoryConstraintID();
OS << ":" << InlineAsm::getMemConstraintName(MCID);
}

Expand Down
11 changes: 6 additions & 5 deletions llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class AArch64DAGToDAGISel : public SelectionDAGISel {
/// SelectInlineAsmMemoryOperand - Implement addressing mode selection for
/// inline asm expressions.
bool SelectInlineAsmMemoryOperand(const SDValue &Op,
unsigned ConstraintID,
InlineAsm::ConstraintCode ConstraintID,
std::vector<SDValue> &OutOps) override;

template <signed Low, signed High, signed Scale>
Expand Down Expand Up @@ -533,13 +533,14 @@ static bool isIntImmediateEq(SDValue N, const uint64_t ImmExpected) {
#endif

bool AArch64DAGToDAGISel::SelectInlineAsmMemoryOperand(
const SDValue &Op, unsigned ConstraintID, std::vector<SDValue> &OutOps) {
const SDValue &Op, const InlineAsm::ConstraintCode ConstraintID,
std::vector<SDValue> &OutOps) {
switch(ConstraintID) {
default:
llvm_unreachable("Unexpected asm memory constraint");
case InlineAsm::Constraint_m:
case InlineAsm::Constraint_o:
case InlineAsm::Constraint_Q:
case InlineAsm::ConstraintCode::m:
case InlineAsm::ConstraintCode::o:
case InlineAsm::ConstraintCode::Q:
// We need to make sure that this one operand does not end up in XZR, thus
// require the address to be in a PointerRegClass register.
const TargetRegisterInfo *TRI = Subtarget->getRegisterInfo();
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,9 +1169,10 @@ class AArch64TargetLowering : public TargetLowering {
std::vector<SDValue> &Ops,
SelectionDAG &DAG) const override;

unsigned getInlineAsmMemConstraint(StringRef ConstraintCode) const override {
InlineAsm::ConstraintCode
getInlineAsmMemConstraint(StringRef ConstraintCode) const override {
if (ConstraintCode == "Q")
return InlineAsm::Constraint_Q;
return InlineAsm::ConstraintCode::Q;
// FIXME: clang has code for 'Ump', 'Utf', 'Usa', and 'Ush' but these are
// followed by llvm_unreachable so we'll leave them unimplemented in
// the backend for now.
Expand Down
Loading