Skip to content

[CodeGen] Enhance inline asm constraint diagnostics #101354

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion llvm/include/llvm/CodeGen/TargetLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -5034,7 +5034,8 @@ class TargetLowering : public TargetLoweringBase {
/// returns a register number of 0 and a null register class pointer.
virtual std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe SmallStringImpl&?

I don't really see how you can do much better than that; an error message is, ultimately, just a string, given we don't support translation or anything like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Error or StringError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea of SmallString. Error and StringError, they consist of std::string and additionally have a return code that we don't have use of.

Ok, since there aren't strong points for using something like Expected, I'll continue with returning an error message by reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Out argument string message is a worse API. I would prefer to use one of the proper return with bundled error message options


virtual InlineAsm::ConstraintCode
getInlineAsmMemConstraint(StringRef ConstraintCode) const {
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,9 @@ static void getRegistersForValue(MachineFunction &MF,
// register class, find it.
Register AssignedReg;
const TargetRegisterClass *RC;
std::string ErrMsg;
std::tie(AssignedReg, RC) = TLI.getRegForInlineAsmConstraint(
&TRI, RefOpInfo.ConstraintCode, RefOpInfo.ConstraintVT);
&TRI, RefOpInfo.ConstraintCode, RefOpInfo.ConstraintVT, ErrMsg);
// RC is unset only on failure. Return immediately.
if (!RC)
return;
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,10 @@ void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
if (Op.Type == InlineAsm::isClobber) {
// Clobbers don't have SDValue operands, hence SDValue().
TLI->ComputeConstraintToUse(Op, SDValue(), DAG);
std::string ErrMsg;
std::pair<unsigned, const TargetRegisterClass *> PhysReg =
TLI->getRegForInlineAsmConstraint(TRI, Op.ConstraintCode,
Op.ConstraintVT);
Op.ConstraintVT, ErrMsg);
if (PhysReg.first == SP)
MF->getFrameInfo().setHasOpaqueSPAdjustment(true);
}
Expand Down
56 changes: 30 additions & 26 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9516,13 +9516,14 @@ static void patchMatchingInput(const SDISelAsmOperandInfo &OpInfo,

const TargetRegisterInfo *TRI = DAG.getSubtarget().getRegisterInfo();
const auto &TLI = DAG.getTargetLoweringInfo();
std::string ErrMsg;

std::pair<unsigned, const TargetRegisterClass *> MatchRC =
TLI.getRegForInlineAsmConstraint(TRI, OpInfo.ConstraintCode,
OpInfo.ConstraintVT);
OpInfo.ConstraintVT, ErrMsg);
std::pair<unsigned, const TargetRegisterClass *> InputRC =
TLI.getRegForInlineAsmConstraint(TRI, MatchingOpInfo.ConstraintCode,
MatchingOpInfo.ConstraintVT);
MatchingOpInfo.ConstraintVT, ErrMsg);
if ((OpInfo.ConstraintVT.isInteger() !=
MatchingOpInfo.ConstraintVT.isInteger()) ||
(MatchRC.second != InputRC.second)) {
Expand Down Expand Up @@ -9588,7 +9589,7 @@ static SDValue getAddressForMemoryInput(SDValue Chain, const SDLoc &Location,
///
/// OpInfo describes the operand
/// RefOpInfo describes the matching operand if any, the operand otherwise
static std::optional<unsigned>
static std::optional<std::string>
getRegistersForValue(SelectionDAG &DAG, const SDLoc &DL,
SDISelAsmOperandInfo &OpInfo,
SDISelAsmOperandInfo &RefOpInfo) {
Expand All @@ -9608,11 +9609,12 @@ getRegistersForValue(SelectionDAG &DAG, const SDLoc &DL,
// register class, find it.
unsigned AssignedReg;
const TargetRegisterClass *RC;
std::string ErrMsg;
std::tie(AssignedReg, RC) = TLI.getRegForInlineAsmConstraint(
&TRI, RefOpInfo.ConstraintCode, RefOpInfo.ConstraintVT);
&TRI, RefOpInfo.ConstraintCode, RefOpInfo.ConstraintVT, ErrMsg);
// RC is unset only on failure. Return immediately.
if (!RC)
return std::nullopt;
return ErrMsg;

// Get the actual register value type. This is important, because the user
// may have asked for (e.g.) the AX register in i32 type. We need to
Expand Down Expand Up @@ -9684,7 +9686,9 @@ getRegistersForValue(SelectionDAG &DAG, const SDLoc &DL,
if (I == RC->end()) {
// RC does not contain the selected register, which indicates a
// mismatch between the register and the required type/bitwidth.
return {AssignedReg};
return "register '" + std::string(TRI.getName(AssignedReg)) +
"' allocated for constraint '" + OpInfo.ConstraintCode +
"' does not match required type " + ValueVT.getEVTString();
}
}

Expand Down Expand Up @@ -9933,18 +9937,6 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
OpInfo.isMatchingInputConstraint()
? ConstraintOperands[OpInfo.getMatchedOperand()]
: OpInfo;
const auto RegError =
getRegistersForValue(DAG, getCurSDLoc(), OpInfo, RefOpInfo);
if (RegError) {
const MachineFunction &MF = DAG.getMachineFunction();
const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
const char *RegName = TRI.getName(*RegError);
emitInlineAsmError(Call, "register '" + Twine(RegName) +
"' allocated for constraint '" +
Twine(OpInfo.ConstraintCode) +
"' does not match required type");
return;
}

auto DetectWriteToReservedRegister = [&]() {
const MachineFunction &MF = DAG.getMachineFunction();
Expand Down Expand Up @@ -9983,12 +9975,17 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
// Otherwise, this outputs to a register (directly for C_Register /
// C_RegisterClass, and a target-defined fashion for
// C_Immediate/C_Other). Find a register that we can use.
if (OpInfo.AssignedRegs.Regs.empty()) {
const auto RegError =
getRegistersForValue(DAG, getCurSDLoc(), OpInfo, RefOpInfo);
if (RegError) {
emitInlineAsmError(
Call, "couldn't allocate output register for constraint '" +
Twine(OpInfo.ConstraintCode) + "'");
Twine(OpInfo.ConstraintCode) +
(RegError->empty() ? "'" : ("': " + Twine(*RegError))));
return;
}
assert(!OpInfo.AssignedRegs.Regs.empty() &&
"register must be allocated or an error emitted");

if (DetectWriteToReservedRegister())
return;
Expand Down Expand Up @@ -10157,13 +10154,17 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
return;
}

// Copy the input into the appropriate registers.
if (OpInfo.AssignedRegs.Regs.empty()) {
emitInlineAsmError(Call,
"couldn't allocate input reg for constraint '" +
Twine(OpInfo.ConstraintCode) + "'");
const auto RegError =
getRegistersForValue(DAG, getCurSDLoc(), OpInfo, RefOpInfo);
if (RegError) {
emitInlineAsmError(
Call, "couldn't allocate input reg for constraint '" +
Twine(OpInfo.ConstraintCode) +
(RegError->empty() ? "'" : ("': " + Twine(*RegError))));
return;
}
assert(!OpInfo.AssignedRegs.Regs.empty() &&
"register must be allocated or an error emitted");

if (DetectWriteToReservedRegister())
return;
Expand All @@ -10177,15 +10178,18 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
0, dl, DAG, AsmNodeOperands);
break;
}
case InlineAsm::isClobber:
case InlineAsm::isClobber: {
// Add the clobbered value to the operand list, so that the register
// allocator is aware that the physreg got clobbered.
const auto RegError =
getRegistersForValue(DAG, getCurSDLoc(), OpInfo, RefOpInfo);
if (!OpInfo.AssignedRegs.Regs.empty())
OpInfo.AssignedRegs.AddInlineAsmOperands(InlineAsm::Kind::Clobber,
false, 0, getCurSDLoc(), DAG,
AsmNodeOperands);
break;
}
}
}

// Finish up input operands. Set the input chain and add the flag last.
Expand Down
11 changes: 7 additions & 4 deletions llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5620,8 +5620,8 @@ void TargetLowering::CollectTargetIntrinsicOperands(

std::pair<unsigned, const TargetRegisterClass *>
TargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *RI,
StringRef Constraint,
MVT VT) const {
StringRef Constraint, MVT VT,
std::string &ErrMsg) const {
if (!Constraint.starts_with("{"))
return std::make_pair(0u, static_cast<TargetRegisterClass *>(nullptr));
assert(*(Constraint.end() - 1) == '}' && "Not a brace enclosed constraint?");
Expand Down Expand Up @@ -5654,6 +5654,8 @@ TargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *RI,
}
}
}
if (!R.second)
ErrMsg = "register is unavailable";

return R;
}
Expand Down Expand Up @@ -5842,12 +5844,13 @@ TargetLowering::ParseConstraints(const DataLayout &DL,
AsmOperandInfo &Input = ConstraintOperands[OpInfo.MatchingInput];

if (OpInfo.ConstraintVT != Input.ConstraintVT) {
std::string ErrMsg;
std::pair<unsigned, const TargetRegisterClass *> MatchRC =
getRegForInlineAsmConstraint(TRI, OpInfo.ConstraintCode,
OpInfo.ConstraintVT);
OpInfo.ConstraintVT, ErrMsg);
std::pair<unsigned, const TargetRegisterClass *> InputRC =
getRegForInlineAsmConstraint(TRI, Input.ConstraintCode,
Input.ConstraintVT);
Input.ConstraintVT, ErrMsg);
if ((OpInfo.ConstraintVT.isInteger() !=
Input.ConstraintVT.isInteger()) ||
(MatchRC.second != InputRC.second)) {
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AArch64/AArch64ISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,8 @@ class AArch64TargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

const char *LowerXConstraint(EVT ConstraintVT) const override;

Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AMDGPU/SIISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@ class SITargetLowering final : public AMDGPUTargetLowering {
uint32_t RsrcDword1, uint64_t RsrcDword2And3) const;
std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;
ConstraintType getConstraintType(StringRef Constraint) const override;
void LowerAsmOperandForConstraint(SDValue Op, StringRef Constraint,
std::vector<SDValue> &Ops,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/ARM/ARMISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,8 @@ class VectorType;

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

const char *LowerXConstraint(EVT ConstraintVT) const override;

Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AVR/AVRISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ class AVRTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

InlineAsm::ConstraintCode
getInlineAsmMemConstraint(StringRef ConstraintCode) const override;
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/BPF/BPFISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class BPFTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

MachineBasicBlock *
EmitInstrWithCustomInserter(MachineInstr &MI,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/CSKY/CSKYISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ class CSKYTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

MachineBasicBlock *
EmitInstrWithCustomInserter(MachineInstr &MI,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/Hexagon/HexagonISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ class HexagonTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

// Intrinsics
SDValue LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG &DAG) const;
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/Lanai/LanaiISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ class LanaiTargetLowering : public TargetLowering {
const MachineFunction &MF) const override;
std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;
ConstraintWeight
getSingleConstraintMatchWeight(AsmOperandInfo &Info,
const char *Constraint) const override;
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/LoongArch/LoongArchISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,8 @@ class LoongArchTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

void LowerAsmOperandForConstraint(SDValue Op, StringRef Constraint,
std::vector<SDValue> &Ops,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/M68k/M68kISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ class M68kTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

// Lower operand with C_Immediate and C_Other constraint type
void LowerAsmOperandForConstraint(SDValue Op, StringRef Constraint,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/MSP430/MSP430ISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ namespace llvm {
getConstraintType(StringRef Constraint) const override;
std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

/// isTruncateFree - Return true if it's free to truncate a value of type
/// Ty1 to type Ty2. e.g. On msp430 it's free to truncate a i16 value in
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/Mips/MipsISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,8 @@ class TargetRegisterClass;

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

/// LowerAsmOperandForConstraint - Lower the specified operand into the Ops
/// vector. If it is invalid, don't add anything to Ops. If hasMemory is
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/NVPTX/NVPTXISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,8 @@ class NVPTXTargetLowering : public TargetLowering {
ConstraintType getConstraintType(StringRef Constraint) const override;
std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

SDValue LowerFormalArguments(SDValue Chain, CallingConv::ID CallConv,
bool isVarArg,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/PowerPC/PPCISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,8 @@ namespace llvm {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

/// getByValTypeAlignment - Return the desired alignment for ByVal aggregate
/// function arguments in the caller parameter area. This is the actual
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/RISCV/RISCVISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,8 @@ class RISCVTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

void LowerAsmOperandForConstraint(SDValue Op, StringRef Constraint,
std::vector<SDValue> &Ops,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/SPIRV/SPIRVISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ class SPIRVTargetLowering : public TargetLowering {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;
unsigned
getNumRegisters(LLVMContext &Context, EVT VT,
std::optional<MVT> RegisterVT = std::nullopt) const override {
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/Sparc/SparcISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ namespace llvm {

std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

bool isOffsetFoldingLegal(const GlobalAddressSDNode *GA) const override;
MVT getScalarShiftAmountTy(const DataLayout &, EVT) const override {
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/SystemZ/SystemZISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,8 @@ class SystemZTargetLowering : public TargetLowering {
const char *getTargetNodeName(unsigned Opcode) const override;
std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;
TargetLowering::ConstraintType
getConstraintType(StringRef Constraint) const override;
TargetLowering::ConstraintWeight
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/VE/VEISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,8 @@ class VETargetLowering : public TargetLowering {
ConstraintType getConstraintType(StringRef Constraint) const override;
std::pair<unsigned, const TargetRegisterClass *>
getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
StringRef Constraint, MVT VT) const override;
StringRef Constraint, MVT VT,
std::string &ErrMsg) const override;

/// } Inline Assembly

Expand Down
Loading
Loading