[globalisel][combine] Fix a rare crash when encountering an instruction whose op0...

The simplest instance of this is an intrinsic with no results which will have the
intrinsic ID as operand 0.

Also fix some benign incorrectness when op0 is a reg but isn't a def that was
guarded against by checking for the extension opcodes.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@343821 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 06f0ea5..fe68c04 100644 (file)
--- a/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -60,12 +60,8 @@ PreferredTuple ChoosePreferredUse(PreferredTuple &CurrentUse,
                                   unsigned OpcodeForCandidate,
                                   MachineInstr *MIForCandidate) {
   if (!CurrentUse.Ty.isValid()) {
-    if (CurrentUse.ExtendOpcode == OpcodeForCandidate)
-      return {TyForCandidate, OpcodeForCandidate, MIForCandidate};
-    if (CurrentUse.ExtendOpcode == TargetOpcode::G_ANYEXT &&
-        (OpcodeForCandidate == TargetOpcode::G_SEXT ||
-         OpcodeForCandidate == TargetOpcode::G_ZEXT ||
-         OpcodeForCandidate == TargetOpcode::G_ANYEXT))
+    if (CurrentUse.ExtendOpcode == OpcodeForCandidate ||
+        CurrentUse.ExtendOpcode == TargetOpcode::G_ANYEXT)
       return {TyForCandidate, OpcodeForCandidate, MIForCandidate};
     return CurrentUse;
   }
@@ -171,10 +167,12 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) {
   PreferredTuple Preferred = {LLT(), PreferredOpcode, nullptr};
   for (auto &UseMI : MRI.use_instructions(LoadValue.getReg())) {
     if (UseMI.getOpcode() == TargetOpcode::G_SEXT ||
-        UseMI.getOpcode() == TargetOpcode::G_ZEXT || !Preferred.Ty.isValid())
+        UseMI.getOpcode() == TargetOpcode::G_ZEXT ||
+        UseMI.getOpcode() == TargetOpcode::G_ANYEXT) {
       Preferred = ChoosePreferredUse(Preferred,
                                      MRI.getType(UseMI.getOperand(0).getReg()),
                                      UseMI.getOpcode(), &UseMI);
+    }
   }
 
   // There were no extends

diff --git a/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir b/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir
index aaa4bfb..c9ba431 100644 (file)
--- a/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir
+++ b/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir
@@ -35,6 +35,16 @@
     ret void
   }
 
+  define void @use_doesnt_def_anything(i8* %addr) {
+  entry:
+    ret void
+  }
+
+  define void @op0_isnt_a_reg(i8* %addr) {
+  entry:
+    ret void
+  }
+
 ...
 
 ---
@@ -126,9 +136,6 @@ body: |
 ---
 name:            sink_to_phi_nondominating
 # CHECK-LABEL: name: sink_to_phi_nondominating
-# This test currently tests that we don't sink if we would sink to a phi. This
-# is needed to avoid inserting into the middle of the leading G_PHI instructions
-# of a BB
 tracksRegLiveness: true
 body: |
   bb.0.entry:
@@ -171,4 +178,37 @@ body: |
     $w1 = COPY %9
 ...
 
+---
+name:            use_doesnt_def_anything
+# CHECK-LABEL: name: use_doesnt_def_anything
+# Check that we don't crash when inspecting a use that doesn't define anything.
+# The real issue which was that the combine rule was looking through
+# non-truncates as if they were truncates and attempting to obtain the result
+# register. It would usually go on to make the right overall decision anyway but
+# would sometimes crash on things like (SOME_INTRINSIC imm). This test covers
+# the case that it would recover from.
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x0, $w1
+    %0:_(p0) = COPY $x0
+    %1:_(s8) = G_LOAD %0 :: (load 1 from %ir.addr)
+    ; CHECK: %1:_(s8) = G_LOAD %0(p0) :: (load 1 from %ir.addr)
+    G_STORE %1(s8), %0(p0) :: (store 1 into %ir.addr)
+    ; CHECK: G_STORE %1(s8), %0(p0) :: (store 1 into %ir.addr)
+...
 
+---
+name:            op0_isnt_a_reg
+# CHECK-LABEL: name: op0_isnt_a_reg
+# This test covers the variant of use_doesnt_def_anything that would crash.
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x0, $w1
+    %0:_(p0) = COPY $x0
+    %1:_(s8) = G_LOAD %0 :: (load 1 from %ir.addr)
+    ; CHECK: %1:_(s8) = G_LOAD %0(p0) :: (load 1 from %ir.addr)
+    G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.aarch64.hint), %1(s8)
+    ; CHECK: G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.aarch64.hint), %1(s8)
+...