Skip to content

Commit 80d3a80

Browse files
authored
[NFC][x86] Cleanup X86FrameLowering::emitSPUpdate (#156948)
I was trying to understand the cases where `emitSPUpdate` may use a `push` to adjust the stack pointer and realized that the code was more complex than it needed to be. * `Chunk` is redundant as it is set to `MaxSPChunk` and never modified. * The only time we use the `push` optimization is if `Offset` is equal to `SlotSize` (as `SlotSize` is never as large as `MaxSPChunk`, so will never equal if `std::min` returned that value). * If we use the `push` optimization, then we've finished adjusting the stack and can return early instead of continuing the loop.
1 parent 9e7b21a commit 80d3a80

File tree

1 file changed

+9
-10
lines changed

1 file changed

+9
-10
lines changed

llvm/lib/Target/X86/X86FrameLowering.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ X86FrameLowering::X86FrameLowering(const X86Subtarget &STI,
5353
STI(STI), TII(*STI.getInstrInfo()), TRI(STI.getRegisterInfo()) {
5454
// Cache a bunch of frame-related predicates for this subtarget.
5555
SlotSize = TRI->getSlotSize();
56+
assert(SlotSize == 4 || SlotSize == 8);
5657
Is64Bit = STI.is64Bit();
5758
IsLP64 = STI.isTarget64BitLP64();
5859
// standard x86_64 uses 64-bit frame/stack pointers, x32 - 32-bit.
@@ -224,7 +225,7 @@ flagsNeedToBePreservedBeforeTheTerminators(const MachineBasicBlock &MBB) {
224225
return false;
225226
}
226227

227-
constexpr int64_t MaxSPChunk = (1LL << 31) - 1;
228+
constexpr uint64_t MaxSPChunk = (1ULL << 31) - 1;
228229

229230
/// emitSPUpdate - Emit a series of instructions to increment / decrement the
230231
/// stack pointer by a constant value.
@@ -245,8 +246,6 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
245246
return;
246247
}
247248

248-
uint64_t Chunk = MaxSPChunk;
249-
250249
MachineFunction &MF = *MBB.getParent();
251250
const X86Subtarget &STI = MF.getSubtarget<X86Subtarget>();
252251
const X86TargetLowering &TLI = *STI.getTargetLowering();
@@ -260,7 +259,7 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
260259
// loop, by inlineStackProbe().
261260
BuildMI(MBB, MBBI, DL, TII.get(X86::STACKALLOC_W_PROBING)).addImm(Offset);
262261
return;
263-
} else if (Offset > Chunk) {
262+
} else if (Offset > MaxSPChunk) {
264263
// Rather than emit a long series of instructions for large offsets,
265264
// load the offset into a register and do one sub/add
266265
unsigned Reg = 0;
@@ -284,7 +283,7 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
284283
.addReg(Reg);
285284
MI->getOperand(3).setIsDead(); // The EFLAGS implicit def is dead.
286285
return;
287-
} else if (Offset > 8 * Chunk) {
286+
} else if (Offset > 8 * MaxSPChunk) {
288287
// If we would need more than 8 add or sub instructions (a >16GB stack
289288
// frame), it's worth spilling RAX to materialize this immediate.
290289
// pushq %rax
@@ -322,8 +321,7 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
322321
}
323322

324323
while (Offset) {
325-
uint64_t ThisVal = std::min(Offset, Chunk);
326-
if (ThisVal == SlotSize) {
324+
if (Offset == SlotSize) {
327325
// Use push / pop for slot sized adjustments as a size optimization. We
328326
// need to find a dead register when using pop.
329327
unsigned Reg = isSub ? (unsigned)(Is64Bit ? X86::RAX : X86::EAX)
@@ -334,11 +332,12 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
334332
BuildMI(MBB, MBBI, DL, TII.get(Opc))
335333
.addReg(Reg, getDefRegState(!isSub) | getUndefRegState(isSub))
336334
.setMIFlag(Flag);
337-
Offset -= ThisVal;
338-
continue;
335+
return;
339336
}
340337
}
341338

339+
uint64_t ThisVal = std::min(Offset, MaxSPChunk);
340+
342341
BuildStackAdjustment(MBB, MBBI, DL, isSub ? -ThisVal : ThisVal, InEpilogue)
343342
.setMIFlag(Flag);
344343

@@ -445,7 +444,7 @@ int64_t X86FrameLowering::mergeSPUpdates(MachineBasicBlock &MBB,
445444
return CalcNewOffset(0);
446445

447446
FoundStackAdjust(PI, Offset);
448-
if (std::abs((int64_t)CalcNewOffset(Offset)) < MaxSPChunk)
447+
if ((uint64_t)std::abs((int64_t)CalcNewOffset(Offset)) < MaxSPChunk)
449448
break;
450449

451450
if (doMergeWithPrevious ? (PI == MBB.begin()) : (PI == MBB.end()))

0 commit comments

Comments
 (0)