[PATCH] D66319: [ARM] Preserve liveness in ARMConstantIslands.

[Eli Friedman via Phabricator via llvm-commits]

efriedma created this revision.
efriedma added a reviewer: SjoerdMeijer.
Herald added subscribers: kristof.beyls, javed.absar.
Herald added a project: LLVM.

We currently don't use liveness information after this point, but it can be useful to catch bugs using -verify-machineinstrs, and optimizations could potentially use this information in the future.

This is the liveness patch I mentioned in D66243 <https://reviews.llvm.org/D66243>, that caught a miscompile.


Repository:
  rL LLVM

https://reviews.llvm.org/D66319

Files:
  lib/Target/ARM/ARMConstantIslandPass.cpp
  test/CodeGen/ARM/constant-island-movwt.mir


Index: test/CodeGen/ARM/constant-island-movwt.mir
===================================================================
--- test/CodeGen/ARM/constant-island-movwt.mir
+++ test/CodeGen/ARM/constant-island-movwt.mir
@@ -898,5 +898,7 @@
 # CHECK-NEXT:    CONSTPOOL_ENTRY 1, %const.0, 4
 # CHECK-NEXT: {{^  $}}
 # CHECK-NEXT:   bb.2.entry (align 1):
+# CHECK-NEXT:   liveins:
+# CHECK-NEXT: {{^  $}}
 # CHECK-NEXT:     $r5 = t2MOVi16 target-flags(arm-lo16) @.str.84, 14, $noreg
 # CHECK-NEXT:     $r5 = t2MOVTi16 $r5, target-flags(arm-hi16) @.str.84, 14, $noreg
Index: lib/Target/ARM/ARMConstantIslandPass.cpp
===================================================================
--- lib/Target/ARM/ARMConstantIslandPass.cpp
+++ lib/Target/ARM/ARMConstantIslandPass.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -357,9 +358,6 @@
   HasFarJump = false;
   bool GenerateTBB = isThumb2 || (isThumb1 && SynthesizeThumb1TBB);
 
-  // This pass invalidates liveness information when it splits basic blocks.
-  MF->getRegInfo().invalidateLiveness();
-
   // Renumber all of the machine basic blocks in the function, guaranteeing that
   // the numbers agree with the position of the block in the function.
   MF->RenumberBlocks();
@@ -880,6 +878,13 @@
 MachineBasicBlock *ARMConstantIslands::splitBlockBeforeInstr(MachineInstr *MI) {
   MachineBasicBlock *OrigBB = MI->getParent();
 
+  // Collect liveness information at I.
+  LivePhysRegs LRs(*MF->getSubtarget().getRegisterInfo());
+  LRs.addLiveOuts(*OrigBB);
+  auto LivenessEnd = ++MachineBasicBlock::iterator(MI).getReverse();
+  for (MachineInstr &LiveMI : make_range(OrigBB->rbegin(), LivenessEnd))
+    LRs.stepBackward(LiveMI);
+
   // Create a new MBB for the code after the OrigBB.
   MachineBasicBlock *NewBB =
     MF->CreateMachineBasicBlock(OrigBB->getBasicBlock());
@@ -908,6 +913,12 @@
   // OrigBB branches to NewBB.
   OrigBB->addSuccessor(NewBB);
 
+  // Update live-in information in the new block.
+  MachineRegisterInfo &MRI = MF->getRegInfo();
+  for (MCPhysReg L : LRs)
+    if (!MRI.isReserved(L))
+      NewBB->addLiveIn(L);
+
   // Update internal data structures to account for the newly inserted MBB.
   // This is almost the same as updateForInsertedWaterBlock, except that
   // the Water goes after OrigBB, not NewBB.
@@ -2331,6 +2342,10 @@
   MachineFunction::iterator MBBI = ++JTBB->getIterator();
   MF->insert(MBBI, NewBB);
 
+  // Copy live-in information to new block.
+  for (const MachineBasicBlock::RegisterMaskPair &RegMaskPair : BB->liveins())
+    NewBB->addLiveIn(RegMaskPair);
+
   // Add an unconditional branch from NewBB to BB.
   // There doesn't seem to be meaningful DebugInfo available; this doesn't
   // correspond directly to anything in the source.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66319.215495.patch
Type: text/x-patch
Size: 2999 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190815/bcc09d76/attachment.bin>