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

[Nadav Rotem via llvm-commits]

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*)