[llvm] r224745 - Make musttail more robust for vector types on x86

[Aaron Ballman]

On Tue, Dec 23, 2014 at 9:40 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Mon, Dec 22, 2014 at 6:58 PM, Reid Kleckner <reid at kleckner.net> wrote:
>> Author: rnk
>> Date: Mon Dec 22 17:58:37 2014
>> New Revision: 224745
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=224745&view=rev
>> Log:
>> Make musttail more robust for vector types on x86
>>
>> Previously I tried to plug musttail into the existing vararg lowering
>> code. That turned out to be a mistake, because non-vararg calls use
>> significantly different register lowering, even on x86. For example, AVX
>> vectors are usually passed in registers to normal functions and memory
>> to vararg functions.  Now musttail uses a completely separate lowering.
>>
>> Hopefully this can be used as the basis for non-x86 perfect forwarding.
>>
>> Reviewers: majnemer
>>
>> Differential Revision: http://reviews.llvm.org/D6156
>>
>> Added:
>>     llvm/trunk/test/CodeGen/X86/musttail-fastcall.ll
>> Modified:
>>     llvm/trunk/include/llvm/CodeGen/CallingConvLower.h
>>     llvm/trunk/lib/CodeGen/CallingConvLower.cpp
>>     llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>>     llvm/trunk/lib/Target/X86/X86MachineFunctionInfo.h
>>     llvm/trunk/test/CodeGen/X86/musttail-varargs.ll
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/CallingConvLower.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/CallingConvLower.h?rev=224745&r1=224744&r2=224745&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/CodeGen/CallingConvLower.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/CallingConvLower.h Mon Dec 22 17:58:37 2014
>> @@ -158,6 +158,16 @@ public:
>>    }
>>  };
>>
>> +/// Describes a register that needs to be forwarded from the prologue to a
>> +/// musttail call.
>> +struct ForwardedRegister {
>> +  ForwardedRegister(unsigned VReg, MCPhysReg PReg, MVT VT)
>> +      : VReg(VReg), PReg(PReg), VT(VT) {}
>> +  unsigned VReg;
>> +  MCPhysReg PReg;
>> +  MVT VT;
>> +};
>> +
>>  /// CCAssignFn - This function assigns a location for Val, updating State to
>>  /// reflect the change.  It returns 'true' if it failed to handle Val.
>>  typedef bool CCAssignFn(unsigned ValNo, MVT ValVT,
>> @@ -470,6 +480,19 @@ public:
>>      return PendingLocs;
>>    }
>>
>> +  /// Compute the remaining unused register parameters that would be used for
>> +  /// the given value type. This is useful when varargs are passed in the
>> +  /// registers that normal prototyped parameters would be passed in, or for
>> +  /// implementing perfect forwarding.
>> +  void getRemainingRegParmsForType(SmallVectorImpl<MCPhysReg> &Regs, MVT VT,
>> +                                   CCAssignFn Fn);
>> +
>> +  /// Compute the set of registers that need to be preserved and forwarded to
>> +  /// any musttail calls.
>> +  void analyzeMustTailForwardedRegisters(
>> +      SmallVectorImpl<ForwardedRegister> &Forwards, ArrayRef<MVT> RegParmTypes,
>> +      CCAssignFn Fn);
>> +
>>  private:
>>    /// MarkAllocated - Mark a register and all of its aliases as allocated.
>>    void MarkAllocated(unsigned Reg);
>>
>> Modified: llvm/trunk/lib/CodeGen/CallingConvLower.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CallingConvLower.cpp?rev=224745&r1=224744&r2=224745&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/CallingConvLower.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/CallingConvLower.cpp Mon Dec 22 17:58:37 2014
>> @@ -14,9 +14,11 @@
>>
>>  #include "llvm/CodeGen/CallingConvLower.h"
>>  #include "llvm/CodeGen/MachineFrameInfo.h"
>> +#include "llvm/CodeGen/MachineRegisterInfo.h"
>>  #include "llvm/IR/DataLayout.h"
>>  #include "llvm/Support/Debug.h"
>>  #include "llvm/Support/ErrorHandling.h"
>> +#include "llvm/Support/SaveAndRestore.h"
>>  #include "llvm/Support/raw_ostream.h"
>>  #include "llvm/Target/TargetLowering.h"
>>  #include "llvm/Target/TargetRegisterInfo.h"
>> @@ -178,3 +180,57 @@ void CCState::AnalyzeCallResult(MVT VT,
>>      llvm_unreachable(nullptr);
>>    }
>>  }
>> +
>> +void CCState::getRemainingRegParmsForType(SmallVectorImpl<MCPhysReg> &Regs,
>> +                                          MVT VT, CCAssignFn Fn) {
>> +  unsigned SavedStackOffset = StackOffset;
>> +  unsigned NumLocs = Locs.size();
>> +
>> +  // Allocate something of this value type repeatedly with just the inreg flag
>> +  // set until we get assigned a location in memory.
>> +  ISD::ArgFlagsTy Flags;
>> +  Flags.setInReg();
>> +  bool HaveRegParm = true;
>> +  while (HaveRegParm) {
>> +    if (Fn(0, VT, VT, CCValAssign::Full, Flags, *this)) {
>> +#ifndef NDEBUG
>> +      dbgs() << "Call has unhandled type " << EVT(VT).getEVTString()
>> +             << " while computing remaining regparms\n";
>> +#endif
>> +      llvm_unreachable(nullptr);
>> +    }
>> +    HaveRegParm = Locs.back().isRegLoc();
>> +  }
>> +
>> +  // Copy all the registers from the value locations we added.
>> +  assert(NumLocs < Locs.size() && "CC assignment failed to add location");
>> +  for (unsigned I = NumLocs, E = Locs.size(); I != E; ++I)
>> +    if (Locs[I].isRegLoc())
>> +      Regs.push_back(MCPhysReg(Locs[I].getLocReg()));
>> +
>> +  // Clear the assigned values and stack memory. We leave the registers marked
>> +  // as allocated so that future queries don't return the same registers, i.e.
>> +  // when i64 and f64 are both passed in GPRs.
>> +  StackOffset = SavedStackOffset;
>> +  Locs.resize(NumLocs);
>> +}
>> +
>> +void CCState::analyzeMustTailForwardedRegisters(
>> +    SmallVectorImpl<ForwardedRegister> &Forwards, ArrayRef<MVT> RegParmTypes,
>> +    CCAssignFn Fn) {
>> +  // Oftentimes calling conventions will not user register parameters for
>> +  // variadic functions, so we need to assume we're not variadic so that we get
>> +  // all the registers that might be used in a non-variadic call.
>> +  SaveAndRestore<bool> SavedVarArg(IsVarArg, false);
>> +
>> +  for (MVT RegVT : RegParmTypes) {
>> +    SmallVector<MCPhysReg, 8> RemainingRegs;
>> +    getRemainingRegParmsForType(RemainingRegs, RegVT, Fn);
>> +    const TargetLowering *TL = MF.getSubtarget().getTargetLowering();
>> +    const TargetRegisterClass *RC = TL->getRegClassFor(RegVT);
>> +    for (MCPhysReg PReg : RemainingRegs) {
>> +      unsigned VReg = MF.addLiveIn(PReg, RC);
>> +      Forwards.push_back(ForwardedRegister(VReg, PReg, RegVT));
>> +    }
>> +  }
>> +}
>>
>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=224745&r1=224744&r2=224745&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon Dec 22 17:58:37 2014
>> @@ -2549,11 +2549,19 @@ X86TargetLowering::LowerFormalArguments(
>>          MFI->CreateFixedObject(1, StackSize, true));
>>    }
>>
>> +  // Figure out if XMM registers are in use.
>> +  bool HaveXMMArgs = Is64Bit && !IsWin64;
>
> I am now getting: warning: variable ‘HaveXMMArgs’ set but not used
> [-Wunused-but-set-variable]
>
> Was there a purpose for HaveXMMArgs, or can this simply be removed?

I have removed it in r224888 -- if it was meant to serve a purpose, it
will be trivial to revive.

~Aaron