This is an archive of the discontinued LLVM Phabricator instance.

Enable Shrink Wrapping for PPC64.
ClosedPublic

Authored by kbarton on Aug 6 2015, 2:00 PM.

Details

Summary

This patch will enable the shrink wrapping optimization for PPC64.

The changes in this patch are as follows:

  1. Modify the emitPrologue and emitEpilogue methods to work properly when the prologue and epilogue blocks are not the first/last blocks in the function
  2. Fix a bug in PPCEarlyReturn optimization caused by an empty entry block in the function
  3. Override the runShrinkWrap PredicateFtor (defined in TargetMachine) to check whether shrink wrapping should run:
    • Shrink wrapping will run on PPC64 (Little Endian and Big Endian) unless -enable-shrink-wrap=false is specified on command line

A new test case, ppc-shrink-wrapping.ll was created based on the existing shrink wrapping tests for x86, arm, and arm64.

Diff Detail

Event Timeline

kbarton updated this revision to Diff 31471.Aug 6 2015, 2:00 PM
kbarton retitled this revision from to Enable Shrink Wrapping for PPC64..
kbarton updated this object.
kbarton added reviewers: hfinkel, wschmidt, nemanjai, seurer.
kbarton added subscribers: llvm-commits, qcolombet.
hfinkel added inline comments.Aug 12 2015, 2:01 AM
lib/Target/PowerPC/PPCFrameLowering.cpp
1002–1025

Does RB stand for Return Block? Please use a more verbose name.

lib/Target/PowerPC/PPCTargetMachine.cpp
384

Don't add a dependency on EnableShrinkWrapOpt here. We would need to duplicate this checking logic in every target, and that's not a good design. (and the command-line argument itself is an internal detail of ShrinkWrap.cpp). Please have the code in ShrinkWrap.cpp (or Passes.cpp) check this before calling the target specific callback.

kbarton updated this revision to Diff 32053.Aug 13 2015, 7:57 AM

Addressed Hal's comments.

hfinkel added inline comments.Aug 14 2015, 8:28 AM
lib/Target/PowerPC/PPCTargetMachine.cpp
383

This is also not the right place for this logic. Please address my feedback here:

http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150810/292978.html

and then we can move forward with this.

kbarton added inline comments.Aug 25 2015, 8:19 AM
lib/Target/PowerPC/PPCTargetMachine.cpp
383

I've made these changed and posted in a new review: http://reviews.llvm.org/D12293

kbarton updated this revision to Diff 33608.Aug 31 2015, 12:08 PM

Changed the interface to override the new TargetFrameLowering::enableShrinkWrapping method defined in http://reviews.llvm.org/D12293.

hfinkel accepted this revision.Sep 8 2015, 4:47 PM
hfinkel edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Sep 8 2015, 4:47 PM
kbarton closed this revision.Sep 9 2015, 6:58 PM

Committed revision 247237.

Seems it might trigger crash in CodeGen/PowerPC/remap-crash.ll .
Investigating. I reproduced the crash on i686-linux (compiled with -m32).

See also

FYI, I used the patch to catch crash.

--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -866,6 +866,12 @@ void MachineInstr::addMemOperand(MachineFunction &MF,

 bool MachineInstr::hasPropertyInBundle(unsigned Mask, QueryType Type) const {
   assert(!isBundledWithPred() && "Must be called on bundle header");
+  if (const auto MBB = getParent()) {
+    const auto MF = MBB->getParent();
+    if (MF) {
+      assert(&MF->getSubtarget());
+    }
+  }
   for (MachineBasicBlock::const_instr_iterator MII = this;; ++MII) {
     if (MII->getDesc().getFlags() & Mask) {
       if (Type == AnyInBundle)
chapuni added inline comments.Sep 11 2015, 1:30 AM
lib/Target/PowerPC/PPCFrameLowering.cpp
1002–1025

It is unsafe if MBBI points end iterator.
Tweaked in r247395. Please reconfirm.