[llvm] r267634 - [X86] Make sure it is safe to clobber EFLAGS, if need be, when choosing

Fri Apr 29 10:41:33 PDT 2016

This looks good to me.  Hans can you merge it?  If not, I will do it.

-Tom

On Fri, Apr 29, 2016 at 09:53:01AM -0700, Nadav Rotem wrote:
> LGTM. 
> 
> > On Apr 29, 2016, at 9:42 AM, Hans Wennborg <hans at chromium.org> wrote:
> > 
> > +Tom: this might be a good candidate for 3.8.1.
> > +Nadav for X86 ownership
> > 
> > On Tue, Apr 26, 2016 at 4:44 PM, Quentin Colombet via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >> Author: qcolombet
> >> Date: Tue Apr 26 18:44:14 2016
> >> New Revision: 267634
> >> 
> >> URL: http://llvm.org/viewvc/llvm-project?rev=267634&view=rev
> >> Log:
> >> [X86] Make sure it is safe to clobber EFLAGS, if need be, when choosing
> >> the prologue.
> >> 
> >> Do not use basic blocks that have EFLAGS live-in as prologue if we need
> >> to realign the stack. Realigning the stack uses AND instruction and this
> >> clobbers EFLAGS.
> >> 
> >> An other alternative would have been to save and restore EFLAGS around
> >> the stack realignment code, but this is likely inefficient.
> >> 
> >> Fixes PR27531.
> >> 
> >> Added:
> >>    llvm/trunk/test/CodeGen/X86/i686-win-shrink-wrapping.ll
> >> Modified:
> >>    llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> >>    llvm/trunk/lib/Target/X86/X86FrameLowering.h
> >> 
> >> Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.cpp?rev=267634&r1=267633&r2=267634&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)
> >> +++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Tue Apr 26 18:44:14 2016
> >> @@ -2609,6 +2609,12 @@ eliminateCallFramePseudoInstr(MachineFun
> >>   return I;
> >> }
> >> 
> >> +bool X86FrameLowering::canUseAsPrologue(const MachineBasicBlock &MBB) const {
> >> +  assert(MBB.getParent() && "Block is not attached to a function!");
> >> +  const MachineFunction &MF = *MBB.getParent();
> >> +  return !TRI->needsStackRealignment(MF) || !MBB.isLiveIn(X86::EFLAGS);
> >> +}
> >> +
> >> bool X86FrameLowering::canUseAsEpilogue(const MachineBasicBlock &MBB) const {
> >>   assert(MBB.getParent() && "Block is not attached to a function!");
> >> 
> >> 
> >> Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.h
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.h?rev=267634&r1=267633&r2=267634&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/lib/Target/X86/X86FrameLowering.h (original)
> >> +++ llvm/trunk/lib/Target/X86/X86FrameLowering.h Tue Apr 26 18:44:14 2016
> >> @@ -127,6 +127,16 @@ public:
> >>   /// Check that LEA can be used on SP in an epilogue sequence for \p MF.
> >>   bool canUseLEAForSPInEpilogue(const MachineFunction &MF) const;
> >> 
> >> +  /// Check whether or not the given \p MBB can be used as a prologue
> >> +  /// for the target.
> >> +  /// The prologue will be inserted first in this basic block.
> >> +  /// This method is used by the shrink-wrapping pass to decide if
> >> +  /// \p MBB will be correctly handled by the target.
> >> +  /// As soon as the target enable shrink-wrapping without overriding
> >> +  /// this method, we assume that each basic block is a valid
> >> +  /// prologue.
> >> +  bool canUseAsPrologue(const MachineBasicBlock &MBB) const override;
> >> +
> >>   /// 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.
> >> 
> >> Added: llvm/trunk/test/CodeGen/X86/i686-win-shrink-wrapping.ll
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/i686-win-shrink-wrapping.ll?rev=267634&view=auto
> >> ==============================================================================
> >> --- llvm/trunk/test/CodeGen/X86/i686-win-shrink-wrapping.ll (added)
> >> +++ llvm/trunk/test/CodeGen/X86/i686-win-shrink-wrapping.ll Tue Apr 26 18:44:14 2016
> >> @@ -0,0 +1,44 @@
> >> +; RUN: llc %s -o - -enable-shrink-wrap=true | FileCheck %s --check-prefix=CHECK --check-prefix=ENABLE
> >> +; RUN: llc %s -o - -enable-shrink-wrap=false | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLE
> >> +target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
> >> +target triple = "i686-pc-windows-msvc18.0.0"
> >> +
> >> +%struct.S = type { i32 }
> >> +
> >> +; Check that we do not use a basic block that has EFLAGS as live-in
> >> +; if we need to realign the stack.
> >> +; PR27531.
> >> +; CHECK-LABEL: stackRealignment:
> >> +; Prologue code.
> >> +; CHECK: pushl
> >> +; Make sure we actually perform some stack realignment.
> >> +; CHECK: andl ${{[-0-9]+}}, %esp
> >> +; This is the end of the entry block.
> >> +; The prologue should have happened before that point because past
> >> +; this point, EFLAGS is live.
> >> +; CHECK: jg
> >> +define x86_thiscallcc void @stackRealignment(%struct.S* %this) {
> >> +entry:
> >> +  %data = alloca [1 x i32], align 4
> >> +  %d = alloca double, align 8
> >> +  %tmp = bitcast [1 x i32]* %data to i8*
> >> +  %arrayinit.begin = getelementptr inbounds [1 x i32], [1 x i32]* %data, i32 0, i32 0
> >> +  %x_ = getelementptr inbounds %struct.S, %struct.S* %this, i32 0, i32 0
> >> +  %tmp1 = load i32, i32* %x_, align 4
> >> +  %cmp = icmp sgt i32 %tmp1, 32
> >> +  %cond = select i1 %cmp, i32 42, i32 128
> >> +  store i32 %cond, i32* %arrayinit.begin, align 4
> >> +  %cmp3 = icmp slt i32 %tmp1, 32
> >> +  br i1 %cmp3, label %cleanup, label %if.end
> >> +
> >> +if.end:                                           ; preds = %entry
> >> +  %tmp2 = bitcast double* %d to i8*
> >> +  call x86_thiscallcc void @bar(%struct.S* nonnull %this, i32* %arrayinit.begin, double* nonnull %d)
> >> +  br label %cleanup
> >> +
> >> +cleanup:                                          ; preds = %if.end, %entry
> >> +  ret void
> >> +}
> >> +
> >> +; Function Attrs: optsize
> >> +declare x86_thiscallcc void @bar(%struct.S*, i32*, double*)
> 