[llvm] r343821 - [globalisel][combine] Fix a rare crash when encountering an instruction whose op0 isn't a reg

[Daniel Sanders via llvm-commits]

Author: dsanders
Date: Thu Oct  4 14:44:32 2018
New Revision: 343821

URL: http://llvm.org/viewvc/llvm-project?rev=343821&view=rev
Log:
[globalisel][combine] Fix a rare crash when encountering an instruction whose op0 isn't a reg

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.


Modified:
    llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp
    llvm/trunk/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir

Modified: llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp?rev=343821&r1=343820&r2=343821&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp Thu Oct  4 14:44:32 2018
@@ -60,12 +60,8 @@ PreferredTuple ChoosePreferredUse(Prefer
                                   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::tryCombineExtending
   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

Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir?rev=343821&r1=343820&r2=343821&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir Thu Oct  4 14:44:32 2018
@@ -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)
+...