[GISel] Fix crash in GlobalISel utils method #153334
Merged
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
The `getDefSrcRegIgnoringCopies` method in GlobalISel Utils crashed when the first operand of the input instruction was not a register, e.g., the `INLINEASM` instruction has a non-register first operand.
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-backend-amdgpu Author: Victor Mustya (vmustya) ChangesThe Full diff: https://github.com/llvm/llvm-project/pull/153334.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 8955dd0370539..c6822f32d01b7 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -467,7 +467,10 @@ std::optional<DefinitionAndSourceRegister>
llvm::getDefSrcRegIgnoringCopies(Register Reg, const MachineRegisterInfo &MRI) {
Register DefSrcReg = Reg;
auto *DefMI = MRI.getVRegDef(Reg);
- auto DstTy = MRI.getType(DefMI->getOperand(0).getReg());
+ auto &Opnd = DefMI->getOperand(0);
+ if (!Opnd.isReg())
+ return DefinitionAndSourceRegister{DefMI, DefSrcReg};
+ auto DstTy = MRI.getType(Opnd.getReg());
if (!DstTy.isValid())
return std::nullopt;
unsigned Opc = DefMI->getOpcode();
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankcombiner-inlineasm-crash.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankcombiner-inlineasm-crash.mir
new file mode 100644
index 0000000000000..4bff92314c07b
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankcombiner-inlineasm-crash.mir
@@ -0,0 +1,40 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=amdgcn-amd-mesa3d -mcpu=gfx1200 -run-pass=amdgpu-regbank-combiner -verify-machineinstrs %s -o - | FileCheck %s
+
+# COM: Check that the pass doesn't crash.
+
+---
+name: test_fmed3
+legalized: true
+regBankSelected: true
+tracksRegLiveness: true
+machineFunctionInfo:
+ mode:
+ ieee: true
+ dx10-clamp: true
+body: |
+ bb.1 :
+ liveins: $vgpr0
+
+ ; CHECK-LABEL: name: test_fmed3
+ ; CHECK: liveins: $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
+ ; CHECK-NEXT: [[C:%[0-9]+]]:sgpr(s32) = G_FCONSTANT float 2.000000e+00
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
+ ; CHECK-NEXT: [[FMUL:%[0-9]+]]:vgpr(s32) = G_FMUL [[COPY]], [[COPY1]]
+ ; CHECK-NEXT: [[C1:%[0-9]+]]:sgpr(s32) = G_FCONSTANT float 1.000000e+00
+ ; CHECK-NEXT: INLINEASM &"v_mov_b32 $0, 0", 0 /* attdialect */, 2228234 /* regdef:VGPR_32 */, def %5(s32)
+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:vgpr(s32) = COPY [[C1]](s32)
+ ; CHECK-NEXT: [[AMDGPU_FMED3_:%[0-9]+]]:vgpr(s32) = nnan G_AMDGPU_FMED3 [[FMUL]], %5, [[COPY2]]
+ ; CHECK-NEXT: $vgpr0 = COPY [[AMDGPU_FMED3_]](s32)
+ %0:vgpr(s32) = COPY $vgpr0
+ %2:sgpr(s32) = G_FCONSTANT float 2.000000e+00
+ %8:vgpr(s32) = COPY %2(s32)
+ %3:vgpr(s32) = G_FMUL %0, %8
+ %6:sgpr(s32) = G_FCONSTANT float 1.000000e+00
+ INLINEASM &"v_mov_b32 $0, 0", 0 /* attdialect */, 2228234 /* regdef:VGPR_32 */, def %5:vgpr_32
+ %10:vgpr(s32) = COPY %6(s32)
+ %4:vgpr(s32) = nnan G_AMDGPU_FMED3 %3(s32), %5(s32), %10(s32)
+ $vgpr0 = COPY %4(s32)
+...
|
arsenm
reviewed
Aug 13, 2025
| # COM: Check that the pass doesn't crash. | ||
|
|
||
| --- | ||
| name: test_fmed3 |
There was a problem hiding this comment.
Can you fix the function name, this doesn't have anything to do with fmed3
| @@ -0,0 +1,40 @@ | |||
| # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5 | |||
| # RUN: llc -mtriple=amdgcn-amd-mesa3d -mcpu=gfx1200 -run-pass=amdgpu-regbank-combiner -verify-machineinstrs %s -o - | FileCheck %s | |||
There was a problem hiding this comment.
Suggested change
| # RUN: llc -mtriple=amdgcn-amd-mesa3d -mcpu=gfx1200 -run-pass=amdgpu-regbank-combiner -verify-machineinstrs %s -o - | FileCheck %s | |
| # RUN: llc -mtriple=amdgcn-amd-mesa3d -mcpu=gfx1200 -run-pass=amdgpu-regbank-combiner %s -o - | FileCheck %s |
| ; CHECK-NEXT: [[AMDGPU_FMED3_:%[0-9]+]]:vgpr(s32) = nnan G_AMDGPU_FMED3 [[FMUL]], %5, [[COPY2]] | ||
| ; CHECK-NEXT: $vgpr0 = COPY [[AMDGPU_FMED3_]](s32) | ||
| %0:vgpr(s32) = COPY $vgpr0 | ||
| %2:sgpr(s32) = G_FCONSTANT float 2.000000e+00 |
Comment on lines
470
to
472
| auto &Opnd = DefMI->getOperand(0); | ||
| if (!Opnd.isReg()) | ||
| return DefinitionAndSourceRegister{DefMI, DefSrcReg}; |
There was a problem hiding this comment.
This happens to work for the inline assembly case, but this will be broken for multi-def instructions.
I don't know why we don't have a version of getVRegDef that returns the MachineOperand; the underlying use-def list is in terms of operands already, so we could just add that without assuming the def index
There was a problem hiding this comment.
There is the def_begin(Register) method, that can be used to get the MachineOperand. I've re-implemented the fix using this method: 15bc48b
| %10:vgpr(s32) = COPY %6(s32) | ||
| %4:vgpr(s32) = nnan G_AMDGPU_FMED3 %3(s32), %5(s32), %10(s32) | ||
| $vgpr0 = COPY %4(s32) | ||
| ... |
There was a problem hiding this comment.
Can you add a case where the source is a one of the > 0 index result from G_UNMERGE_VALUES
arsenm
approved these changes
Aug 20, 2025
Co-authored-by: Matt Arsenault <arsenm2@gmail.com>
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/12153 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.
The
getDefSrcRegIgnoringCopiesmethod in GlobalISel Utils crashed whenthe first operand of the input instruction was not a register, e.g.,
the
INLINEASMinstruction has a non-register first operand.