Skip to content

Commit 884f2ad

Browse files
arsenmarsenm
authored
DiagnosticInfo: Clean up usage of DiagnosticInfoInlineAsm (#119485)
Currently LLVMContext::emitError emits any error as an "inline asm" error which does not make any sense. InlineAsm appears to be special, in that it uses a "LocCookie" from srcloc metadata, which looks like a parallel mechanism to ordinary source line locations. This meant that other types of failures had degraded source information reported when available. Introduce some new generic error types, and only use inline asm in the appropriate contexts. The DiagnosticInfo types are still a bit of a mess, and I'm not sure why DiagnosticInfoWithLocationBase exists instead of just having an optional DiagnosticLocation in the base class. DK_Generic is for any error that derives from an IR level instruction, and thus can pull debug locations directly from it. DK_GenericWithLoc is functionally the generic codegen error, since it does not depend on the IR and instead can construct a DiagnosticLocation from the MI debug location.
1 parent 2fe30bc commit 884f2ad

15 files changed

+167
-82
lines changed

‎llvm/include/llvm/CodeGen/MachineInstr.h‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -555,13 +555,18 @@ class MachineInstr
555555
/// will be dropped.
556556
void dropDebugNumber() { DebugInstrNum = 0; }
557557

558-
/// Emit an error referring to the source location of this instruction.
559-
/// This should only be used for inline assembly that is somehow
560-
/// impossible to compile. Other errors should have been handled much
561-
/// earlier.
562-
///
563-
/// If this method returns, the caller should try to recover from the error.
564-
void emitError(StringRef Msg) const;
558+
/// For inline asm, get the !srcloc metadata node if we have it, and decode
559+
/// the loc cookie from it.
560+
const MDNode *getLocCookieMD() const;
561+
562+
/// Emit an error referring to the source location of this instruction. This
563+
/// should only be used for inline assembly that is somehow impossible to
564+
/// compile. Other errors should have been handled much earlier.
565+
void emitInlineAsmError(const Twine &ErrMsg) const;
566+
567+
// Emit an error in the LLVMContext referring to the source location of this
568+
// instruction, if available.
569+
void emitGenericError(const Twine &ErrMsg) const;
565570

566571
/// Returns the target instruction descriptor of this MachineInstr.
567572
const MCInstrDesc &getDesc() const { return *MCID; }

‎llvm/include/llvm/IR/DiagnosticInfo.h‎

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ enum DiagnosticSeverity : char {
5858
/// Defines the different supported kind of a diagnostic.
5959
/// This enum should be extended with a new ID for each added concrete subclass.
6060
enum DiagnosticKind {
61+
DK_Generic,
62+
DK_GenericWithLoc,
6163
DK_InlineAsm,
6264
DK_ResourceLimit,
6365
DK_StackSize,
@@ -134,6 +136,33 @@ class DiagnosticInfo {
134136

135137
using DiagnosticHandlerFunction = std::function<void(const DiagnosticInfo &)>;
136138

139+
class DiagnosticInfoGeneric : public DiagnosticInfo {
140+
const Twine &MsgStr;
141+
const Instruction *Inst = nullptr;
142+
143+
public:
144+
/// \p MsgStr is the message to be reported to the frontend.
145+
/// This class does not copy \p MsgStr, therefore the reference must be valid
146+
/// for the whole life time of the Diagnostic.
147+
DiagnosticInfoGeneric(const Twine &MsgStr,
148+
DiagnosticSeverity Severity = DS_Error)
149+
: DiagnosticInfo(DK_Generic, Severity), MsgStr(MsgStr) {}
150+
151+
DiagnosticInfoGeneric(const Instruction *I, const Twine &ErrMsg,
152+
DiagnosticSeverity Severity = DS_Warning)
153+
: DiagnosticInfo(DK_Generic, Severity), MsgStr(ErrMsg), Inst(I) {}
154+
155+
const Twine &getMsgStr() const { return MsgStr; }
156+
const Instruction *getInstruction() const { return Inst; }
157+
158+
/// \see DiagnosticInfo::print.
159+
void print(DiagnosticPrinter &DP) const override;
160+
161+
static bool classof(const DiagnosticInfo *DI) {
162+
return DI->getKind() == DK_Generic;
163+
}
164+
};
165+
137166
/// Diagnostic information for inline asm reporting.
138167
/// This is basically a message and an optional location.
139168
class DiagnosticInfoInlineAsm : public DiagnosticInfo {
@@ -146,21 +175,12 @@ class DiagnosticInfoInlineAsm : public DiagnosticInfo {
146175
const Instruction *Instr = nullptr;
147176

148177
public:
149-
/// \p MsgStr is the message to be reported to the frontend.
150-
/// This class does not copy \p MsgStr, therefore the reference must be valid
151-
/// for the whole life time of the Diagnostic.
152-
DiagnosticInfoInlineAsm(const Twine &MsgStr,
153-
DiagnosticSeverity Severity = DS_Error)
154-
: DiagnosticInfo(DK_InlineAsm, Severity), MsgStr(MsgStr) {}
155-
156178
/// \p LocCookie if non-zero gives the line number for this report.
157179
/// \p MsgStr gives the message.
158180
/// This class does not copy \p MsgStr, therefore the reference must be valid
159181
/// for the whole life time of the Diagnostic.
160182
DiagnosticInfoInlineAsm(uint64_t LocCookie, const Twine &MsgStr,
161-
DiagnosticSeverity Severity = DS_Error)
162-
: DiagnosticInfo(DK_InlineAsm, Severity), LocCookie(LocCookie),
163-
MsgStr(MsgStr) {}
183+
DiagnosticSeverity Severity = DS_Error);
164184

165185
/// \p Instr gives the original instruction that triggered the diagnostic.
166186
/// \p MsgStr gives the message.
@@ -354,6 +374,31 @@ class DiagnosticInfoWithLocationBase : public DiagnosticInfo {
354374
DiagnosticLocation Loc;
355375
};
356376

377+
class DiagnosticInfoGenericWithLoc : public DiagnosticInfoWithLocationBase {
378+
private:
379+
/// Message to be reported.
380+
const Twine &MsgStr;
381+
382+
public:
383+
/// \p MsgStr is the message to be reported to the frontend.
384+
/// This class does not copy \p MsgStr, therefore the reference must be valid
385+
/// for the whole life time of the Diagnostic.
386+
DiagnosticInfoGenericWithLoc(const Twine &MsgStr, const Function &Fn,
387+
const DiagnosticLocation &Loc,
388+
DiagnosticSeverity Severity = DS_Error)
389+
: DiagnosticInfoWithLocationBase(DK_GenericWithLoc, Severity, Fn, Loc),
390+
MsgStr(MsgStr) {}
391+
392+
const Twine &getMsgStr() const { return MsgStr; }
393+
394+
/// \see DiagnosticInfo::print.
395+
void print(DiagnosticPrinter &DP) const override;
396+
397+
static bool classof(const DiagnosticInfo *DI) {
398+
return DI->getKind() == DK_GenericWithLoc;
399+
}
400+
};
401+
357402
/// Diagnostic information for stack size etc. reporting.
358403
/// This is basically a function and a size.
359404
class DiagnosticInfoResourceLimit : public DiagnosticInfoWithLocationBase {

‎llvm/include/llvm/IR/LLVMContext.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,6 @@ class LLVMContext {
305305
/// be prepared to drop the erroneous construct on the floor and "not crash".
306306
/// The generated code need not be correct. The error message will be
307307
/// implicitly prefixed with "error: " and should not end with a ".".
308-
void emitError(uint64_t LocCookie, const Twine &ErrorStr);
309308
void emitError(const Instruction *I, const Twine &ErrorStr);
310309
void emitError(const Twine &ErrorStr);
311310

‎llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,11 @@ static void EmitInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
312312
}
313313
}
314314
if (Error) {
315-
std::string msg;
316-
raw_string_ostream Msg(msg);
317-
Msg << "invalid operand in inline asm: '" << AsmStr << "'";
318-
MMI->getModule()->getContext().emitError(LocCookie, msg);
315+
const Function &Fn = MI->getMF()->getFunction();
316+
DiagnosticInfoInlineAsm DI(LocCookie,
317+
"invalid operand in inline asm: '" +
318+
Twine(AsmStr) + "'");
319+
Fn.getContext().diagnose(DI);
319320
}
320321
}
321322
break;
@@ -347,20 +348,11 @@ void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
347348
// enabled, so we use emitRawComment.
348349
OutStreamer->emitRawComment(MAI->getInlineAsmStart());
349350

350-
// Get the !srcloc metadata node if we have it, and decode the loc cookie from
351-
// it.
352-
uint64_t LocCookie = 0;
353-
const MDNode *LocMD = nullptr;
354-
for (const MachineOperand &MO : llvm::reverse(MI->operands())) {
355-
if (MO.isMetadata() && (LocMD = MO.getMetadata()) &&
356-
LocMD->getNumOperands() != 0) {
357-
if (const ConstantInt *CI =
358-
mdconst::dyn_extract<ConstantInt>(LocMD->getOperand(0))) {
359-
LocCookie = CI->getZExtValue();
360-
break;
361-
}
362-
}
363-
}
351+
const MDNode *LocMD = MI->getLocCookieMD();
352+
uint64_t LocCookie =
353+
LocMD
354+
? mdconst::extract<ConstantInt>(LocMD->getOperand(0))->getZExtValue()
355+
: 0;
364356

365357
// Emit the inline asm to a temporary string so we can emit it through
366358
// EmitInlineAsm.
@@ -397,20 +389,23 @@ void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
397389
Msg += LS;
398390
Msg += TRI->getRegAsmName(RR);
399391
}
392+
393+
const Function &Fn = MF->getFunction();
400394
const char *Note =
401395
"Reserved registers on the clobber list may not be "
402396
"preserved across the asm statement, and clobbering them may "
403397
"lead to undefined behaviour.";
404-
MMI->getModule()->getContext().diagnose(DiagnosticInfoInlineAsm(
405-
LocCookie, Msg, DiagnosticSeverity::DS_Warning));
406-
MMI->getModule()->getContext().diagnose(
398+
LLVMContext &Ctx = Fn.getContext();
399+
Ctx.diagnose(DiagnosticInfoInlineAsm(LocCookie, Msg,
400+
DiagnosticSeverity::DS_Warning));
401+
Ctx.diagnose(
407402
DiagnosticInfoInlineAsm(LocCookie, Note, DiagnosticSeverity::DS_Note));
408403

409404
for (const Register RR : RestrRegs) {
410405
if (std::optional<std::string> reason =
411406
TRI->explainReservedReg(*MF, RR)) {
412-
MMI->getModule()->getContext().diagnose(DiagnosticInfoInlineAsm(
413-
LocCookie, *reason, DiagnosticSeverity::DS_Note));
407+
Ctx.diagnose(DiagnosticInfoInlineAsm(LocCookie, *reason,
408+
DiagnosticSeverity::DS_Note));
414409
}
415410
}
416411
}

‎llvm/lib/CodeGen/MachineInstr.cpp‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2219,26 +2219,36 @@ MachineInstrExpressionTrait::getHashValue(const MachineInstr* const &MI) {
22192219
return hash_combine_range(HashComponents.begin(), HashComponents.end());
22202220
}
22212221

2222-
void MachineInstr::emitError(StringRef Msg) const {
2222+
const MDNode *MachineInstr::getLocCookieMD() const {
22232223
// Find the source location cookie.
2224-
uint64_t LocCookie = 0;
22252224
const MDNode *LocMD = nullptr;
22262225
for (unsigned i = getNumOperands(); i != 0; --i) {
22272226
if (getOperand(i-1).isMetadata() &&
22282227
(LocMD = getOperand(i-1).getMetadata()) &&
22292228
LocMD->getNumOperands() != 0) {
2230-
if (const ConstantInt *CI =
2231-
mdconst::dyn_extract<ConstantInt>(LocMD->getOperand(0))) {
2232-
LocCookie = CI->getZExtValue();
2233-
break;
2234-
}
2229+
if (mdconst::hasa<ConstantInt>(LocMD->getOperand(0)))
2230+
return LocMD;
22352231
}
22362232
}
22372233

2238-
if (const MachineBasicBlock *MBB = getParent())
2239-
if (const MachineFunction *MF = MBB->getParent())
2240-
return MF->getFunction().getContext().emitError(LocCookie, Msg);
2241-
report_fatal_error(Msg);
2234+
return nullptr;
2235+
}
2236+
2237+
void MachineInstr::emitInlineAsmError(const Twine &Msg) const {
2238+
assert(isInlineAsm());
2239+
const MDNode *LocMD = getLocCookieMD();
2240+
uint64_t LocCookie =
2241+
LocMD
2242+
? mdconst::extract<ConstantInt>(LocMD->getOperand(0))->getZExtValue()
2243+
: 0;
2244+
LLVMContext &Ctx = getMF()->getFunction().getContext();
2245+
Ctx.diagnose(DiagnosticInfoInlineAsm(LocCookie, Msg));
2246+
}
2247+
2248+
void MachineInstr::emitGenericError(const Twine &Msg) const {
2249+
const Function &Fn = getMF()->getFunction();
2250+
Fn.getContext().diagnose(
2251+
DiagnosticInfoGenericWithLoc(Msg, Fn, getDebugLoc()));
22422252
}
22432253

22442254
MachineInstrBuilder llvm::BuildMI(MachineFunction &MF, const DebugLoc &DL,

‎llvm/lib/CodeGen/RegAllocBase.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ void RegAllocBase::allocatePhysRegs() {
127127
if (AllocOrder.empty())
128128
report_fatal_error("no registers from class available to allocate");
129129
else if (MI && MI->isInlineAsm()) {
130-
MI->emitError("inline assembly requires more registers than available");
130+
MI->emitInlineAsmError(
131+
"inline assembly requires more registers than available");
131132
} else if (MI) {
132133
LLVMContext &Context =
133134
MI->getParent()->getParent()->getFunction().getContext();

‎llvm/lib/CodeGen/RegAllocFast.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -964,9 +964,10 @@ void RegAllocFastImpl::allocVirtReg(MachineInstr &MI, LiveReg &LR,
964964
// Nothing we can do: Report an error and keep going with an invalid
965965
// allocation.
966966
if (MI.isInlineAsm())
967-
MI.emitError("inline assembly requires more registers than available");
967+
MI.emitInlineAsmError(
968+
"inline assembly requires more registers than available");
968969
else
969-
MI.emitError("ran out of registers during register allocation");
970+
MI.emitInlineAsmError("ran out of registers during register allocation");
970971

971972
LR.Error = true;
972973
LR.PhysReg = 0;

‎llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -319,13 +319,14 @@ getCopyFromParts(SelectionDAG &DAG, const SDLoc &DL, const SDValue *Parts,
319319
static void diagnosePossiblyInvalidConstraint(LLVMContext &Ctx, const Value *V,
320320
const Twine &ErrMsg) {
321321
const Instruction *I = dyn_cast_or_null<Instruction>(V);
322-
if (!V)
322+
if (!I)
323323
return Ctx.emitError(ErrMsg);
324324

325-
const char *AsmError = ", possible invalid constraint for vector type";
326325
if (const CallInst *CI = dyn_cast<CallInst>(I))
327-
if (CI->isInlineAsm())
328-
return Ctx.emitError(I, ErrMsg + AsmError);
326+
if (CI->isInlineAsm()) {
327+
return Ctx.diagnose(DiagnosticInfoInlineAsm(
328+
*CI, ErrMsg + ", possible invalid constraint for vector type"));
329+
}
329330

330331
return Ctx.emitError(I, ErrMsg);
331332
}
@@ -10509,7 +10510,7 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
1050910510
void SelectionDAGBuilder::emitInlineAsmError(const CallBase &Call,
1051010511
const Twine &Message) {
1051110512
LLVMContext &Ctx = *DAG.getContext();
10512-
Ctx.emitError(&Call, Message);
10513+
Ctx.diagnose(DiagnosticInfoInlineAsm(Call, Message));
1051310514

1051410515
// Make sure we leave the DAG in a valid state
1051510516
const TargetLowering &TLI = DAG.getTargetLoweringInfo();

‎llvm/lib/CodeGen/XRayInstrumentation.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "llvm/CodeGen/TargetInstrInfo.h"
2525
#include "llvm/CodeGen/TargetSubtargetInfo.h"
2626
#include "llvm/IR/Attributes.h"
27+
#include "llvm/IR/DiagnosticInfo.h"
2728
#include "llvm/IR/Function.h"
2829
#include "llvm/InitializePasses.h"
2930
#include "llvm/Pass.h"
@@ -211,8 +212,12 @@ bool XRayInstrumentation::runOnMachineFunction(MachineFunction &MF) {
211212
auto &FirstMI = *FirstMBB.begin();
212213

213214
if (!MF.getSubtarget().isXRaySupported()) {
214-
FirstMI.emitError("An attempt to perform XRay instrumentation for an"
215-
" unsupported target.");
215+
216+
const Function &Fn = FirstMBB.getParent()->getFunction();
217+
Fn.getContext().diagnose(DiagnosticInfoUnsupported(
218+
Fn, "An attempt to perform XRay instrumentation for an"
219+
" unsupported target."));
220+
216221
return false;
217222
}
218223

‎llvm/lib/IR/DiagnosticInfo.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,20 @@ int llvm::getNextAvailablePluginDiagnosticKind() {
4848

4949
const char *OptimizationRemarkAnalysis::AlwaysPrint = "";
5050

51+
void DiagnosticInfoGeneric::print(DiagnosticPrinter &DP) const {
52+
DP << getMsgStr();
53+
}
54+
55+
void DiagnosticInfoGenericWithLoc::print(DiagnosticPrinter &DP) const {
56+
DP << getLocationStr() << ": " << getMsgStr();
57+
}
58+
59+
DiagnosticInfoInlineAsm::DiagnosticInfoInlineAsm(uint64_t LocCookie,
60+
const Twine &MsgStr,
61+
DiagnosticSeverity Severity)
62+
: DiagnosticInfo(DK_InlineAsm, Severity), LocCookie(LocCookie),
63+
MsgStr(MsgStr) {}
64+
5165
DiagnosticInfoInlineAsm::DiagnosticInfoInlineAsm(const Instruction &I,
5266
const Twine &MsgStr,
5367
DiagnosticSeverity Severity)

0 commit comments

Comments
 (0)