[llvm] FastISel: Assert the emitted instruction defines the result (PR #226502)
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 26 12:40:19 PDT 2026
https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/226502
>From 34e55a6865ea63a3e633cc34a1f773d5d4fff6fa Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Fri, 25 Sep 2026 16:01:33 +0200
Subject: [PATCH 1/3] FastISel: Assert the emitted instruction defines the
result
The fallback path copied the result out of implicit_defs()[0], assuming
the first implicit physical register def is the result. That is an X86
assumption about MUL/IMUL, and it is unreachable for all but
fastEmitInst_r: FastISelEmitter skips any instruction whose first
operand is not an output register, so every opcode reaching these
helpers from generated code has an explicit def.
Co-Authored-By: Claude Opus 5 <noreply at anthropic.com>
---
llvm/lib/CodeGen/SelectionDAG/FastISel.cpp | 113 +++++----------------
1 file changed, 28 insertions(+), 85 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index 5fc4611d077b0..5cbc7f385304d 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -2018,18 +2018,10 @@ Register FastISel::fastEmitInst_rr(unsigned MachineInstOpcode,
Op0 = constrainOperandRegClass(II, Op0, II.getNumDefs());
Op1 = constrainOperandRegClass(II, Op1, II.getNumDefs() + 1);
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addReg(Op0)
- .addReg(Op1);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addReg(Op0)
- .addReg(Op1);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
+ .addReg(Op0)
+ .addReg(Op1);
return ResultReg;
}
@@ -2043,20 +2035,11 @@ Register FastISel::fastEmitInst_rrr(unsigned MachineInstOpcode,
Op1 = constrainOperandRegClass(II, Op1, II.getNumDefs() + 1);
Op2 = constrainOperandRegClass(II, Op2, II.getNumDefs() + 2);
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addReg(Op0)
- .addReg(Op1)
- .addReg(Op2);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addReg(Op0)
- .addReg(Op1)
- .addReg(Op2);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
+ .addReg(Op0)
+ .addReg(Op1)
+ .addReg(Op2);
return ResultReg;
}
@@ -2068,18 +2051,10 @@ Register FastISel::fastEmitInst_ri(unsigned MachineInstOpcode,
Register ResultReg = createResultReg(RC);
Op0 = constrainOperandRegClass(II, Op0, II.getNumDefs());
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addReg(Op0)
- .addImm(Imm);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addReg(Op0)
- .addImm(Imm);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
+ .addReg(Op0)
+ .addImm(Imm);
return ResultReg;
}
@@ -2091,20 +2066,11 @@ Register FastISel::fastEmitInst_rii(unsigned MachineInstOpcode,
Register ResultReg = createResultReg(RC);
Op0 = constrainOperandRegClass(II, Op0, II.getNumDefs());
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addReg(Op0)
- .addImm(Imm1)
- .addImm(Imm2);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addReg(Op0)
- .addImm(Imm1)
- .addImm(Imm2);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
+ .addReg(Op0)
+ .addImm(Imm1)
+ .addImm(Imm2);
return ResultReg;
}
@@ -2115,16 +2081,9 @@ Register FastISel::fastEmitInst_f(unsigned MachineInstOpcode,
Register ResultReg = createResultReg(RC);
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addFPImm(FPImm);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addFPImm(FPImm);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
+ .addFPImm(FPImm);
return ResultReg;
}
@@ -2137,20 +2096,11 @@ Register FastISel::fastEmitInst_rri(unsigned MachineInstOpcode,
Op0 = constrainOperandRegClass(II, Op0, II.getNumDefs());
Op1 = constrainOperandRegClass(II, Op1, II.getNumDefs() + 1);
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addReg(Op0)
- .addReg(Op1)
- .addImm(Imm);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addReg(Op0)
- .addReg(Op1)
- .addImm(Imm);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
+ .addReg(Op0)
+ .addReg(Op1)
+ .addImm(Imm);
return ResultReg;
}
@@ -2159,15 +2109,8 @@ Register FastISel::fastEmitInst_i(unsigned MachineInstOpcode,
Register ResultReg = createResultReg(RC);
const MCInstrDesc &II = TII.get(MachineInstOpcode);
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addImm(Imm);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II).addImm(Imm);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg).addImm(Imm);
return ResultReg;
}
>From ea51b99a39bfb34ad52f3a3146af19c32dcfd99c Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Sat, 26 Sep 2026 11:43:18 +0200
Subject: [PATCH 2/3] Add a FastISel helper for with-overflow multiply emission
---
llvm/lib/Target/X86/X86FastISel.cpp | 41 ++++++++++++++++++++---------
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index 63d8dca0b10bb..c9d7f04f596dc 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -88,6 +88,12 @@ class X86FastISel final : public FastISel {
bool X86FastEmitExtend(ISD::NodeType Opc, EVT DstVT, Register Src, EVT SrcVT,
Register &ResultReg);
+ /// Emit a MUL or IMUL of \p LHSReg and \p RHSReg. \p AccReg is the
+ /// accumulator the instruction implicitly reads and implicitly defines with
+ /// the low half of the product.
+ Register X86FastEmitMul(unsigned Opc, MVT VT, MCRegister AccReg,
+ Register LHSReg, Register RHSReg);
+
bool X86SelectAddress(const Value *V, X86AddressMode &AM);
bool X86SelectCallAddress(const Value *V, X86AddressMode &AM);
@@ -711,6 +717,23 @@ bool X86FastISel::X86FastEmitExtend(ISD::NodeType Opc, EVT DstVT, Register Src,
return true;
}
+Register X86FastISel::X86FastEmitMul(unsigned Opc, MVT VT, MCRegister AccReg,
+ Register LHSReg, Register RHSReg) {
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
+ AccReg)
+ .addReg(LHSReg);
+
+ const MCInstrDesc &II = TII.get(Opc);
+ Register ResultReg = createResultReg(TLI.getRegClassFor(VT));
+ RHSReg = constrainOperandRegClass(II, RHSReg, II.getNumDefs());
+
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II).addReg(RHSReg);
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
+ ResultReg)
+ .addReg(AccReg);
+ return ResultReg;
+}
+
bool X86FastISel::handleConstantAddresses(const Value *V, X86AddressMode &AM) {
// Handle constant address.
if (const GlobalValue *GV = dyn_cast<GlobalValue>(V)) {
@@ -2903,23 +2926,17 @@ bool X86FastISel::fastLowerIntrinsicCall(const IntrinsicInst *II) {
static const uint16_t MULOpc[] =
{ X86::MUL8r, X86::MUL16r, X86::MUL32r, X86::MUL64r };
static const MCPhysReg Reg[] = { X86::AL, X86::AX, X86::EAX, X86::RAX };
- // First copy the first operand into RAX, which is an implicit input to
- // the X86::MUL*r instruction.
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD,
- TII.get(TargetOpcode::COPY), Reg[VT.SimpleTy-MVT::i8])
- .addReg(LHSReg);
- ResultReg = fastEmitInst_r(MULOpc[VT.SimpleTy-MVT::i8],
- TLI.getRegClassFor(VT), RHSReg);
+ // The first operand goes in RAX, which is an implicit input to the
+ // X86::MUL*r instruction.
+ ResultReg = X86FastEmitMul(MULOpc[VT.SimpleTy - MVT::i8], VT,
+ Reg[VT.SimpleTy - MVT::i8], LHSReg, RHSReg);
} else if (BaseOpc == X86ISD::SMUL && !ResultReg) {
static const uint16_t MULOpc[] =
{ X86::IMUL8r, X86::IMUL16rr, X86::IMUL32rr, X86::IMUL64rr };
if (VT == MVT::i8) {
- // Copy the first operand into AL, which is an implicit input to the
+ // The first operand goes in AL, which is an implicit input to the
// X86::IMUL8r instruction.
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD,
- TII.get(TargetOpcode::COPY), X86::AL)
- .addReg(LHSReg);
- ResultReg = fastEmitInst_r(MULOpc[0], TLI.getRegClassFor(VT), RHSReg);
+ ResultReg = X86FastEmitMul(MULOpc[0], VT, X86::AL, LHSReg, RHSReg);
} else
ResultReg = fastEmitInst_rr(MULOpc[VT.SimpleTy-MVT::i8],
TLI.getRegClassFor(VT), LHSReg, RHSReg);
>From f26ee9ac63a66b0d1db6db51e5f81e5cedac32d7 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Sat, 26 Sep 2026 12:00:09 +0200
Subject: [PATCH 3/3] Assert fastEmitInst_rrrr's instruction defines the result
---
llvm/lib/Target/X86/X86FastISel.cpp | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index c9d7f04f596dc..36a57b2a0f2fc 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -4065,22 +4065,12 @@ Register X86FastISel::fastEmitInst_rrrr(unsigned MachineInstOpcode,
Op2 = constrainOperandRegClass(II, Op2, II.getNumDefs() + 2);
Op3 = constrainOperandRegClass(II, Op3, II.getNumDefs() + 3);
- if (II.getNumDefs() >= 1)
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
- .addReg(Op0)
- .addReg(Op1)
- .addReg(Op2)
- .addReg(Op3);
- else {
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II)
- .addReg(Op0)
- .addReg(Op1)
- .addReg(Op2)
- .addReg(Op3);
- BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(TargetOpcode::COPY),
- ResultReg)
- .addReg(II.implicit_defs()[0]);
- }
+ assert(II.getNumDefs() >= 1 && "instruction must define the result");
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, II, ResultReg)
+ .addReg(Op0)
+ .addReg(Op1)
+ .addReg(Op2)
+ .addReg(Op3);
return ResultReg;
}
More information about the llvm-commits
mailing list