[X86] Enable shrink-wrapping by default

[qcolombet]

Summary

Hi,

This patch enables shrink-wrapping by default for x86.
Both Chandler and I have tested shrink-wrapping and no correctness problems came out or runtime regressions.
Although the gain may be small, the code looks nicer with shrink-wrapping enabled and the performance could just be better.

Finally, I’ve run the gdb test suite with and without shrink-wrapping enabled for an optimized compiler and the results were identical.

For the record shrink-wrapping was introduced after this review:
http://reviews.llvm.org/D9210

More information on Shrink-Wrapping:
http://lists.llvm.org/pipermail/llvm-dev/2015-May/085220.html

OK to commit?

Thanks,
-Quentin

[qcolombet]

For the record, the changes in the tests are because shrink-wrapping moves the related code into if blocks.

[majnemer]

https://llvm.org/bugs/show_bug.cgi?id=24193 is still outstanding.

[qcolombet]

Hi David,

https://llvm.org/bugs/show_bug.cgi?id=24193 is still outstanding.

Oh that’s right, I forgot about this one.
An easy fix would be to disallow that for win64 for now.
Anyway, I’ll look into it.

Thanks for reminding me!

Q.

[qcolombet]

Update patch after PR24193 is fixed, i.e., now the Win64 test case did not show any changes.

[qcolombet]

Alright, PR24193 is now fixed.

Is it OK to commit?

Thanks,
-Quentin

[qcolombet]

ping.

[spatel]

LGTM.

[qcolombet]

Thanks Sanjay and Nadav!

I am holding on the commit, it seems enabling shrink-wrapping will cause problem with EH emission and although I believe the problem may not be shrink-wrapping, we will need to fix it first.

For more details, see
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312215.html

Cheers,
-Quentin

[qcolombet]

Alright, everything fixed.

Committed revision 253528.

lib/Target/X86/X86FrameLowering.h

public:
 
   /// Check whether or not the given \p MBB can be used as a epilogue
   /// for the target.
   /// The epilogue will be inserted before the first terminator of that block.
   /// This method is used by the shrink-wrapping pass to decide if
   /// \p MBB will be correctly handled by the target.
   bool canUseAsEpilogue(const MachineBasicBlock &MBB) const override;
 
+  /// Returns true if the target will correctly handle shrink wrapping.
+  bool enableShrinkWrapping(const MachineFunction &MF) const override {
+    return true;
+  }
+
   /// Wraps up getting a CFI index and building a MachineInstr for it.
   void BuildCFI(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
                 DebugLoc DL, MCCFIInstruction CFIInst) const;
 
 private:
   uint64_t calculateMaxStackAlign(const MachineFunction &MF) const;
 
   /// Aligns the stack pointer by ANDing it with -MaxAlign.

test/CodeGen/X86/tail-opts.ll

 
 ; If one tail merging candidate falls through into the other,
 ; tail merging is likely profitable regardless of how few
 ; instructions are involved. This function should have only
 ; one ret instruction.
 
 ; CHECK-LABEL: foo:
 ; CHECK:        callq func
-; CHECK-NEXT: .LBB4_2:
 ; CHECK-NEXT:   popq
+; CHECK-NEXT: .LBB4_2:
 ; CHECK-NEXT:   ret
 
 define void @foo(i1* %V) nounwind {
 entry:
   %t0 = icmp eq i1* %V, null
   br i1 %t0, label %return, label %bb
 
 bb: