Skip to content

Commit

Permalink
[GlobalISel] Fix crash in applyShiftOfShiftedLogic caused by CSEMIRBu…
Browse files Browse the repository at this point in the history
…ilder reuse instruction

If LogicNonShiftReg is the same to Shift1Base, and shift1 const is the same to MatchInfo.Shift2 const, CSEMIRBuilder will reuse the old shift1 when build shift2.
So, if we erase MatchInfo.Shift2 at the end, actually we remove old shift1. And it will cause crash later.

Solution for this issue is just erase it earlier to avoid the crash.

Fix #58423

Reviewed By: arsenm

Differential Revision: https://reviews.llvm.org/D138187
  • Loading branch information
bcl5980 committed Nov 19, 2022
1 parent 00be357 commit fe07eeb
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
10 changes: 8 additions & 2 deletions llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
Expand Up @@ -1598,6 +1598,13 @@ void CombinerHelper::applyShiftOfShiftedLogic(MachineInstr &MI,
Register Shift1 =
Builder.buildInstr(Opcode, {DestType}, {Shift1Base, Const}).getReg(0);

// If LogicNonShiftReg is the same to Shift1Base, and shift1 const is the same
// to MatchInfo.Shift2 const, CSEMIRBuilder will reuse the old shift1 when
// build shift2. So, if we erase MatchInfo.Shift2 at the end, actually we
// remove old shift1. And it will cause crash later. So erase it earlier to
// avoid the crash.
MatchInfo.Shift2->eraseFromParent();

Register Shift2Const = MI.getOperand(2).getReg();
Register Shift2 = Builder
.buildInstr(Opcode, {DestType},
Expand All @@ -1607,8 +1614,7 @@ void CombinerHelper::applyShiftOfShiftedLogic(MachineInstr &MI,
Register Dest = MI.getOperand(0).getReg();
Builder.buildInstr(MatchInfo.Logic->getOpcode(), {Dest}, {Shift1, Shift2});

// These were one use so it's safe to remove them.
MatchInfo.Shift2->eraseFromParent();
// This was one use so it's safe to remove it.
MatchInfo.Logic->eraseFromParent();

MI.eraseFromParent();
Expand Down
15 changes: 15 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/pr58423.ll
@@ -0,0 +1,15 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -global-isel -global-isel-abort=0 | FileCheck %s

; this used to crash
define i32 @f(i32 %a) {
; CHECK-LABEL: f:
; CHECK: // %bb.0:
; CHECK-NEXT: lsl w8, w0, #8
; CHECK-NEXT: orr w0, w8, w0, lsl #16
; CHECK-NEXT: ret
%shl = shl i32 %a, 8
%or = or i32 %a, %shl
%r = shl i32 %or, 8
ret i32 %r
}

0 comments on commit fe07eeb

Please sign in to comment.