[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