This is an archive of the discontinued LLVM Phabricator instance.

[MachineFrameInfo][RISCV] Call ensureStackAlignment for objects created with scalable vector stack id.
ClosedPublic

Authored by craig.topper on Oct 13 2022, 1:35 PM.

Details

Summary

This is an alternative to fix PR57939 for RISC-V. It definitely
can be argued that the stack temporaries for RISC-V are being created
with an unnecessarily large alignment. But ignoring the alignment
in MachineFrameInfo also seems bad.

Looking at the test update that go with the current ID==0 check,
it was intending to exclude things like the NoAlloc stackid. So I'm
not sure if scalable vectors are intentionally being excluded.

Diff Detail

Event Timeline

craig.topper created this revision.Oct 13 2022, 1:35 PM
craig.topper requested review of this revision.Oct 13 2022, 1:35 PM
frasercrmck added a comment.EditedOct 19 2022, 7:59 AM

Seems like a reasonable approach to me. I feel it's too difficult for targets to account for the alignment of non-default stack IDs themselves. And that's without your suspicions that scalable vectors may not even be knowingly excluded at the present time.

llvm/include/llvm/CodeGen/MachineFrameInfo.h
504

Maybe a shared helper function like contributesToMaxAlignment(uint8_t ID) to group code together?

Rebase tests. Still need to address review feedback

Add function as suggested.

reames accepted this revision.Oct 20 2022, 11:30 AM

LGTM, this looks to be in the spirit of the existing code.

In a follow up, we should probably adjust estimateStackSize in an analogous manner.

This revision is now accepted and ready to land.Oct 20 2022, 11:30 AM