[llvm] r343693 - [globalisel][combines] Don't sink G_TRUNC down to use if that use is a G_PHI

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 3 08:43:39 PDT 2018


Author: dsanders
Date: Wed Oct  3 08:43:39 2018
New Revision: 343693

URL: http://llvm.org/viewvc/llvm-project?rev=343693&view=rev
Log:
[globalisel][combines] Don't sink G_TRUNC down to use if that use is a G_PHI

This fixes a problem where the register allocator fails to eliminate a PHI
because there's a non-PHI in the middle of the PHI instructions at the start
of a BB.

This G_TRUNC can be better placed but this at least fixes the correctness issue
quickly. I'll follow up with a patch to the verifier to catch this kind of bug
in future.


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

Modified: llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp?rev=343693&r1=343692&r2=343693&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/CombinerHelper.cpp Wed Oct  3 08:43:39 2018
@@ -154,12 +154,18 @@ bool CombinerHelper::tryCombineExtending
   assert(Preferred.Ty != LoadValueTy && "Extending to same type?");
 
   // Rewrite the load and schedule the canonical use for erasure.
-  const auto TruncateUse = [](MachineIRBuilder &Builder, MachineOperand &UseMO,
-                              unsigned DstReg, unsigned SrcReg) {
+  const auto TruncateUse = [&MI](MachineIRBuilder &Builder,
+                                 MachineOperand &UseMO, unsigned DstReg,
+                                 unsigned SrcReg) {
     MachineInstr &UseMI = *UseMO.getParent();
     MachineBasicBlock &UseMBB = *UseMI.getParent();
 
     Builder.setInsertPt(UseMBB, MachineBasicBlock::iterator(UseMI));
+
+    if (UseMI.isPHI())
+      Builder.setInsertPt(*MI.getParent(),
+                          std::next(MachineBasicBlock::iterator(MI)));
+
     Builder.buildTrunc(DstReg, SrcReg);
   };
 

Added: 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=343693&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir (added)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir Wed Oct  3 08:43:39 2018
@@ -0,0 +1,111 @@
+# RUN: llc -O0 -run-pass=aarch64-prelegalizer-combiner -global-isel %s -o - | FileCheck %s
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64--"
+  define void @multiple_copies(i8* %addr) {
+  entry:
+    br i1 0, label %if, label %else
+  if:
+    br label %exit
+  else:
+    br label %exit
+  exit:
+    ret void
+  }
+
+  define void @sink_to_phi(i8* %addr) {
+  entry:
+    br i1 0, label %if, label %exit
+  if:
+    br label %exit
+  exit:
+    ret void
+  }
+...
+
+---
+name:            multiple_copies
+# CHECK-LABEL: name: multiple_copies
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x0, $w1
+    successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)
+    ; CHECK: [[T0:%[0-9]+]]:_(s32) = G_SEXTLOAD
+    %0:_(p0) = COPY $x0
+    %1:_(s32) = COPY $w1
+    %2:_(s8) = G_LOAD %0 :: (load 1 from %ir.addr)
+    %3:_(s32) = G_SEXT %2
+    %4:_(s32) = G_CONSTANT i32 1
+    %5:_(s1) = G_ICMP intpred(ne), %1:_(s32), %4:_
+    G_BRCOND %5:_(s1), %bb.1
+    G_BR %bb.2.else
+  bb.1.if:
+  ; CHECK: bb.1.if:
+    successors: %bb.3(0x80000000)
+    %10:_(s8) = G_CONSTANT i8 1
+    ; CHECK: [[T1:%[0-9]+]]:_(s8) = G_TRUNC [[T0]](s32)
+    %6:_(s8) = G_ADD %2, %10
+    ; CHECK: [[T2:%[0-9]+]]:_(s8) = G_ADD [[T1]], {{.*}}
+    G_BR %bb.3.exit
+  bb.2.else:
+  ; CHECK: bb.2.else:
+    successors: %bb.3(0x80000000)
+    %11:_(s8) = G_CONSTANT i8 1
+    ; CHECK: [[T3:%[0-9]+]]:_(s8) = G_TRUNC [[T0]](s32)
+    %7:_(s8) = G_SUB %2, %11
+    ; CHECK: [[T4:%[0-9]+]]:_(s8) = G_SUB [[T3]], {{.*}}
+    G_BR %bb.3.exit
+  bb.3.exit:
+  ; CHECK: bb.3.exit:
+    %8:_(s8) = G_PHI %6:_(s8), %bb.1, %7:_(s8), %bb.2
+    ; CHECK: [[T5:%[0-9]+]]:_(s8) = G_PHI [[T2]](s8), %bb.1, [[T4]](s8)
+    %9:_(s32) = G_ZEXT %8
+    ; CHECK: [[T6:%[0-9]+]]:_(s32) = G_ZEXT [[T5]](s8)
+    ; CHECK: $w0 = COPY [[T0]](s32)
+    ; CHECK: $w1 = COPY [[T6]](s32)
+    $w0 = COPY %3
+    $w1 = COPY %9
+...
+
+---
+name:            sink_to_phi
+# CHECK-LABEL: name: sink_to_phi
+# 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:
+    liveins: $x0, $w1
+    successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)
+    ; CHECK: [[T0:%[0-9]+]]:_(s32) = G_SEXTLOAD
+    %0:_(p0) = COPY $x0
+    %1:_(s32) = COPY $w1
+    %2:_(s8) = G_LOAD %0 :: (load 1 from %ir.addr)
+    ; CHECK: [[T4:%[0-9]+]]:_(s8) = G_TRUNC [[T0]](s32)
+    %3:_(s32) = G_SEXT %2
+    %4:_(s32) = G_CONSTANT i32 1
+    %5:_(s1) = G_ICMP intpred(ne), %1:_(s32), %4:_
+    G_BRCOND %5:_(s1), %bb.1
+    G_BR %bb.2.exit
+  bb.1.if:
+  ; CHECK: bb.1.if:
+    successors: %bb.2(0x80000000)
+    %10:_(s8) = G_CONSTANT i8 1
+    ; CHECK: [[T1:%[0-9]+]]:_(s8) = G_TRUNC [[T0]](s32)
+    %6:_(s8) = G_ADD %2, %10
+    ; CHECK: [[T2:%[0-9]+]]:_(s8) = G_ADD [[T1]], {{.*}}
+    G_BR %bb.2.exit
+  bb.2.exit:
+  ; CHECK: bb.2.exit:
+    %8:_(s8) = G_PHI %6:_(s8), %bb.1, %2:_(s8), %bb.0
+    ; CHECK: [[T5:%[0-9]+]]:_(s8) = G_PHI [[T2]](s8), %bb.1, [[T4]](s8)
+    %9:_(s32) = G_ZEXT %8
+    ; CHECK: [[T6:%[0-9]+]]:_(s32) = G_ZEXT [[T5]](s8)
+    ; CHECK: $w0 = COPY [[T0]](s32)
+    ; CHECK: $w1 = COPY [[T6]](s32)
+    $w0 = COPY %3
+    $w1 = COPY %9
+...




More information about the llvm-commits mailing list