[PATCH] D42534: [mips] Compute MaxCallFrame size early on

[Simon Dardis via Phabricator via llvm-commits]

sdardis added a comment.

In https://reviews.llvm.org/D42534#990829, @MatzeB wrote:

> In https://reviews.llvm.org/D42534#990357, @sdardis wrote:
>
> > +CC'ing @MatzeB for additional insight on what needs to be changed.
> >
> > Found the bug. What's occurring with the debug information test failures with this patch, is that during the insertion of debug instructions for function arguments, it has to determine which register to assign to the debug instruction, see SelectionDAGISel.cpp:505. Here it asks for the frame pointer register which for some RISC like targets by the size of the frame for the function.
> >
> > It seems to me that we need to compute the max call frame size before that point (SelectionDAGISel.cpp:505) for targets which use call frame setup and destroy pseudo instructions, this however requires changes for every target which implements finalizeLowering() to compute the maxCallFrameSize.
>
>
> Yeah code calling hasFP() early in the process is a real annoyance. You can find me wiggling in the review of https://reviews.llvm.org/D32622 too. At the time it seemed to me like it only influences scheduling heuristic so I went with this hack in the ARM target:
>
>   // hasFP ends up calling getMaxCallFrameComputed() which may not be
>   // available when getPressureLimit() is called as part of
>   // ScheduleDAGRRList.
>   bool HasFP = MF.getFrameInfo().isMaxCallFrameSizeComputed()
>                ? TFI->hasFP(MF) : true;
>   
>
> instead.
>
> But it looks like you found a bigger problem than the heuristic. If you can find ways for ISel to not do this anymore I would certainly apreciate it!


Sketch solution: Extend the ISD Opcodes to include READ_FP, as SelectionDAG cannot determine ahead of time if function uses a frame pointer (in the general case). The node takes a single argument, the input chain and returns a pair of results, the current value of the frame pointer and the output chain. The semantics of the node are that it returns the value of the frame pointer at that point if the function is using a frame pointer or the current value of stack pointer if the function does not use a frame pointer. The pseudo instruction is then eliminated early during the finalization of instruction selection by replacing it either with a ISD::COPY or by replacing the users of the result ISD::READ_FP with the physreg directly which now can be computed correctly.

ISel can then use the result of this node without needing the maximum frame size during the selection of any block but then ScheduleDAGRRList would still get the wrong result as it only sees one block at a time. We could track if we have seen basic blocks whose instructions contain call setup/destroy instructions which trigger the creation of reserved call frame or some other target specific requirement for a frame pointer. This however would mean that getPressureLimit would see an inconsistent result for a function.


https://reviews.llvm.org/D42534