Skip to content

Commit 5d99120

Browse files
arsenmarsenm
committed
Reapply "DiagnosticInfo: Clean up usage of DiagnosticInfoInlineAsm" (#119575)
This reverts commit 40986fe. Reapply with fix to prevent temporary Twine from going out of scope.
1 parent 36c7d14 commit 5d99120

File tree

15 files changed

+166
-82
lines changed

15 files changed

+166
-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: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,10 @@ 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+
Fn.getContext().diagnose(DiagnosticInfoInlineAsm(
317+
LocCookie,
318+
"invalid operand in inline asm: '" + Twine(AsmStr) + "'"));
319319
}
320320
}
321321
break;
@@ -347,20 +347,11 @@ void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
347347
// enabled, so we use emitRawComment.
348348
OutStreamer->emitRawComment(MAI->getInlineAsmStart());
349349

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-
}
350+
const MDNode *LocMD = MI->getLocCookieMD();
351+
uint64_t LocCookie =
352+
LocMD
353+
? mdconst::extract<ConstantInt>(LocMD->getOperand(0))->getZExtValue()
354+
: 0;
364355

365356
// Emit the inline asm to a temporary string so we can emit it through
366357
// EmitInlineAsm.
@@ -397,20 +388,23 @@ void AsmPrinter::emitInlineAsm(const MachineInstr *MI) const {
397388
Msg += LS;
398389
Msg += TRI->getRegAsmName(RR);
399390
}
391+
392+
const Function &Fn = MF->getFunction();
400393
const char *Note =
401394
"Reserved registers on the clobber list may not be "
402395
"preserved across the asm statement, and clobbering them may "
403396
"lead to undefined behaviour.";
404-
MMI->getModule()->getContext().diagnose(DiagnosticInfoInlineAsm(
405-
LocCookie, Msg, DiagnosticSeverity::DS_Warning));
406-
MMI->getModule()->getContext().diagnose(
397+
LLVMContext &Ctx = Fn.getContext();
398+
Ctx.diagnose(DiagnosticInfoInlineAsm(LocCookie, Msg,
399+
DiagnosticSeverity::DS_Warning));
400+
Ctx.diagnose(
407401
DiagnosticInfoInlineAsm(LocCookie, Note, DiagnosticSeverity::DS_Note));
408402

409403
for (const Register RR : RestrRegs) {
410404
if (std::optional<std::string> reason =
411405
TRI->explainReservedReg(*MF, RR)) {
412-
MMI->getModule()->getContext().diagnose(DiagnosticInfoInlineAsm(
413-
LocCookie, *reason, DiagnosticSeverity::DS_Note));
406+
Ctx.diagnose(DiagnosticInfoInlineAsm(LocCookie, *reason,
407+
DiagnosticSeverity::DS_Note));
414408
}
415409
}
416410
}

‎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
}
@@ -10503,7 +10504,7 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
1050310504
void SelectionDAGBuilder::emitInlineAsmError(const CallBase &Call,
1050410505
const Twine &Message) {
1050510506
LLVMContext &Ctx = *DAG.getContext();
10506-
Ctx.emitError(&Call, Message);
10507+
Ctx.diagnose(DiagnosticInfoInlineAsm(Call, Message));
1050710508

1050810509
// Make sure we leave the DAG in a valid state
1050910510
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)