[PATCH] D27559: AArch64: Enable post-ra liveness updates

[Geoff Berry via Phabricator via llvm-commits]

gberry added inline comments.


================
Comment at: lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:901
+    // Hiding the LR use with RET_ReallyLR may lead to extra kills in the
+    // function and missing live-ins. We are fine in practice because LSR
+    // handling ensures the register value is restored before RET, but we have
----------------
Typo? "LSR" -> "LR"


================
Comment at: lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:902
+    // function and missing live-ins. We are fine in practice because LSR
+    // handling ensures the register value is restored before RET, but we have
+    // need the undef flag here to appease the MachineVerifier liveness checks.
----------------
Typo: remove "have"


================
Comment at: lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:906
         BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(AArch64::RET))
-          .addReg(AArch64::LR);
+          .addReg(AArch64::LR, RegState::Undef);
     transferImpOps(MI, MIB, MIB);
----------------
Is the reason you need LR to be undef here because we haven't added the corresponding def of LR to the function at this point?


================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:835
   }
+  StoreI->clearRegisterKills(StRt, TRI);
+
----------------
This is conservative, right?  Ideally you would copy the kill flags over to the last new instruction that uses StRt?


================
Comment at: test/CodeGen/AArch64/ldst-opt-dbg-limit.mir:128
     CFI_INSTRUCTION 0
-    STRWui killed %w1, killed %x0, 1 :: (store 4 into %ir.dst1)
+    STRWui %w1, %x0, 1 :: (store 4 into %ir.dst1)
     RET %lr
----------------
I don't understand why you removed the kills here.


================
Comment at: test/CodeGen/AArch64/loh.mir:57
     BL @extfunc, csr_aarch64_aapcs ; clobbers x9
-    %x9 = ADDXri %x9, target-flags(aarch64-pageoff) @g0, 0    
+    ; Liveness verification forces me to use 'undef' in front of %x9...
+    %x9 = ADDXri undef %x9, target-flags(aarch64-pageoff) @g0, 0
----------------
Maybe add "because of the above clobber of x9" to this comment?


================
Comment at: test/CodeGen/AArch64/loh.mir:103
     %x11 = ADRP target-flags(aarch64-page, aarch64-got) @g5
-    %s11 = LDRSui %x4, target-flags(aarch64-pageoff, aarch64-got) @g5
+    %s11 = LDRSui %x11, target-flags(aarch64-pageoff, aarch64-got) @g5
 
----------------
I don't follow this change of registers.


================
Comment at: test/CodeGen/AArch64/loh.mir:173
     %x10 = ADRP target-flags(aarch64-page) @g0
-    HINT 0, implicit def dead %x10 ; clobbers x10
     %x11 = ADDXri %x10, target-flags(aarch64-pageoff) @g0, 0
----------------
I don't follow this either.  It seems like you are changing what is being tested.


Repository:
  rL LLVM

https://reviews.llvm.org/D27559