This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Fix wrong codegen when stack pointer has to realign performing dynalloc
ClosedPublic

Authored by lkail on Jul 20 2020, 5:27 AM.

Details

Summary

Current powerpc backend generates wrong code sequence if stack pointer has to realign if -fstack-clash-protection enabled. When probing dynamic stack allocation, current PREPARE_PROBED_ALLOCA takes NegSizeReg as input and returns FinalStackPtr. FinalStackPtr=StackPtr+ActualNegSize is calculated correctly, however code following PREPARE_PROBED_ALLOCA still uses value of NegSizeReg, which does not contain ActualNegSize if MaxAlign > TargetAlign, to calculate loop trip count and residual number of bytes.

This patch fixes https://bugs.llvm.org/show_bug.cgi?id=46759

Diff Detail

Event Timeline

lkail created this revision.Jul 20 2020, 5:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2020, 5:27 AM
lkail edited the summary of this revision. (Show Details)Jul 20 2020, 5:28 AM
lkail edited the summary of this revision. (Show Details)Jul 20 2020, 7:12 AM
jsji added a comment.Jul 20 2020, 7:20 AM

@lkail Can we split HasBP && MaxAlign > 1 situation into another patch? Thanks.

lkail updated this revision to Diff 279392.Jul 20 2020, 7:16 PM
lkail retitled this revision from [PowerPC] Fix wrong codegen when stack pointer has to realign to [PowerPC] Fix wrong codegen when stack pointer has to realign performing dynalloc.
lkail edited the summary of this revision. (Show Details)

Removed fix of wrong codegen in prologue, will post another patch to fix.

jsji accepted this revision as: jsji.Jul 21 2020, 8:16 PM

LGTM with some nits.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
11962

KillNegSizeReg only used once, I think we can inline it here. Also add some comments or move By introducing PREPARE_PROBED_ALLOCA_NEGSIZE_OPT... up here.

11975

Maybe rename PREPARE_PROBED_ALLOCA_NEGSIZE_OPT_64 to something like PREPARE_PROBED_ALLOCA_SAME_REG

11978

Maybe we can common up these .add..

This revision is now accepted and ready to land.Jul 21 2020, 8:16 PM
lkail updated this revision to Diff 279699.Jul 21 2020, 8:31 PM
This comment was removed by lkail.
lkail updated this revision to Diff 279700.Jul 21 2020, 8:39 PM

Simplified code according to @jsji 's comments.

This revision was automatically updated to reflect the committed changes.
hans added a comment.Jul 22 2020, 6:42 AM

Was I cc'd because you want this cherry-picked to the 11.x branch?

lkail added a comment.Jul 22 2020, 6:52 AM

Was I cc'd because you want this cherry-picked to the 11.x branch?

Yes. Together with https://reviews.llvm.org/D84218. Thanks, Hans.