[PATCH] D80886: [PHIElimination] Fix the killed flag for LowerPHINode()
Zhang Kang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun May 31 03:11:51 PDT 2020
ZhangKang created this revision.
ZhangKang added reviewers: hfinkel, echristo, efriedma, thegameg, qcolombet, bjope, dexonsmith, alex-t, stoklund, Nicola, nemanjai, PowerPC.
ZhangKang added a project: LLVM.
Herald added subscribers: wuzish, hiraditya.
ZhangKang added parent revisions: D80274: [MachineVerifier] Handle the PHI node for verifyLiveVariables(), D80538: [MachineVerifier] Add a new TiedOpsRewritten flag to fix verify two-address constraint error.
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.
Below is an example:
%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
After `phi-node-elimination` pass, we will get:
%38:g8rc_and_g8rc_nox0 = COPY killed %59
%37:gprc = COPY killed %57
%36:gprc = COPY killed %58
%35:gprc = COPY %57
It's obvious that, we have set the wrong killed flag for %57, if we enabled the verification for
`phi-node-elimination` pass, we will get the error `Using a killed virtual register`.
In fact, the right output should be this:
%38:g8rc_and_g8rc_nox0 = COPY killed %59
%37:gprc = COPY %57
%36:gprc = COPY killed %58
%35:gprc = COPY killed %57
This patch is to fix above error, it can fix below cases:
LLVM :: CodeGen/Hexagon/late_instr.ll
LLVM :: CodeGen/Hexagon/rdf-filter-defs.ll
LLVM :: CodeGen/Hexagon/swp-epilog-phi11.ll
LLVM :: CodeGen/Hexagon/swp-epilog-phi13.ll
LLVM :: CodeGen/Hexagon/swp-epilog-phi7.ll
LLVM :: CodeGen/Hexagon/swp-lots-deps.ll
LLVM :: CodeGen/Hexagon/swp-phi-def-use.ll
LLVM :: CodeGen/Hexagon/swp-phi-dep1.ll
LLVM :: CodeGen/Hexagon/swp-phi.ll
LLVM :: CodeGen/Hexagon/swp-stages5.ll
LLVM :: CodeGen/PowerPC/sms-phi-2.ll
LLVM :: CodeGen/Thumb/2009-08-12-ConstIslandAssert.ll
LLVM :: CodeGen/X86/2009-04-27-CoalescerAssert.ll
LLVM :: CodeGen/X86/avx512-masked_memop-16-8.ll
LLVM :: CodeGen/X86/crash.ll
LLVM :: CodeGen/X86/mmx-coalescing.ll
LLVM :: CodeGen/X86/statepoint-vector.ll
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80886
Files:
llvm/lib/CodeGen/PHIElimination.cpp
llvm/test/CodeGen/PowerPC/phi-eliminate.mir
Index: llvm/test/CodeGen/PowerPC/phi-eliminate.mir
===================================================================
--- llvm/test/CodeGen/PowerPC/phi-eliminate.mir
+++ llvm/test/CodeGen/PowerPC/phi-eliminate.mir
@@ -171,9 +171,9 @@
; 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 killed %57
+ ; CHECK: %37:gprc = COPY %57
; CHECK: %36:gprc = COPY killed %58
- ; CHECK: %35:gprc = COPY %57
+ ; 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
Index: llvm/lib/CodeGen/PHIElimination.cpp
===================================================================
--- llvm/lib/CodeGen/PHIElimination.cpp
+++ llvm/lib/CodeGen/PHIElimination.cpp
@@ -324,21 +324,43 @@
// 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 = VI.findKill(&MBB);
+ bool IsPHICopyAfterOldKill = false;
+
+ if (reusedIncoming && OldKill) {
+ // Calculate whether the PHICopy is after the OldKill.
+ // In general, the PHICopy is inserted as the first not-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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80886.267493.patch
Type: text/x-patch
Size: 3564 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200531/fa0b6305/attachment.bin>
More information about the llvm-commits
mailing list