Skip to content

Commit 534d221

Browse files
authored
(reland) [GlobalISel] Diagnose inline assembly constraint lowering errors (#139049)
The initial patch (#135782 caused issues because it emits an error, and llc is sensitive to it. It also caused compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp to fail. Use warnings instead + reject lowering. That way, the fallback path is used without llc/clang returning a failure code. If fallback isn't enabled then the warnings provide context as to why lowering failed. Original commit description for #135782: Instead of printing something to dbgs (which is not visible to all users), emit a diagnostic like the DAG does. We still crash later because we fail to select the inline assembly, but at least now users will know why it's crashing. In a future patch we could also recover from the error like the DAG does, so the lowering can keep going until it either crashes or gives a different error later.
1 parent 0d47a45 commit 534d221

File tree

2 files changed

+71
-33
lines changed

2 files changed

+71
-33
lines changed

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "llvm/CodeGen/MachineOperand.h"
1717
#include "llvm/CodeGen/MachineRegisterInfo.h"
1818
#include "llvm/CodeGen/TargetLowering.h"
19+
#include "llvm/IR/DiagnosticInfo.h"
1920
#include "llvm/IR/Module.h"
2021

2122
#define DEBUG_TYPE "inline-asm-lowering"
@@ -231,6 +232,19 @@ bool InlineAsmLowering::lowerInlineAsm(
231232
TargetLowering::AsmOperandInfoVector TargetConstraints =
232233
TLI->ParseConstraints(DL, TRI, Call);
233234

235+
const auto ConstraintError = [&](const GISelAsmOperandInfo &Info, Twine Msg) {
236+
// Use warnings in combination with a "return false" to trigger the fallback
237+
// path. If fallback isn't enabled, then another error will be emitted later
238+
// and the warnings will provide context as to why the error occured.
239+
LLVMContext &Ctx = MIRBuilder.getContext();
240+
Ctx.diagnose(DiagnosticInfoInlineAsm(
241+
Call, "invalid constraint '" + Info.ConstraintCode + "': " + Msg,
242+
DS_Warning));
243+
// TODO: If we could detect that the fallback isn't enabled, we could
244+
// recover here by defining all result registers as G_IMPLICIT_DEF.
245+
return false;
246+
};
247+
234248
ExtraFlags ExtraInfo(Call);
235249
unsigned ArgNo = 0; // ArgNo - The argument of the CallInst.
236250
unsigned ResNo = 0; // ResNo - The result number of the next output.
@@ -243,8 +257,8 @@ bool InlineAsmLowering::lowerInlineAsm(
243257
OpInfo.CallOperandVal = const_cast<Value *>(Call.getArgOperand(ArgNo));
244258

245259
if (isa<BasicBlock>(OpInfo.CallOperandVal)) {
246-
LLVM_DEBUG(dbgs() << "Basic block input operands not supported yet\n");
247-
return false;
260+
return ConstraintError(OpInfo,
261+
"basic block input operands not supported yet");
248262
}
249263

250264
Type *OpTy = OpInfo.CallOperandVal->getType();
@@ -258,9 +272,8 @@ bool InlineAsmLowering::lowerInlineAsm(
258272

259273
// FIXME: Support aggregate input operands
260274
if (!OpTy->isSingleValueType()) {
261-
LLVM_DEBUG(
262-
dbgs() << "Aggregate input operands are not supported yet\n");
263-
return false;
275+
return ConstraintError(OpInfo,
276+
"aggregate input operands not supported yet");
264277
}
265278

266279
OpInfo.ConstraintVT =
@@ -344,9 +357,8 @@ bool InlineAsmLowering::lowerInlineAsm(
344357

345358
// Find a register that we can use.
346359
if (OpInfo.Regs.empty()) {
347-
LLVM_DEBUG(dbgs()
348-
<< "Couldn't allocate output register for constraint\n");
349-
return false;
360+
return ConstraintError(
361+
OpInfo, "could not allocate output register for constraint");
350362
}
351363

352364
// Add information to the INLINEASM instruction to know that this
@@ -389,13 +401,13 @@ bool InlineAsmLowering::lowerInlineAsm(
389401

390402
const InlineAsm::Flag MatchedOperandFlag(Inst->getOperand(InstFlagIdx).getImm());
391403
if (MatchedOperandFlag.isMemKind()) {
392-
LLVM_DEBUG(dbgs() << "Matching input constraint to mem operand not "
393-
"supported. This should be target specific.\n");
394-
return false;
404+
return ConstraintError(
405+
OpInfo,
406+
"matching input constraint to mem operand not supported; this "
407+
"should be target specific");
395408
}
396409
if (!MatchedOperandFlag.isRegDefKind() && !MatchedOperandFlag.isRegDefEarlyClobberKind()) {
397-
LLVM_DEBUG(dbgs() << "Unknown matching constraint\n");
398-
return false;
410+
return ConstraintError(OpInfo, "unknown matching constraint");
399411
}
400412

401413
// We want to tie input to register in next operand.
@@ -425,9 +437,10 @@ bool InlineAsmLowering::lowerInlineAsm(
425437

426438
if (OpInfo.ConstraintType == TargetLowering::C_Other &&
427439
OpInfo.isIndirect) {
428-
LLVM_DEBUG(dbgs() << "Indirect input operands with unknown constraint "
429-
"not supported yet\n");
430-
return false;
440+
return ConstraintError(
441+
OpInfo,
442+
"indirect input operands with unknown constraint not supported "
443+
"yet");
431444
}
432445

433446
if (OpInfo.ConstraintType == TargetLowering::C_Immediate ||
@@ -437,9 +450,7 @@ bool InlineAsmLowering::lowerInlineAsm(
437450
if (!lowerAsmOperandForConstraint(OpInfo.CallOperandVal,
438451
OpInfo.ConstraintCode, Ops,
439452
MIRBuilder)) {
440-
LLVM_DEBUG(dbgs() << "Don't support constraint: "
441-
<< OpInfo.ConstraintCode << " yet\n");
442-
return false;
453+
return ConstraintError(OpInfo, "unsupported constraint");
443454
}
444455

445456
assert(Ops.size() > 0 &&
@@ -456,9 +467,8 @@ bool InlineAsmLowering::lowerInlineAsm(
456467
if (OpInfo.ConstraintType == TargetLowering::C_Memory) {
457468

458469
if (!OpInfo.isIndirect) {
459-
LLVM_DEBUG(dbgs()
460-
<< "Cannot indirectify memory input operands yet\n");
461-
return false;
470+
return ConstraintError(
471+
OpInfo, "indirect memory input operands are not supported yet");
462472
}
463473

464474
assert(OpInfo.isIndirect && "Operand must be indirect to be a mem!");
@@ -482,18 +492,15 @@ bool InlineAsmLowering::lowerInlineAsm(
482492
"Unknown constraint type!");
483493

484494
if (OpInfo.isIndirect) {
485-
LLVM_DEBUG(dbgs() << "Can't handle indirect register inputs yet "
486-
"for constraint '"
487-
<< OpInfo.ConstraintCode << "'\n");
488-
return false;
495+
return ConstraintError(
496+
OpInfo, "indirect register inputs are not supported yet");
489497
}
490498

491499
// Copy the input into the appropriate registers.
492500
if (OpInfo.Regs.empty()) {
493-
LLVM_DEBUG(
494-
dbgs()
495-
<< "Couldn't allocate input register for register constraint\n");
496-
return false;
501+
return ConstraintError(
502+
OpInfo,
503+
"could not allocate input register for register constraint");
497504
}
498505

499506
unsigned NumRegs = OpInfo.Regs.size();
@@ -503,9 +510,10 @@ bool InlineAsmLowering::lowerInlineAsm(
503510
"source registers");
504511

505512
if (NumRegs > 1) {
506-
LLVM_DEBUG(dbgs() << "Input operands with multiple input registers are "
507-
"not supported yet\n");
508-
return false;
513+
return ConstraintError(
514+
OpInfo,
515+
"input operands with multiple input registers are not supported "
516+
"yet");
509517
}
510518

511519
InlineAsm::Flag Flag(InlineAsm::Kind::RegUse, NumRegs);
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
; RUN: not llc -mtriple=amdgcn -mcpu=fiji -O0 -global-isel -global-isel-abort=2 -pass-remarks-missed='gisel*' %s -o - 2>&1 | FileCheck %s
2+
3+
; CHECK: warning: invalid constraint '': aggregate input operands not supported yet
4+
define amdgpu_kernel void @aggregates([4 x i8] %val) {
5+
tail call void asm sideeffect "s_nop", "r"([4 x i8] %val)
6+
ret void
7+
}
8+
9+
; CHECK: warning: invalid constraint '{s999}': could not allocate output register for constraint
10+
define amdgpu_kernel void @bad_output() {
11+
tail call i32 asm sideeffect "s_nop", "={s999}"()
12+
ret void
13+
}
14+
15+
; CHECK: warning: invalid constraint '{s998}': could not allocate input register for register constraint
16+
define amdgpu_kernel void @bad_input() {
17+
tail call void asm sideeffect "s_nop", "{s998}"(i32 poison)
18+
ret void
19+
}
20+
; CHECK: warning: invalid constraint '{s997}': indirect register inputs are not supported yet
21+
define amdgpu_kernel void @indirect_input() {
22+
tail call void asm sideeffect "s_nop", "*{s997}"(ptr elementtype(i32) poison)
23+
ret void
24+
}
25+
26+
; CHECK: warning: invalid constraint 'i': unsupported constraint
27+
define amdgpu_kernel void @badimm() {
28+
tail call void asm sideeffect "s_nop", "i"(i32 poison)
29+
ret void
30+
}

0 commit comments

Comments
 (0)