This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Probe the gap between stackptr and realigned stackptr
ClosedPublic

Authored by lkail on Sep 22 2020, 2:35 AM.

Details

Summary

During reviewing https://reviews.llvm.org/D84419, @efriedma mentioned the gap between realigned stack pointer and origin stack pointer should be probed too whatever the alignment is. This patch fixes the issue for PPC64.

Diff Detail

Event Timeline

lkail created this revision.Sep 22 2020, 2:35 AM
lkail requested review of this revision.Sep 22 2020, 2:35 AM
lkail updated this revision to Diff 294530.Sep 26 2020, 7:02 PM

Fix format and add more tests.

I cannot tell for the arch-specific part, but I see you've been reusing the different stack / alignemnt . probe test case from x86, so at least these configurations seems to be covered. +1 for me.

jsji accepted this revision as: jsji.Nov 24 2020, 8:00 AM

LGTM. Some comment update please.

llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
1210

getStackProbeSize returns unsigned.
So this should still be safe , as we won't have large uint64 ProbeSize that may become negative when casting to int64_t.
However, this looks like a potential trap to me.

Maybe we should use unsigned for ProbeSize?

1282

Document ScratchReg and TempReg please. And also document the assumption, eg: stackptr is in BPReg etc.

1286

// ScratchReg = stackptr % align

1291

// TempReg = stackptr - (stackptr % align)

1295

// ScratchReg = (stackptr % align) % probesize

This revision is now accepted and ready to land.Nov 24 2020, 8:00 AM
lkail updated this revision to Diff 307498.Nov 24 2020, 7:22 PM

Refined comments.

lkail marked 5 inline comments as done.Nov 24 2020, 7:24 PM
This revision was landed with ongoing or failed builds.Nov 24 2020, 11:01 PM
This revision was automatically updated to reflect the committed changes.