[PATCH] D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN

Shiva Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 22 23:05:34 PDT 2018


shiva0217 added a comment.

In https://reviews.llvm.org/D53485#1271895, @MatzeB wrote:

> What about `MachineVerifier::verifyStackFrame()`? That currently rejects any function with nested callframe setups. You will probably see your testcases fail when running with `-verify-machineinstrs`.


Hi @MatzeB, our testcases originally not nested, the error caused by the scheduler tries to interleaving callframe setups. The patch guides the scheduler not try to interleave the callframe setups. The nested callframe setups won't occur in our cases with the patch. So they could pass the MachineVerifier.

In https://reviews.llvm.org/D53485#1271922, @MatzeB wrote:

> The code in `PEI::replaceFrameIndices` also seems sketchy to me as in case of nested callframes it would set `InsideCallSequence` to false after the inner ADJCALLSTACKUP (though at a first glance I wasn't sure yet why that variable exists at all, and why we would not track sp adjustments outside of callframe setups...)


It seems that this part of the code assume nested callframes shouldn't occur. I think the sp adjustments outside the ADJCALLSTACKDOWN/ADJCALLSTACKUP will track by  `if(TII.isFrameInstr(*I) SPAdj += TII.getSPAdjust(*I);` Because prolog/epilog instructions should be setting with FrameSetup/FrameDestroy MIflags, so `if(TII.isFrameInstr(*I)` will return true.


Repository:
  rL LLVM

https://reviews.llvm.org/D53485





More information about the llvm-commits mailing list