[x86][WIP] CSR alignment, dead code?

[reames]

Summary

Fair warning: I'm touching code I'm not familiar with. My approach might be completely wrong here.

As far as I can tell, the line in question has no effect on runs with realignment enabled; the alignment of the spill slots is already handled within the MachineFrameInfo::CreateFixedSpillSlot.

With realignment disabled (via no-realign-stack), this appears to bypass the checking in MFI which prevents the stack alignment from being increased for CSRs. What I'm really not clear on is why this doesn't trip the assert in ensureMaxAlignment. Help?

(To exercise this code, you have to disable fp elimination. Anyone know why we unconditional realign with a FP and ignore it otherwise? I don't see any profitability decision being made...)

[ahatanak]

MachineFrameInfo::ensureMaxAlignment(unsigned Align) updates MaxAlignment so that it is as least as large as Align. When you say "the line in question has no effect", does that mean MaxAlignment never gets updated or MaxAlignment never gets used after line 1346?

[reames]

In D12824#244770, @ahatanak wrote:

MachineFrameInfo::ensureMaxAlignment(unsigned Align) updates MaxAlignment so that it is as least as large as Align. When you say "the line in question has no effect", does that mean MaxAlignment never gets updated or MaxAlignment never gets used after line 1346?

Ok, it looks like I was wrong here. I'd misread the code; most of the CreateX functions on MFI call ensureMaxAlignment, CreateFixedSplilStackObject does not. I had thought line was redundant, but I was wrong. So, that invalidates the actual patch.

However, can you possibly explain *why* we're setting the max alignment even when we can't actually realign the stack? That seems odd to say the least.

[ahatanak]

I don't know why. It looks like it isn't really needed when the stack can't be realigned but it's updating MaxAlignment anyway.

Regarding your other question, my understanding is that when frame pointer elimination is enabled, the stack can't be realigned to spill registers because the frame pointer register is not available. If you look at X86RegisterInfo::canRealignStack, it returns true only when the frame pointer register is reserved. That doesn't happen when frame pointer elimination is enabled.

lib/Target/X86/X86FrameLowering.cpp

for (unsigned i = CSI.size(); i != 0; --i) {
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
     // ensure alignment
     SpillSlotOffset -= std::abs(SpillSlotOffset) % RC->getAlignment();
     // spill into slot
     SpillSlotOffset -= RC->getSize();
     int SlotIndex =
         MFI->CreateFixedSpillStackObject(RC->getSize(), SpillSlotOffset);
     CSI[i - 1].setFrameIdx(SlotIndex);
-    MFI->ensureMaxAlignment(RC->getAlignment());
   }
 
   return true;
 }
 
 bool X86FrameLowering::spillCalleeSavedRegisters(
     MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
     const std::vector<CalleeSavedInfo> &CSI,