[PATCH] D151276: Weaken MFI Max Call Frame Size Assertion
Sergei Barannikov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 23 21:28:08 PDT 2023
barannikov88 added a comment.
In D151276#4366847 <https://reviews.llvm.org/D151276#4366847>, @oskarwirga wrote:
> In D151276#4366712 <https://reviews.llvm.org/D151276#4366712>, @barannikov88 wrote:
>
>> I'm not sure this is the correct fix. The assertion specifically checks that MaxCallFrameSize and AdjustsStack computed here are the same as computed by `MachineFrameInfo::computeMaxCallFrameSize`.
>> Have you been able to figure out why the results of the calculations are different?
>
> Yes, bcl5980 commented on the github issue:
>
>> In the AArch64TargetLowering::finalizeLowering it call MFI.computeMaxCallFrameSize(MF) to get MaxCallFrameSize
>> MaxCallFrameSize is 8 because a ADJCALLSTACKDOWN/ADJCALLSTACKUP in bb.2
>> And in the pass unreachable-mbb-elimination, bb.2 will be removed
>> Then the assert will be triggered in the pass prologepilog
>> Not sure how to fix the assert
>
> I was able to fix this assertion by appending to `UnreachableMachineBlockElim::runOnMachineFunction`
>
> MachineFrameInfo &MFI = F.getFrameInfo();
> MFI.computeMaxCallFrameSize(F);
>
> but was advised to weaken this assertion by @MatzeB with his reasoning being that the assertion was mostly to catch cases where the value //grew// later, rather than shrank.
In this case AdjustsStack on line 352 should be initialized to false and `MFI.adjustsStack() == AdjustsStack` should also be removed making this assertion kind of useless.
If @MatzeB is happy with it, I'm too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151276/new/
https://reviews.llvm.org/D151276
More information about the llvm-commits
mailing list