[llvm] r268336 - LivePhysRegs: Automatically determine presence of pristine regs.

Andrew Trick via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 15:55:29 PDT 2016


> On May 4, 2016, at 12:03 PM, Philip Reames <listmail at philipreames.com> wrote:
> 
> Attn: Andy and Juergen.

The stackmap format already has an entry for the callee saves that are preserved in the current frame. Including all of those that aren't preserved in every stackmap entry would be innefficient.

StackMapLiveness should not include pristine regs. That needs to be clearly documented both in the pass and in the stackmap format documentation.

Philip, are you planning to use this feature, and could you add comments?

If not, then Matthias, could you work with me to get this documented?

-Andy

> On 05/02/2016 05:08 PM, Matthias Braun via llvm-commits wrote:
>> Author: matze
>> Date: Mon May  2 19:08:46 2016
>> New Revision: 268336
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=268336&view=rev
>> Log:
>> LivePhysRegs: Automatically determine presence of pristine regs.
>> 
>> Remove the AddPristinesAndCSRs parameters from
>> addLiveIns()/addLiveOuts().
>> 
>> We need to respect pristine registers after prologue epilogue insertion,
>> Seeing that we got this wrong in at least two commits already, we should
>> rather pay the small price to query MachineFrameInfo for it.
>> 
>> There are three cases that did not set AddPristineAndCSRs to true even
>> after register allocation:
>> - ExecutionDepsFix: live-out registers are used as a hint that the
>>   register is used soon. This is not true for pristine registers so
>>   use the new addLiveOutsNoPristines() to maintain this behaviour.
>> - SystemZShortenInst: Not setting AddPristineAndCSRs to true looks like
>>   a bug, should do the right thing automatically now.
>> - StackMapLivenessAnalysis: Not adding pristine registers looks like a
>>   bug to me. Added a FIXME comment but maintain the current behaviour
>>   as a change may need to get coordinated with GC runtimes.
>> 
>> Modified:
>>     llvm/trunk/include/llvm/CodeGen/LivePhysRegs.h
>>     llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp
>>     llvm/trunk/lib/CodeGen/LivePhysRegs.cpp
>>     llvm/trunk/lib/CodeGen/StackMapLivenessAnalysis.cpp
>>     llvm/trunk/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
>>     llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp
>>     llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
>>     llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp
>>     llvm/trunk/lib/Target/X86/X86FixupBWInsts.cpp
>>     llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
>> 
>> Modified: llvm/trunk/include/llvm/CodeGen/LivePhysRegs.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/LivePhysRegs.h?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/CodeGen/LivePhysRegs.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/LivePhysRegs.h Mon May  2 19:08:46 2016
>> @@ -112,15 +112,20 @@ public:
>>    void stepForward(const MachineInstr &MI,
>>          SmallVectorImpl<std::pair<unsigned, const MachineOperand*>> &Clobbers);
>>  -  /// \brief Adds all live-in registers of basic block @p MBB; After prologue/
>> -  /// epilogue insertion \p AddPristines should be set to true to insert the
>> +  /// Adds all live-in registers of basic block @p MBB.
>> +  /// Live in registers are the registers in the blocks live-in list and the
>>    /// pristine registers.
>> -  void addLiveIns(const MachineBasicBlock *MBB, bool AddPristines = false);
>> +  void addLiveIns(const MachineBasicBlock *MBB);
>>  -  /// \brief Adds all live-out registers of basic block @p MBB; After prologue/
>> -  /// epilogue insertion \p AddPristinesAndCSRs should be set to true.
>> -  void addLiveOuts(const MachineBasicBlock *MBB,
>> -                   bool AddPristinesAndCSRs = false);
>> +  /// Adds all live-out registers of basic block @p MBB.
>> +  /// Live out registers are the union of the live-in registers of the successor
>> +  /// blocks and pristine registers. Live out registers of the end block are the
>> +  /// callee saved registers.
>> +  void addLiveOuts(const MachineBasicBlock *MBB);
>> +
>> +  /// Like addLiveOuts() but does not add pristine registers/callee saved
>> +  /// registers.
>> +  void addLiveOutsNoPristines(const MachineBasicBlock *MBB);
>>      typedef SparseSet<unsigned>::const_iterator const_iterator;
>>    const_iterator begin() const { return LiveRegs.begin(); }
>> 
>> Modified: llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp Mon May  2 19:08:46 2016
>> @@ -558,7 +558,9 @@ void ExeDepsFix::processUndefReads(Machi
>>      // Collect this block's live out register units.
>>    LiveRegSet.init(TRI);
>> -  LiveRegSet.addLiveOuts(MBB);
>> +  // We do not need to care about pristine registers as they are just preserved
>> +  // but not actually used in the function.
>> +  LiveRegSet.addLiveOutsNoPristines(MBB);
>>      MachineInstr *UndefMI = UndefReads.back().first;
>>    unsigned OpIdx = UndefReads.back().second;
>> 
>> Modified: llvm/trunk/lib/CodeGen/LivePhysRegs.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LivePhysRegs.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/LivePhysRegs.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/LivePhysRegs.cpp Mon May  2 19:08:46 2016
>> @@ -135,22 +135,25 @@ static void addLiveIns(LivePhysRegs &Liv
>>  /// Add pristine registers to the given \p LiveRegs. This function removes
>>  /// actually saved callee save registers when \p InPrologueEpilogue is false.
>>  static void addPristines(LivePhysRegs &LiveRegs, const MachineFunction &MF,
>> +                         const MachineFrameInfo &MFI,
>>                           const TargetRegisterInfo &TRI) {
>> -  const MachineFrameInfo &MFI = *MF.getFrameInfo();
>> -  if (!MFI.isCalleeSavedInfoValid())
>> -    return;
>> -
>>    for (const MCPhysReg *CSR = TRI.getCalleeSavedRegs(&MF); CSR && *CSR; ++CSR)
>>      LiveRegs.addReg(*CSR);
>>    for (const CalleeSavedInfo &Info : MFI.getCalleeSavedInfo())
>>      LiveRegs.removeReg(Info.getReg());
>>  }
>>  -void LivePhysRegs::addLiveOuts(const MachineBasicBlock *MBB,
>> -                               bool AddPristinesAndCSRs) {
>> -  if (AddPristinesAndCSRs) {
>> -    const MachineFunction &MF = *MBB->getParent();
>> -    addPristines(*this, MF, *TRI);
>> +void LivePhysRegs::addLiveOutsNoPristines(const MachineBasicBlock *MBB) {
>> +  // To get the live-outs we simply merge the live-ins of all successors.
>> +  for (const MachineBasicBlock *Succ : MBB->successors())
>> +    ::addLiveIns(*this, *Succ);
>> +}
>> +
>> +void LivePhysRegs::addLiveOuts(const MachineBasicBlock *MBB) {
>> +  const MachineFunction &MF = *MBB->getParent();
>> +  const MachineFrameInfo &MFI = *MF.getFrameInfo();
>> +  if (MFI.isCalleeSavedInfoValid()) {
>> +    addPristines(*this, MF, MFI, *TRI);
>>      if (MBB->isReturnBlock()) {
>>        // The return block has no successors whose live-ins we could merge
>>        // below. So instead we add the callee saved registers manually.
>> @@ -159,16 +162,13 @@ void LivePhysRegs::addLiveOuts(const Mac
>>      }
>>    }
>>  -  // To get the live-outs we simply merge the live-ins of all successors.
>> -  for (const MachineBasicBlock *Succ : MBB->successors())
>> -    ::addLiveIns(*this, *Succ);
>> +  addLiveOutsNoPristines(MBB);
>>  }
>>  -void LivePhysRegs::addLiveIns(const MachineBasicBlock *MBB,
>> -                              bool AddPristines) {
>> -  if (AddPristines) {
>> -    const MachineFunction &MF = *MBB->getParent();
>> -    addPristines(*this, MF, *TRI);
>> -  }
>> +void LivePhysRegs::addLiveIns(const MachineBasicBlock *MBB) {
>> +  const MachineFunction &MF = *MBB->getParent();
>> +  const MachineFrameInfo &MFI = *MF.getFrameInfo();
>> +  if (MFI.isCalleeSavedInfoValid())
>> +    addPristines(*this, MF, MFI, *TRI);
>>    ::addLiveIns(*this, *MBB);
>>  }
>> 
>> Modified: llvm/trunk/lib/CodeGen/StackMapLivenessAnalysis.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/StackMapLivenessAnalysis.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/StackMapLivenessAnalysis.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/StackMapLivenessAnalysis.cpp Mon May  2 19:08:46 2016
>> @@ -127,7 +127,8 @@ bool StackMapLiveness::calculateLiveness
>>    for (auto &MBB : MF) {
>>      DEBUG(dbgs() << "****** BB " << MBB.getName() << " ******\n");
>>      LiveRegs.init(TRI);
>> -    LiveRegs.addLiveOuts(&MBB);
>> +    // FIXME: This should probably be addLiveOuts().
>> +    LiveRegs.addLiveOutsNoPristines(&MBB);
>>      bool HasStackMap = false;
>>      // Reverse iterate over all instructions and add the current live register
>>      // set to an instruction if we encounter a patchpoint instruction.
>> 
>> Modified: llvm/trunk/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp (original)
>> +++ llvm/trunk/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp Mon May  2 19:08:46 2016
>> @@ -607,7 +607,7 @@ bool AArch64ExpandPseudo::expandCMP_SWAP
>>    MachineOperand &New = MI.getOperand(4);
>>      LivePhysRegs LiveRegs(&TII->getRegisterInfo());
>> -  LiveRegs.addLiveOuts(&MBB, /*AddPristinesAndCSRs=*/true);
>> +  LiveRegs.addLiveOuts(&MBB);
>>    for (auto I = std::prev(MBB.end()); I != MBBI; --I)
>>      LiveRegs.stepBackward(*I);
>>  @@ -685,7 +685,7 @@ bool AArch64ExpandPseudo::expandCMP_SWAP
>>    MachineOperand &NewHi = MI.getOperand(7);
>>      LivePhysRegs LiveRegs(&TII->getRegisterInfo());
>> -  LiveRegs.addLiveOuts(&MBB, /*AddPristinesAndCSRs=*/true);
>> +  LiveRegs.addLiveOuts(&MBB);
>>    for (auto I = std::prev(MBB.end()); I != MBBI; --I)
>>      LiveRegs.stepBackward(*I);
>>  
>> Modified: llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp Mon May  2 19:08:46 2016
>> @@ -775,7 +775,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
>>    MachineOperand &New = MI.getOperand(4);
>>      LivePhysRegs LiveRegs(&TII->getRegisterInfo());
>> -  LiveRegs.addLiveOuts(&MBB, /*AddPristinesAndCSRs=*/true);
>> +  LiveRegs.addLiveOuts(&MBB);
>>    for (auto I = std::prev(MBB.end()); I != MBBI; --I)
>>      LiveRegs.stepBackward(*I);
>>  @@ -897,7 +897,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
>>    unsigned DesiredHi = TRI->getSubReg(Desired.getReg(), ARM::gsub_1);
>>      LivePhysRegs LiveRegs(&TII->getRegisterInfo());
>> -  LiveRegs.addLiveOuts(&MBB, /*AddPristinesAndCSRs=*/true);
>> +  LiveRegs.addLiveOuts(&MBB);
>>    for (auto I = std::prev(MBB.end()); I != MBBI; --I)
>>      LiveRegs.stepBackward(*I);
>>  
>> Modified: llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp Mon May  2 19:08:46 2016
>> @@ -566,7 +566,7 @@ void ARMLoadStoreOpt::moveLiveRegsBefore
>>    // Initialize if we never queried in this block.
>>    if (!LiveRegsValid) {
>>      LiveRegs.init(TRI);
>> -    LiveRegs.addLiveOuts(&MBB, true);
>> +    LiveRegs.addLiveOuts(&MBB);
>>      LiveRegPos = MBB.end();
>>      LiveRegsValid = true;
>>    }
>> 
>> Modified: llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp Mon May  2 19:08:46 2016
>> @@ -467,7 +467,7 @@ bool Thumb1FrameLowering::emitPopSpecial
>>    // Look for a temporary register to use.
>>    // First, compute the liveness information.
>>    LivePhysRegs UsedRegs(STI.getRegisterInfo());
>> -  UsedRegs.addLiveOuts(&MBB, /*AddPristines*/ true);
>> +  UsedRegs.addLiveOuts(&MBB);
>>    // The semantic of pristines changed recently and now,
>>    // the callee-saved registers that are touched in the function
>>    // are not part of the pristines set anymore.
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86FixupBWInsts.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FixupBWInsts.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86FixupBWInsts.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86FixupBWInsts.cpp Mon May  2 19:08:46 2016
>> @@ -245,7 +245,7 @@ void FixupBWInstPass::processBasicBlock(
>>    // to update this for each instruction.
>>    LiveRegs.clear();
>>    // We run after PEI, so we need to AddPristinesAndCSRs.
>> -  LiveRegs.addLiveOuts(&MBB, /*AddPristinesAndCSRs=*/true);
>> +  LiveRegs.addLiveOuts(&MBB);
>>      for (auto I = MBB.rbegin(); I != MBB.rend(); ++I) {
>>      MachineInstr *NewMI = nullptr;
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=268336&r1=268335&r2=268336&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Mon May  2 19:08:46 2016
>> @@ -4537,7 +4537,7 @@ void X86InstrInfo::copyPhysReg(MachineBa
>>      // as this is usually wrong to read an undef value.
>>      if (MachineBasicBlock::LQR_Unknown == LQR) {
>>        LivePhysRegs LPR(&getRegisterInfo());
>> -      LPR.addLiveOuts(&MBB, /*AddPristinesAndCSRs*/ true);
>> +      LPR.addLiveOuts(&MBB);
>>        MachineBasicBlock::iterator I = MBB.end();
>>        while (I != MI) {
>>          --I;
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 



More information about the llvm-commits mailing list