[llvm] 0037a5f - [PHIElimination] Fix the killed flag for LowerPHINode()

Kang Zhang via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 30 01:19:20 PDT 2020


Author: Kang Zhang
Date: 2020-07-30T08:18:50Z
New Revision: 0037a5f894345eef5552d6620548cc8ad5900b41

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

LOG: [PHIElimination] Fix the killed flag for LowerPHINode()

Summary:
In the phi-node-elimination pass, we set the killed flag incorrectly.
When we eliminate the PHI node, we replace the PHI with a copy for the
incoming value.

Before this patch, we will set incoming value as killed(PHICopy). And
we will remove the killed flag from last using incoming value(OldKill).
This is correct, only if the new PHICopy is after the OldKill.

Reviewed By: bjope

Differential Revision: https://reviews.llvm.org/D80886

Added: 
    llvm/test/CodeGen/PowerPC/phi-eliminate.mir

Modified: 
    llvm/lib/CodeGen/PHIElimination.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp
index 311b87fa9e3b..521ae367cde3 100644
--- a/llvm/lib/CodeGen/PHIElimination.cpp
+++ b/llvm/lib/CodeGen/PHIElimination.cpp
@@ -324,21 +324,43 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
       // Increment use count of the newly created virtual register.
       LV->setPHIJoin(IncomingReg);
 
-      // When we are reusing the incoming register, it may already have been
-      // killed in this block. The old kill will also have been inserted at
-      // AfterPHIsIt, so it appears before the current PHICopy.
-      if (reusedIncoming)
-        if (MachineInstr *OldKill = VI.findKill(&MBB)) {
-          LLVM_DEBUG(dbgs() << "Remove old kill from " << *OldKill);
-          LV->removeVirtualRegisterKilled(IncomingReg, *OldKill);
-          LLVM_DEBUG(MBB.dump());
+      MachineInstr *OldKill = nullptr;
+      bool IsPHICopyAfterOldKill = false;
+
+      if (reusedIncoming && (OldKill = VI.findKill(&MBB))) {
+        // Calculate whether the PHICopy is after the OldKill.
+        // In general, the PHICopy is inserted as the first non-phi instruction
+        // by default, so it's before the OldKill. But some Target hooks for
+        // createPHIDestinationCopy() may modify the default insert position of
+        // PHICopy.
+        for (auto I = MBB.SkipPHIsAndLabels(MBB.begin()), E = MBB.end();
+             I != E; ++I) {
+          if (I == PHICopy)
+            break;
+
+          if (I == OldKill) {
+            IsPHICopyAfterOldKill = true;
+            break;
+          }
         }
+      }
+
+      // When we are reusing the incoming register and it has been marked killed
+      // by OldKill, if the PHICopy is after the OldKill, we should remove the
+      // killed flag from OldKill.
+      if (IsPHICopyAfterOldKill) {
+        LLVM_DEBUG(dbgs() << "Remove old kill from " << *OldKill);
+        LV->removeVirtualRegisterKilled(IncomingReg, *OldKill);
+        LLVM_DEBUG(MBB.dump());
+      }
 
-      // Add information to LiveVariables to know that the incoming value is
-      // killed.  Note that because the value is defined in several places (once
-      // each for each incoming block), the "def" block and instruction fields
-      // for the VarInfo is not filled in.
-      LV->addVirtualRegisterKilled(IncomingReg, *PHICopy);
+      // Add information to LiveVariables to know that the first used incoming
+      // value or the resued incoming value whose PHICopy is after the OldKIll
+      // is killed. Note that because the value is defined in several places
+      // (once each for each incoming block), the "def" block and instruction
+      // fields for the VarInfo is not filled in.
+      if (!OldKill || IsPHICopyAfterOldKill)
+        LV->addVirtualRegisterKilled(IncomingReg, *PHICopy);
     }
 
     // Since we are going to be deleting the PHI node, if it is the last use of

diff  --git a/llvm/test/CodeGen/PowerPC/phi-eliminate.mir b/llvm/test/CodeGen/PowerPC/phi-eliminate.mir
new file mode 100644
index 000000000000..e74ca4ca0e5d
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/phi-eliminate.mir
@@ -0,0 +1,295 @@
+# RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 %s -o - \
+# RUN:   -run-pass=livevars,phi-node-elimination | FileCheck %s
+
+--- |
+  define void @phi_eliminate(i32 %0, i32 %1, i8* %2) {
+    %scevgep3 = getelementptr i8, i8* %2, i64 undef
+    call void @llvm.set.loop.iterations.i64(i64 undef)
+    br label %4
+
+  4:                                                ; preds = %4, %3
+    %5 = phi i32 [ %8, %4 ], [ %0, %3 ]
+    %6 = phi i8* [ %scevgep3, %3 ], [ %7, %4 ]
+    %7 = getelementptr i8, i8* %6, i64 -1
+    %8 = sdiv i32 %5, %1
+    %9 = mul nsw i32 %8, %1
+    %10 = sub nsw i32 %5, %9
+    %11 = icmp ult i32 %10, 10
+    %12 = trunc i32 %10 to i8
+    %13 = select i1 %11, i8 48, i8 55
+    %14 = add i8 %13, %12
+    store i8 %14, i8* %7, align 1
+    %15 = call i1 @llvm.loop.decrement.i64(i64 1)
+    br i1 %15, label %4, label %16
+
+  16:                                               ; preds = %4
+    ret void
+  }
+
+  declare void @llvm.set.loop.iterations.i64(i64)
+
+  declare i1 @llvm.loop.decrement.i64(i64)
+
+  declare void @llvm.stackprotector(i8*, i8**)
+...
+---
+name:            phi_eliminate
+alignment:       16
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:
+  - { id: 0, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 1, class: gprc, preferred-register: '' }
+  - { id: 2, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 3, class: g8rc, preferred-register: '' }
+  - { id: 4, class: gprc, preferred-register: '' }
+  - { id: 5, class: g8rc, preferred-register: '' }
+  - { id: 6, class: g8rc, preferred-register: '' }
+  - { id: 7, class: g8rc, preferred-register: '' }
+  - { id: 8, class: gprc, preferred-register: '' }
+  - { id: 9, class: gprc, preferred-register: '' }
+  - { id: 10, class: g8rc, preferred-register: '' }
+  - { id: 11, class: gprc, preferred-register: '' }
+  - { id: 12, class: gprc, preferred-register: '' }
+  - { id: 13, class: crrc, preferred-register: '' }
+  - { id: 14, class: gprc_and_gprc_nor0, preferred-register: '' }
+  - { id: 15, class: gprc_and_gprc_nor0, preferred-register: '' }
+  - { id: 16, class: gprc, preferred-register: '' }
+  - { id: 17, class: gprc, preferred-register: '' }
+  - { id: 18, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 19, class: gprc, preferred-register: '' }
+  - { id: 20, class: gprc, preferred-register: '' }
+  - { id: 21, class: gprc, preferred-register: '' }
+  - { id: 22, class: crrc, preferred-register: '' }
+  - { id: 23, class: gprc, preferred-register: '' }
+  - { id: 24, class: gprc, preferred-register: '' }
+  - { id: 25, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 26, class: gprc, preferred-register: '' }
+  - { id: 27, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 28, class: gprc, preferred-register: '' }
+  - { id: 29, class: gprc, preferred-register: '' }
+  - { id: 30, class: gprc, preferred-register: '' }
+  - { id: 31, class: crrc, preferred-register: '' }
+  - { id: 32, class: gprc, preferred-register: '' }
+  - { id: 33, class: gprc, preferred-register: '' }
+  - { id: 34, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 35, class: gprc, preferred-register: '' }
+  - { id: 36, class: gprc, preferred-register: '' }
+  - { id: 37, class: gprc, preferred-register: '' }
+  - { id: 38, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 39, class: g8rc, preferred-register: '' }
+  - { id: 40, class: gprc, preferred-register: '' }
+  - { id: 41, class: gprc, preferred-register: '' }
+  - { id: 42, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 43, class: gprc, preferred-register: '' }
+  - { id: 44, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 45, class: gprc, preferred-register: '' }
+  - { id: 46, class: gprc, preferred-register: '' }
+  - { id: 47, class: crrc, preferred-register: '' }
+  - { id: 48, class: gprc, preferred-register: '' }
+  - { id: 49, class: gprc, preferred-register: '' }
+  - { id: 50, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 51, class: g8rc, preferred-register: '' }
+  - { id: 52, class: gprc, preferred-register: '' }
+  - { id: 53, class: gprc, preferred-register: '' }
+  - { id: 54, class: g8rc_and_g8rc_nox0, preferred-register: '' }
+  - { id: 55, class: gprc, preferred-register: '' }
+  - { id: 56, class: gprc, preferred-register: '' }
+liveins:
+  - { reg: '$x3', virtual-reg: '%5' }
+  - { reg: '$x4', virtual-reg: '%6' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    1
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 4294967295
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+callSites:       []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  ; CHECK-LABEL: name: phi_eliminate
+  ; CHECK: bb.0 (%ir-block.3):
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   liveins: $x3, $x4
+  ; CHECK:   %6:g8rc = COPY killed $x4
+  ; CHECK:   %5:g8rc = COPY killed $x3
+  ; CHECK:   %9:gprc = COPY killed %6.sub_32
+  ; CHECK:   %8:gprc = COPY killed %5.sub_32
+  ; CHECK:   MTCTR8loop undef %10:g8rc, implicit-def dead $ctr8
+  ; CHECK:   %14:gprc_and_gprc_nor0 = LI 55
+  ; CHECK:   %15:gprc_and_gprc_nor0 = LI 48
+
+  ; CHECK: bb.1 (%ir-block.4):
+  ; CHECK:   successors: %bb.2(0x40000000), %bb.7(0x40000000)
+  ; CHECK:   %19:gprc = DIVW %8, %9
+  ; CHECK:   BDNZ8 %bb.2, implicit-def $ctr8, implicit $ctr8
+
+  ; CHECK: bb.7:
+  ; CHECK:   successors: %bb.5(0x80000000)
+  ; CHECK:   %61:gprc = COPY killed %8
+  ; CHECK:   %62:g8rc_and_g8rc_nox0 = IMPLICIT_DEF
+  ; CHECK:   %63:gprc = COPY killed %19
+  ; CHECK:   B %bb.5
+
+  ; CHECK: bb.2 (%ir-block.4):
+  ; CHECK:   successors: %bb.3(0x40000000), %bb.4(0x40000000)
+  ; CHECK:   %20:gprc = nsw MULLW %19, %9
+  ; CHECK:   %21:gprc = SUBF killed %20, killed %8
+  ; CHECK:   %22:crrc = CMPLWI %21, 10
+  ; CHECK:   %23:gprc = ISEL %15, %14, killed %22.sub_lt
+  ; CEHCK:   %24:gprc = ADD4 killed %23, killed %21
+  ; CHECK:   %25:g8rc_and_g8rc_nox0 = STBU killed %24, -1, undef %0:g8rc_and_g8rc_nox0 :: (store 1 into %ir.7)
+  ; CHECK:   %26:gprc = DIVW %19, %9
+  ; CHECK:   %57:gprc = COPY killed %26
+  ; CHECK:   %58:gprc = COPY %19
+  ; CHECK:   %59:g8rc_and_g8rc_nox0 = COPY killed %25
+  ; CHECK:   %60:gprc = COPY killed %19
+  ; CHECK:   BDZ8 %bb.4, implicit-def $ctr8, implicit $ctr8
+  ; CHECK:   B %bb.3
+
+  ; CHECK: bb.3 (%ir-block.4):
+  ; CHECK:   successors: %bb.3(0x7c000000), %bb.4(0x04000000)
+  ; CHECK:   %38:g8rc_and_g8rc_nox0 = COPY killed %59
+  ; CHECK:   %37:gprc = COPY %57
+  ; CHECK:   %36:gprc = COPY killed %58
+  ; CHECK:   %35:gprc = COPY killed %57
+  ; CHECK:   %27:g8rc_and_g8rc_nox0 = COPY killed %38
+  ; CHECK:   %56:gprc = COPY %35
+  ; CHECK:   %28:gprc = DIVW %56, %9
+  ; CHECK:   %29:gprc = nsw MULLW killed %37, %9
+  ; CHECK:   %30:gprc = SUBF killed %29, killed %36
+  ; CHECK:   %31:crrc = CMPLWI %30, 10
+  ; CHECK:   %32:gprc = ISEL %15, %14, killed %31.sub_lt
+  ; CHECK:   %33:gprc = ADD4 killed %32, killed %30
+  ; CHECK:   %34:g8rc_and_g8rc_nox0 = STBU killed %33, -1, killed %27 :: (store unknown-size into %ir.7, align 1)
+  ; CHECK:   %57:gprc = COPY killed %28
+  ; CHECK:   %58:gprc = COPY killed %35
+  ; CHECK:   %59:g8rc_and_g8rc_nox0 = COPY killed %34
+  ; CHECK:   %60:gprc = COPY killed %56
+  ; CHECK:   BDNZ8 %bb.3, implicit-def $ctr8, implicit $ctr8
+  ; CHECK:   B %bb.4
+
+  ; CHECK: bb.4:
+  ; CHECK:   successors: %bb.5(0x80000000)
+  ; CHECK:   %44:g8rc_and_g8rc_nox0 = COPY killed %59
+  ; CHECK:   %43:gprc = COPY killed %57
+  ; CHECK:   %41:gprc = COPY killed %60
+  ; CHECK:   %39:g8rc = COPY killed %44
+  ; CHECK:   %61:gprc = COPY killed %41
+  ; CHECK:   %62:g8rc_and_g8rc_nox0 = COPY killed %39
+  ; CHECK:   %63:gprc = COPY killed %43
+
+  ; CHECK: bb.5:
+  ; CHECK:   successors: %bb.6(0x80000000)
+  ; CHECK:   %55:gprc = COPY killed %63
+  ; CHECK:   %54:g8rc_and_g8rc_nox0 = COPY killed %62
+  ; CHECK:   %53:gprc = COPY killed %61
+  ; CHECK:   %45:gprc = nsw MULLW killed %55, killed %9
+  ; CHECK:   %46:gprc = SUBF killed %45, killed %53
+  ; CHECK:   %47:crrc = CMPLWI %46, 10
+  ; CHECK:   %48:gprc = ISEL killed %15, killed %14, killed %47.sub_lt
+  ; CHECK:   %49:gprc = ADD4 killed %48, killed %46
+  ; CHECK:   dead %50:g8rc_and_g8rc_nox0 = STBU killed %49, -1, killed %54 :: (store unknown-size into %ir.7, align 1)
+  ; CHECK:   B %bb.6
+
+  ; CHECK: bb.6 (%ir-block.16):
+  ; CHECK:   BLR8 implicit $lr8, implicit $rm
+
+  bb.0 (%ir-block.3):
+    successors: %bb.1(0x80000000)
+    liveins: $x3, $x4
+
+    %6:g8rc = COPY killed $x4
+    %5:g8rc = COPY killed $x3
+    %9:gprc = COPY killed %6.sub_32
+    %8:gprc = COPY killed %5.sub_32
+    MTCTR8loop undef %10:g8rc, implicit-def dead $ctr8
+    %14:gprc_and_gprc_nor0 = LI 55
+    %15:gprc_and_gprc_nor0 = LI 48
+
+  bb.1 (%ir-block.4):
+    successors: %bb.2(0x40000000), %bb.5(0x40000000)
+
+    %19:gprc = DIVW %8, %9
+    BDZ8 %bb.5, implicit-def $ctr8, implicit $ctr8
+    B %bb.2
+
+  bb.2 (%ir-block.4):
+    successors: %bb.3(0x40000000), %bb.4(0x40000000)
+
+    %20:gprc = nsw MULLW %19, %9
+    %21:gprc = SUBF killed %20, killed %8
+    %22:crrc = CMPLWI %21, 10
+    %23:gprc = ISEL %15, %14, killed %22.sub_lt
+    %24:gprc = ADD4 killed %23, killed %21
+    %25:g8rc_and_g8rc_nox0 = STBU killed %24, -1, undef %0:g8rc_and_g8rc_nox0 :: (store 1 into %ir.7)
+    %26:gprc = DIVW %19, %9
+    BDZ8 %bb.4, implicit-def $ctr8, implicit $ctr8
+    B %bb.3
+
+  bb.3 (%ir-block.4):
+    successors: %bb.3(0x7c000000), %bb.4(0x04000000)
+
+    %35:gprc = PHI %26, %bb.2, %28, %bb.3
+    %36:gprc = PHI %19, %bb.2, %35, %bb.3
+    %37:gprc = PHI %26, %bb.2, %28, %bb.3
+    %38:g8rc_and_g8rc_nox0 = PHI %25, %bb.2, %34, %bb.3
+    %27:g8rc_and_g8rc_nox0 = COPY killed %38
+    %56:gprc = COPY %35
+    %28:gprc = DIVW %56, %9
+    %29:gprc = nsw MULLW killed %37, %9
+    %30:gprc = SUBF killed %29, killed %36
+    %31:crrc = CMPLWI %30, 10
+    %32:gprc = ISEL %15, %14, killed %31.sub_lt
+    %33:gprc = ADD4 killed %32, killed %30
+    %34:g8rc_and_g8rc_nox0 = STBU killed %33, -1, killed %27 :: (store unknown-size into %ir.7, align 1)
+    BDNZ8 %bb.3, implicit-def $ctr8, implicit $ctr8
+    B %bb.4
+
+  bb.4:
+    successors: %bb.5(0x80000000)
+
+    %41:gprc = PHI %19, %bb.2, %56, %bb.3
+    %43:gprc = PHI %26, %bb.2, %28, %bb.3
+    %44:g8rc_and_g8rc_nox0 = PHI %25, %bb.2, %34, %bb.3
+    %39:g8rc = COPY killed %44
+
+  bb.5:
+    successors: %bb.6(0x80000000)
+
+    %53:gprc = PHI %8, %bb.1, %41, %bb.4
+    %54:g8rc_and_g8rc_nox0 = PHI undef %0:g8rc_and_g8rc_nox0, %bb.1, %39, %bb.4
+    %55:gprc = PHI %19, %bb.1, %43, %bb.4
+    %45:gprc = nsw MULLW killed %55, killed %9
+    %46:gprc = SUBF killed %45, killed %53
+    %47:crrc = CMPLWI %46, 10
+    %48:gprc = ISEL killed %15, killed %14, killed %47.sub_lt
+    %49:gprc = ADD4 killed %48, killed %46
+    dead %50:g8rc_and_g8rc_nox0 = STBU killed %49, -1, killed %54 :: (store unknown-size into %ir.7, align 1)
+    B %bb.6
+
+  bb.6 (%ir-block.16):
+    BLR8 implicit $lr8, implicit $rm
+
+...


        


More information about the llvm-commits mailing list