[AMDGPU][AsmParser] Simplify getting source locations of operands. #158323
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
Remember indexes of MCOperands in MCParsedAsmOperands as we add them to instructions. Then use the indexes to find locations by known MCOperands indexes. Happens to fix some reported locations in tests. NFCI otherwise. getImmLoc() is to be eliminated as well; there's enough work for another patch.
|
@llvm/pr-subscribers-backend-amdgpu Author: Ivan Kosarev (kosarev) ChangesRemember indexes of MCOperands in MCParsedAsmOperands as we add them to instructions. Then use the indexes to find locations by known MCOperands indexes. Happens to fix some reported locations in tests. NFCI otherwise. getImmLoc() is to be eliminated as well; there's enough work for another patch. Patch is 55.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/158323.diff 6 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index e420f2ad676f9..15d03403e3868 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -188,20 +188,6 @@ class AMDGPUOperand : public MCParsedAsmOperand {
ImmTyByteSel,
};
- // Immediate operand kind.
- // It helps to identify the location of an offending operand after an error.
- // Note that regular literals and mandatory literals (KImm) must be handled
- // differently. When looking for an offending operand, we should usually
- // ignore mandatory literals because they are part of the instruction and
- // cannot be changed. Report location of mandatory operands only for VOPD,
- // when both OpX and OpY have a KImm and there are no other literals.
- enum ImmKindTy {
- ImmKindTyNone,
- ImmKindTyLiteral,
- ImmKindTyMandatoryLiteral,
- ImmKindTyConst,
- };
-
private:
struct TokOp {
const char *Data;
@@ -212,7 +198,6 @@ class AMDGPUOperand : public MCParsedAsmOperand {
int64_t Val;
ImmTy Type;
bool IsFPImm;
- mutable ImmKindTy Kind;
Modifiers Mods;
};
@@ -228,6 +213,9 @@ class AMDGPUOperand : public MCParsedAsmOperand {
const MCExpr *Expr;
};
+ // The index of the associated MCInst operand.
+ mutable int MCOpIdx = -1;
+
public:
bool isToken() const override { return Kind == Token; }
@@ -239,38 +227,6 @@ class AMDGPUOperand : public MCParsedAsmOperand {
return Kind == Immediate;
}
- void setImmKindNone() const {
- assert(isImm());
- Imm.Kind = ImmKindTyNone;
- }
-
- void setImmKindLiteral() const {
- assert(isImm());
- Imm.Kind = ImmKindTyLiteral;
- }
-
- void setImmKindMandatoryLiteral() const {
- assert(isImm());
- Imm.Kind = ImmKindTyMandatoryLiteral;
- }
-
- void setImmKindConst() const {
- assert(isImm());
- Imm.Kind = ImmKindTyConst;
- }
-
- bool IsImmKindLiteral() const {
- return isImm() && Imm.Kind == ImmKindTyLiteral;
- }
-
- bool IsImmKindMandatoryLiteral() const {
- return isImm() && Imm.Kind == ImmKindTyMandatoryLiteral;
- }
-
- bool isImmKindConst() const {
- return isImm() && Imm.Kind == ImmKindTyConst;
- }
-
bool isInlinableImm(MVT type) const;
bool isLiteralImm(MVT type) const;
@@ -1055,6 +1011,8 @@ class AMDGPUOperand : public MCParsedAsmOperand {
return SMRange(StartLoc, EndLoc);
}
+ int getMCOpIdx() const { return MCOpIdx; }
+
Modifiers getModifiers() const {
assert(isRegKind() || isImmTy(ImmTyNone));
return isRegKind() ? Reg.Mods : Imm.Mods;
@@ -1242,7 +1200,6 @@ class AMDGPUOperand : public MCParsedAsmOperand {
auto Op = std::make_unique<AMDGPUOperand>(Immediate, AsmParser);
Op->Imm.Val = Val;
Op->Imm.IsFPImm = IsFPImm;
- Op->Imm.Kind = ImmKindTyNone;
Op->Imm.Type = Type;
Op->Imm.Mods = Modifiers();
Op->StartLoc = Loc;
@@ -1836,25 +1793,24 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
ParseStatus parseHwregFunc(OperandInfoTy &HwReg, OperandInfoTy &Offset,
OperandInfoTy &Width);
+ static SMLoc getLaterLoc(SMLoc a, SMLoc b);
+
SMLoc getFlatOffsetLoc(const OperandVector &Operands) const;
SMLoc getSMEMOffsetLoc(const OperandVector &Operands) const;
SMLoc getBLGPLoc(const OperandVector &Operands) const;
+ SMLoc getOperandLoc(const OperandVector &Operands, int MCOpIdx) const;
SMLoc getOperandLoc(std::function<bool(const AMDGPUOperand&)> Test,
const OperandVector &Operands) const;
- SMLoc getImmLoc(AMDGPUOperand::ImmTy Type, const OperandVector &Operands) const;
- SMLoc getRegLoc(MCRegister Reg, const OperandVector &Operands) const;
- SMLoc getLitLoc(const OperandVector &Operands,
- bool SearchMandatoryLiterals = false) const;
- SMLoc getMandatoryLitLoc(const OperandVector &Operands) const;
- SMLoc getConstLoc(const OperandVector &Operands) const;
+ SMLoc getImmLoc(AMDGPUOperand::ImmTy Type,
+ const OperandVector &Operands) const;
SMLoc getInstLoc(const OperandVector &Operands) const;
bool validateInstruction(const MCInst &Inst, const SMLoc &IDLoc, const OperandVector &Operands);
bool validateOffset(const MCInst &Inst, const OperandVector &Operands);
bool validateFlatOffset(const MCInst &Inst, const OperandVector &Operands);
bool validateSMEMOffset(const MCInst &Inst, const OperandVector &Operands);
- bool validateSOPLiteral(const MCInst &Inst) const;
+ bool validateSOPLiteral(const MCInst &Inst, const OperandVector &Operands);
bool validateConstantBusLimitations(const MCInst &Inst, const OperandVector &Operands);
std::optional<unsigned> checkVOPDRegBankConstraints(const MCInst &Inst,
bool AsVOPD3);
@@ -1895,7 +1851,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
const unsigned CPol);
bool validateTFE(const MCInst &Inst, const OperandVector &Operands);
bool validateSetVgprMSB(const MCInst &Inst, const OperandVector &Operands);
- std::optional<StringRef> validateLdsDirect(const MCInst &Inst);
+ bool validateLdsDirect(const MCInst &Inst, const OperandVector &Operands);
bool validateWMMA(const MCInst &Inst, const OperandVector &Operands);
unsigned getConstantBusLimit(unsigned Opcode) const;
bool usesConstantBus(const MCInst &Inst, unsigned OpIdx);
@@ -2337,6 +2293,8 @@ uint64_t AMDGPUOperand::applyInputFPModifiers(uint64_t Val, unsigned Size) const
}
void AMDGPUOperand::addImmOperands(MCInst &Inst, unsigned N, bool ApplyModifiers) const {
+ MCOpIdx = Inst.getNumOperands();
+
if (isExpr()) {
Inst.addOperand(MCOperand::createExpr(Expr));
return;
@@ -2350,7 +2308,6 @@ void AMDGPUOperand::addImmOperands(MCInst &Inst, unsigned N, bool ApplyModifiers
} else {
assert(!isImmTy(ImmTyNone) || !hasModifiers());
Inst.addOperand(MCOperand::createImm(Imm.Val));
- setImmKindNone();
}
}
@@ -2379,7 +2336,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
if (AMDGPU::isInlinableLiteral64(Literal.getZExtValue(),
AsmParser->hasInv2PiInlineImm())) {
Inst.addOperand(MCOperand::createImm(Literal.getZExtValue()));
- setImmKindConst();
return;
}
@@ -2400,7 +2356,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
}
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindLiteral();
return;
}
@@ -2411,7 +2366,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
case AMDGPU::OPERAND_KIMM64:
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindMandatoryLiteral();
return;
case AMDGPU::OPERAND_REG_IMM_BF16:
@@ -2424,7 +2378,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
// 1/(2*pi) = 0.15915494 since bf16 is in fact fp32 with cleared low 16
// bits. Prevent rounding below.
Inst.addOperand(MCOperand::createImm(0x3e22));
- setImmKindLiteral();
return;
}
[[fallthrough]];
@@ -2459,11 +2412,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
uint64_t ImmVal = FPLiteral.bitcastToAPInt().getZExtValue();
Inst.addOperand(MCOperand::createImm(ImmVal));
- if (OpTy == AMDGPU::OPERAND_KIMM32 || OpTy == AMDGPU::OPERAND_KIMM16) {
- setImmKindMandatoryLiteral();
- } else {
- setImmKindLiteral();
- }
return;
}
default:
@@ -2492,7 +2440,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
AMDGPU::isInlinableLiteral32(static_cast<int32_t>(Val),
AsmParser->hasInv2PiInlineImm())) {
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindConst();
return;
}
[[fallthrough]];
@@ -2500,14 +2447,12 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
case AMDGPU::OPERAND_REG_IMM_NOINLINE_V2FP16:
Inst.addOperand(MCOperand::createImm(Lo_32(Val)));
- setImmKindLiteral();
return;
case AMDGPU::OPERAND_REG_IMM_INT64:
case AMDGPU::OPERAND_REG_INLINE_C_INT64:
if (AMDGPU::isInlinableLiteral64(Val, AsmParser->hasInv2PiInlineImm())) {
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindConst();
return;
}
@@ -2519,7 +2464,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
Val = Lo_32(Val);
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindLiteral();
return;
case AMDGPU::OPERAND_REG_IMM_FP64:
@@ -2527,7 +2471,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
case AMDGPU::OPERAND_REG_INLINE_AC_FP64:
if (AMDGPU::isInlinableLiteral64(Val, AsmParser->hasInv2PiInlineImm())) {
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindConst();
return;
}
@@ -2547,7 +2490,6 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
}
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindLiteral();
return;
case AMDGPU::OPERAND_REG_IMM_INT16:
@@ -2555,12 +2497,10 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
if (isSafeTruncation(Val, 16) &&
AMDGPU::isInlinableIntLiteral(static_cast<int16_t>(Val))) {
Inst.addOperand(MCOperand::createImm(Lo_32(Val)));
- setImmKindConst();
return;
}
Inst.addOperand(MCOperand::createImm(Val & 0xffff));
- setImmKindLiteral();
return;
case AMDGPU::OPERAND_REG_INLINE_C_FP16:
@@ -2569,12 +2509,10 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
AMDGPU::isInlinableLiteralFP16(static_cast<int16_t>(Val),
AsmParser->hasInv2PiInlineImm())) {
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindConst();
return;
}
Inst.addOperand(MCOperand::createImm(Val & 0xffff));
- setImmKindLiteral();
return;
case AMDGPU::OPERAND_REG_IMM_BF16:
@@ -2583,12 +2521,10 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
AMDGPU::isInlinableLiteralBF16(static_cast<int16_t>(Val),
AsmParser->hasInv2PiInlineImm())) {
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindConst();
return;
}
Inst.addOperand(MCOperand::createImm(Val & 0xffff));
- setImmKindLiteral();
return;
case AMDGPU::OPERAND_REG_INLINE_C_V2INT16: {
@@ -2617,18 +2553,15 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
case AMDGPU::OPERAND_KIMM32:
Inst.addOperand(MCOperand::createImm(Literal.getLoBits(32).getZExtValue()));
- setImmKindMandatoryLiteral();
return;
case AMDGPU::OPERAND_KIMM16:
Inst.addOperand(MCOperand::createImm(Literal.getLoBits(16).getZExtValue()));
- setImmKindMandatoryLiteral();
return;
case AMDGPU::OPERAND_KIMM64:
if ((isInt<32>(Val) || isUInt<32>(Val)) && !getModifiers().Lit64)
Val <<= 32;
Inst.addOperand(MCOperand::createImm(Val));
- setImmKindMandatoryLiteral();
return;
default:
llvm_unreachable("invalid operand size");
@@ -2636,6 +2569,7 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
}
void AMDGPUOperand::addRegOperands(MCInst &Inst, unsigned N) const {
+ MCOpIdx = Inst.getNumOperands();
Inst.addOperand(MCOperand::createReg(AMDGPU::getMCReg(getReg(), AsmParser->getSTI())));
}
@@ -3942,6 +3876,8 @@ bool AMDGPUAsmParser::validateConstantBusLimitations(
OperandIndices OpIndices = getSrcOperandIndices(Opcode);
+ unsigned ConstantBusLimit = getConstantBusLimit(Opcode);
+
for (int OpIdx : OpIndices) {
if (OpIdx == -1)
continue;
@@ -3985,17 +3921,14 @@ bool AMDGPUAsmParser::validateConstantBusLimitations(
}
}
}
- }
- ConstantBusUseCount += NumLiterals;
- if (ConstantBusUseCount <= getConstantBusLimit(Opcode))
- return true;
-
- SMLoc LitLoc = getLitLoc(Operands);
- SMLoc RegLoc = getRegLoc(LastSGPR, Operands);
- SMLoc Loc = (LitLoc.getPointer() < RegLoc.getPointer()) ? RegLoc : LitLoc;
- Error(Loc, "invalid operand (violates constant bus restrictions)");
- return false;
+ if (ConstantBusUseCount + NumLiterals > ConstantBusLimit) {
+ Error(getOperandLoc(Operands, OpIdx),
+ "invalid operand (violates constant bus restrictions)");
+ return false;
+ }
+ }
+ return true;
}
std::optional<unsigned>
@@ -4408,19 +4341,15 @@ bool AMDGPUAsmParser::validateMovrels(const MCInst &Inst,
const int Src0Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::src0);
assert(Src0Idx != -1);
- SMLoc ErrLoc;
const MCOperand &Src0 = Inst.getOperand(Src0Idx);
if (Src0.isReg()) {
auto Reg = mc2PseudoReg(Src0.getReg());
const MCRegisterInfo *TRI = getContext().getRegisterInfo();
if (!isSGPR(Reg, TRI))
return true;
- ErrLoc = getRegLoc(Reg, Operands);
- } else {
- ErrLoc = getConstLoc(Operands);
}
- Error(ErrLoc, "source operand must be a VGPR");
+ Error(getOperandLoc(Operands, Src0Idx), "source operand must be a VGPR");
return false;
}
@@ -4442,7 +4371,7 @@ bool AMDGPUAsmParser::validateMAIAccWrite(const MCInst &Inst,
auto Reg = mc2PseudoReg(Src0.getReg());
const MCRegisterInfo *TRI = getContext().getRegisterInfo();
if (!isGFX90A() && isSGPR(Reg, TRI)) {
- Error(getRegLoc(Reg, Operands),
+ Error(getOperandLoc(Operands, Src0Idx),
"source operand must be either a VGPR or an inline constant");
return false;
}
@@ -4464,7 +4393,7 @@ bool AMDGPUAsmParser::validateMAISrc2(const MCInst &Inst,
return true;
if (Inst.getOperand(Src2Idx).isImm() && isInlineConstant(Inst, Src2Idx)) {
- Error(getConstLoc(Operands),
+ Error(getOperandLoc(Operands, Src2Idx),
"inline constants are not allowed for this operand");
return false;
}
@@ -4494,16 +4423,14 @@ bool AMDGPUAsmParser::validateMFMA(const MCInst &Inst,
bool Success = true;
if (Info->NumRegsSrcA != mfmaScaleF8F6F4FormatToNumRegs(CBSZ)) {
int Src0Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::src0);
- Error(getRegLoc(mc2PseudoReg(Inst.getOperand(Src0Idx).getReg()),
- Operands),
+ Error(getOperandLoc(Operands, Src0Idx),
"wrong register tuple size for cbsz value " + Twine(CBSZ));
Success = false;
}
if (Info->NumRegsSrcB != mfmaScaleF8F6F4FormatToNumRegs(BLGP)) {
int Src1Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::src1);
- Error(getRegLoc(mc2PseudoReg(Inst.getOperand(Src1Idx).getReg()),
- Operands),
+ Error(getOperandLoc(Operands, Src1Idx),
"wrong register tuple size for blgp value " + Twine(BLGP));
Success = false;
}
@@ -4530,7 +4457,7 @@ bool AMDGPUAsmParser::validateMFMA(const MCInst &Inst,
return true;
if (TRI->regsOverlap(Src2Reg, DstReg)) {
- Error(getRegLoc(mc2PseudoReg(Src2Reg), Operands),
+ Error(getOperandLoc(Operands, Src2Idx),
"source 2 operand must not partially overlap with dst");
return false;
}
@@ -4724,9 +4651,8 @@ static bool IsRevOpcode(const unsigned Opcode)
}
}
-std::optional<StringRef>
-AMDGPUAsmParser::validateLdsDirect(const MCInst &Inst) {
-
+bool AMDGPUAsmParser::validateLdsDirect(const MCInst &Inst,
+ const OperandVector &Operands) {
using namespace SIInstrFlags;
const unsigned Opcode = Inst.getOpcode();
const MCInstrDesc &Desc = MII.get(Opcode);
@@ -4735,7 +4661,7 @@ AMDGPUAsmParser::validateLdsDirect(const MCInst &Inst) {
// with 9-bit operands only. Ignore encodings which do not accept these.
const auto Enc = VOP1 | VOP2 | VOP3 | VOPC | VOP3P | SIInstrFlags::SDWA;
if ((Desc.TSFlags & Enc) == 0)
- return std::nullopt;
+ return true;
for (auto SrcName : {OpName::src0, OpName::src1, OpName::src2}) {
auto SrcIdx = getNamedOperandIdx(Opcode, SrcName);
@@ -4744,18 +4670,27 @@ AMDGPUAsmParser::validateLdsDirect(const MCInst &Inst) {
const auto &Src = Inst.getOperand(SrcIdx);
if (Src.isReg() && Src.getReg() == LDS_DIRECT) {
- if (isGFX90A() || isGFX11Plus())
- return StringRef("lds_direct is not supported on this GPU");
+ if (isGFX90A() || isGFX11Plus()) {
+ Error(getOperandLoc(Operands, SrcIdx),
+ "lds_direct is not supported on this GPU");
+ return false;
+ }
- if (IsRevOpcode(Opcode) || (Desc.TSFlags & SIInstrFlags::SDWA))
- return StringRef("lds_direct cannot be used with this instruction");
+ if (IsRevOpcode(Opcode) || (Desc.TSFlags & SIInstrFlags::SDWA)) {
+ Error(getOperandLoc(Operands, SrcIdx),
+ "lds_direct cannot be used with this instruction");
+ return false;
+ }
- if (SrcName != OpName::src0)
- return StringRef("lds_direct may be used as src0 only");
+ if (SrcName != OpName::src0) {
+ Error(getOperandLoc(Operands, SrcIdx),
+ "lds_direct may be used as src0 only");
+ return false;
+ }
}
}
- return std::nullopt;
+ return true;
}
SMLoc AMDGPUAsmParser::getFlatOffsetLoc(const OperandVector &Operands) const {
@@ -4881,7 +4816,8 @@ bool AMDGPUAsmParser::validateSMEMOffset(const MCInst &Inst,
return false;
}
-bool AMDGPUAsmParser::validateSOPLiteral(const MCInst &Inst) const {
+bool AMDGPUAsmParser::validateSOPLiteral(const MCInst &Inst,
+ const OperandVector &Operands) {
unsigned Opcode = Inst.getOpcode();
const MCInstrDesc &Desc = MII.get(Opcode);
if (!(Desc.TSFlags & (SIInstrFlags::SOP2 | SIInstrFlags::SOPC)))
@@ -4914,7 +4850,12 @@ bool AMDGPUAsmParser::validateSOPLiteral(const MCInst &Inst) const {
}
}
- return NumLiterals + NumExprs <= 1;
+ if (NumLiterals + NumExprs <= 1)
+ return true;
+
+ Error(getOperandLoc(Operands, Src1Idx),
+ "only one unique literal operand is allowed");
+ return false;
}
bool AMDGPUAsmParser::validateOpSel(const MCInst &Inst) {
@@ -5090,9 +5031,8 @@ bool AMDGPUAsmParser::validateDPP(const MCInst &Inst,
const MCOperand &Src1 = Inst.getOperand(Src1Idx);
const MCRegisterInfo *TRI = getContext().getRegisterInfo();
if (Src1.isReg() && isSGPR(mc2PseudoReg(Src1.getReg()), TRI)) {
- auto Reg = mc2PseudoReg(Inst.getOperand(Src1Idx).getReg());
- SMLoc S = getRegLoc(Reg, Operands);
- Error(S, "invalid operand for instruction");
+ Error(getOperandLoc(Operands, Src1Idx),
+ "invalid operand for instruction");
return false;
}
if (Src1.isImm()) {
@@ -5125,9 +5065,8 @@ bool AMDGPUAsmParser::validateVOPLiteral(const MCInst &Inst,
OperandIndices OpIndices = getSrcOperandIndices(Opcode, HasMandatoryLiteral);
- unsigned NumExprs = 0;
- unsigned NumLiterals = 0;
- uint64_t LiteralValue;
+ std::optional<unsigned> LiteralOpIdx;
+ std::optional<uint64_t> LiteralValue;
for (int OpIdx : OpIndices) {
if (OpIdx == -1)
@@ -5139,6 +5078,7 @@ bool AMDGPUAsmParser::validateVOPLiteral(const MCInst &Inst,
if (!isSISrcOperand(Desc, OpIdx))
continue;
+ bool IsAnotherLiteral = false;
if (MO.isImm() && !isInlineConstant(Inst, OpIdx)) {
uint64_t Value = static_cast<uint64_t>(MO.getImm());
bool IsForcedFP64 =
@@ -5151,34 +5091,37 @@ bool AMDGPUAsmParser::validateVOPLiteral(const MCInst &Inst,
if (!IsValid32Op && !isInt<32>(Value) && !isUInt<32>(Value) &&
!IsForcedFP64 && (!has64BitLiterals() || Desc.getSize() != 4)) {
- Error(getLitLoc(Operands), "invalid operand for instruction");
+ Error(getOperandLoc(Operands, OpIdx),
+ "invalid operand for instruction");
return false;
}
if (IsFP64 && IsValid32Op && !IsForcedFP64)
Value = Hi_32(Value);
- if ...
[truncated]
|
kosarev
requested review from
jayfoad,
arsenm,
Sisyph,
rampitec and
mbrkusanin
rampitec
reviewed
Sep 12, 2025
| }; | ||
|
|
||
| // The index of the associated MCInst operand. | ||
| mutable int MCOpIdx = -1; |
There was a problem hiding this comment.
Would take all the AddImm*() / AddReg*() members to lose their const qualifiers. Alternatively, since AMDGPUOperands store a reference to the AsmParser, we could store the mapping there, which looks more complicated than necessary and then again requires dealing with the const-ness of that reference. Wasn't able to convince myself any of the options are worth it.
Note that the removed ImmKindTy Kind field was also declared mutable, so the patch doesn't make it worse.
| ParseStatus parseHwregFunc(OperandInfoTy &HwReg, OperandInfoTy &Offset, | ||
| OperandInfoTy &Width); | ||
|
|
||
| static SMLoc getLaterLoc(SMLoc a, SMLoc b); |
There was a problem hiding this comment.
Needs a comment, it is not clear what it does until you see the body. Or just inline the body.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
kosarev
force-pushed
the
asm-op-locs
branch
from
5e9195f to
0747d07
Compare
rampitec
approved these changes
Sep 15, 2025
| }; | ||
|
|
||
| // The index of the associated MCInst operand. | ||
| mutable int MCOpIdx = -1; |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/31128 Here is the relevant piece of the build log for the reference |
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.
Remember indexes of MCOperands in MCParsedAsmOperands as we add them to instructions. Then use the indexes to find locations by known MCOperands indexes.
Happens to fix some reported locations in tests. NFCI otherwise.
getImmLoc() is to be eliminated as well; there's enough work for another patch.