[PATCH] D78717: [SystemZ] Implement -fstack-clash-protection

[serge via Phabricator via llvm-commits]

serge-sans-paille added a comment.



>>> As a general note, the code duplication between inlineStackProbe and emitProbedAlloca is a bit unfortunate, not sure if there's a better way here.
>> 
>> I'll explore that aspect too, thanks for the code review :-)

At first glance, the code is indeed very similar, to the exception of the handling of the tail (no probing needed for the tail in inlineStackProbe, but one is required for emitProbedAlloca) and the fact that the Size of the alloca is a constant in one case, and not in the other case. But it looks like some extra argument could do the trick.

> It would also be useful if you could explain the comment you added "stack probing may involve looping, and control flow generations is disallowed at this point. Rely to later processing through `inlineStackProbe`".  Is this still true? It probably is, I just can't see why this is needed since emitPrologue() and inlineStackProbe() are run nearly right after eachother in PEI::insertPrologEpilogCode().

It's clearer if I just state that we're reusing the existing probe infrastructure. I'm not a MachineInstruction expert and don't recall why I wrote this :-/


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78717/new/

https://reviews.llvm.org/D78717