This is an archive of the discontinued LLVM Phabricator instance.

[X86] Fix stack probe issue on windows32.
ClosedPublic

Authored by LuoYuanke on Aug 7 2019, 7:54 PM.

Details

Summary

On windows if the frame size exceed 4096 bytes, compiler need to generate a call to _alloca_probe. X86CallFrameOptimization pass changes the reserved stack size and cause of stack probe function not be inserted. This patch fix the issue by detecting the call frame size, if the size exceed 4096 bytes, drop X86CallFrameOptimization.

Diff Detail

Repository
rL LLVM

Event Timeline

LuoYuanke created this revision.Aug 7 2019, 7:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 7 2019, 7:54 PM
rnk added a comment.Aug 8 2019, 10:13 AM

Seems reasonable, but I have some suggestions.

llvm/lib/Target/X86/X86CallFrameOptimization.cpp
247–254 ↗(On Diff #214056)

I see this pattern for calculating the stack probe size is already duplicated twice. Please factor it out into a method of X86ISelLowering, perhaps next to getStackProbeSymbolName and update the two callers along with this one.

258–261 ↗(On Diff #214056)

I see that TargetInstrInfo uses the phrase "frame size" to refer to the size of the arguments used to pass call arguments, but I think that's easily confused with the size of the current function's stack frame. Maybe this comment would be clearer:

If any call allocates more argument stack memory than the stack probe size, don't do this optimization. Otherwise, this pass would need to synthesize additional stack probe calls to allocate memory for arguments.

263 ↗(On Diff #214056)

Please move this check into the isLegal method where other preconditions are checked.

LuoYuanke updated this revision to Diff 214347.Aug 9 2019, 3:53 AM

Update the patch corresponding to Reid's comments.

LuoYuanke marked 3 inline comments as done.Aug 9 2019, 3:55 AM
rnk accepted this revision.Aug 9 2019, 1:10 PM

lgtm, thanks!

This revision is now accepted and ready to land.Aug 9 2019, 1:10 PM
This revision was automatically updated to reflect the committed changes.