[llvm] fe07eeb - [GlobalISel] Fix crash in applyShiftOfShiftedLogic caused by CSEMIRBuilder reuse instruction

[via llvm-commits]

Author: chenglin.bi
Date: 2022-11-19T09:13:44+08:00
New Revision: fe07eeb8254021fafe3dc0e88cc59862c1215f40

URL: https://github.com/llvm/llvm-project/commit/fe07eeb8254021fafe3dc0e88cc59862c1215f40
DIFF: https://github.com/llvm/llvm-project/commit/fe07eeb8254021fafe3dc0e88cc59862c1215f40.diff

LOG: [GlobalISel] Fix crash in applyShiftOfShiftedLogic caused by CSEMIRBuilder 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

Added: 
    llvm/test/CodeGen/AArch64/GlobalISel/pr58423.ll

Modified: 
    llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 22dc3c736d7f3..297e009b5da45 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -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},
@@ -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();

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/pr58423.ll b/llvm/test/CodeGen/AArch64/GlobalISel/pr58423.ll
new file mode 100644
index 0000000000000..0a032b0b0dbcc
--- /dev/null
+++ b/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
+}