[llvm] r251113 - [CodeGen] Mark setjmp/catchret MBBs address-taken

Thu Nov 12 10:17:27 PST 2015

I disable shrink-wrapping when EH funclet or windows CFI are involved.
Committed revision 252917.

My questions still stand when you have time :).

Cheers,
-Quentin
> On Nov 12, 2015, at 9:37 AM, Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Hi Joseph,
> 
> Thanks for the detail explanation.
> 
> Based on the description, it seems to me the MachineFunction representation is, at least partly, not properly modeling the constraints of the EH code.
> Indeed, in the funclets, there is nothing that uses of modify CSRs registers, so it looks like we do not need any prologue and epilogue in that case. I thought that what we were missing is something that expand CATCHRET into "lea BLOCKADER, rax”, i.e., just a pseudo expansion, but not a full prologue and epilogue.
> 
> But anyway, let me ask a couple more questions:
> - When you said the funclet needs their own prologue and epilogue my impression is that right now this prologue and epilogue are needlessly identical to the function prologue and epilogue. Is this true or they actually different and this is just an artefact or the test case?
> - Is it worth adding the support for funclet in shrink-wrapping? My impression was that the constraints on EH prologue and epilogue are too tight to have it kicking in.
> 
>> On Nov 11, 2015, at 6:55 PM, Joseph Tremoulet <jotrem at microsoft.com> wrote:
>> 
>> Hi,
>> 
>> It seems that the key thing that the shrink-wrapping code needs to be taught is that a single MachineFunction, if it has EH funclets (MF.getMMI().hasEHFunclets()), is (at least for the purposes of shrink-wrapping) really a collection of functions.  There is a "main" or "root" function which uses the opcodes you're used to at its entries/exits.  Then there are the other functions, each of which is a "funclet", whose entries are MachineBasicBlocks for which MBB.isEHFuncletEntry() returns true, and whose non-exceptional exits are catchrets (MBB.getFirstTerminator()->getOpcode() == MF.getSubtarget().getInstrInfo()->getCatchReturnOpcode()).
>> 
>> In addition to the main function needing the usual prolog and epilog(s), each funclet needs its own prolog and epilog(s) -- if you look at PEI::calculateSets, in the non-shrink-wrap case, you can see that isEHFuncletEntry() MBBs get added to the list of saves, and catchrets get added to the list of restores by virtue of isReturnBlock() returning true for them (and the immediate cause of the missing code that you noticed is because the catchret is no longer getting added to the restore list and so emitEpilogue never gets called for it).
>> 
>> I should probably explain how the funclets (and main function) are related to each other at run-time and in the MachineCFG:
>> Funclets have a parent/child relationship that forms a tree with the main function as the root/ancestor.  By the nature of how they get invoked, the property holds at run-time that for any frame on the call-stack which corresponds to a funclet, if you walk up the caller frames from there you are guaranteed to find a frame on the stack corresponding to the funclet's parent in the tree, and you are guaranteed to find it before finding a frame for any other funclet in the MachineFunction. Note this holds transitively, so you can always walk all the way up to the root.  Funclets are invoked by EH personality routines that pass them arguments which they can use to find their ancestors' call frames on the stack, and when we generate code for a funclet we bake in assumptions about its ancestors' frames, including an assumption that the MachineFrameInfo describes the root function's frame as it exists at the point where the funclet is executing.
>> 
>> Most passes get by without knowing all this, and just looking at the MachineCFG.  Every funclet entry MBB has predecessor edges from all the MBBs in the parent funclet which might hold, at their terminator, the point at which the parent is stopped when the child is executing.  Every funclet exit MBB has a successor edges to the MBB in the parent funclet that corresponds to the point where execution of the parent will eventually resume after the child returns.  By presenting the side-effects of the intervening call frames as part of the execution of the terminators heading these edges, this gives a correct view of the control/data-flow through a single conceptual execution of the MachineFunction as a whole.
>> 
>> So I think what that means for shrink-wrapping is that (assuming, at least to start, that you want to consider only the root function shrink-wrappable):
>> 1) When it identifies "all the frame related operations" in the function, it will want to include funclet entries/exits as "frame related operations", since the funclet code bracketed by these has expectations about the MachineFrameInfo describing the root function's frame when it executes.
>> 2) PEI::calculateSets will still need to add funclet entries to the "saves" list and funclet exits to the "restores" list
>> (and that ultimately it could shrink-wrap each funclet independently)
>> 
>> (or if you're just looking for a way to defer this, that would be to punt on MachineFunctions for which MF.getMMI().hasEHFunclets() is true)
> 
> I’ll do that for now.
> 
>> 
>> --
>> 
>> On a related note (well, conceptually orthogonal but practically intertwined), I think the CFI looks wrong too.  https://msdn.microsoft.com/en-us/library/6ck032hh.aspx <https://msdn.microsoft.com/en-us/library/6ck032hh.aspx> describes (briefly) how shrink-wrapped code is described in Windows CFI.  I should have looked at it *before* writing all the above, because it says " A [shrink-wrapped save] that saves nonvolatile registers by using a PUSH or modifies the RSP register by using an additional fixed stack allocation is not supported", which seems to be incompatible with the current LLVM shrink-wrapping codegen :(.  If I'm reading that page correctly, you'd have to leave most of the prolog at the function entry, but could leave the CSR slots uninitialized and merge their allocation with the other SP update, then at the chosen save point just store the CSRs to their slots; the chosen restore would only load the CSRs back out of those slots, and the original epilog could omit the pops and merge the CSR slot reclamation with the other SP update.
>> 
>> The way to test if a MachineFunction has that restriction is MF.getTarget().getMCAsmInfo()->usesWindowsCFI().
> 
> Thanks, that helps!
> 
> Cheers,
> -Quentin
>> 
>> 
>> Hope that helps,
>> -Joseph
>> 
>> 
>> -----Original Message-----
>> From: qcolombet at apple.com [mailto:qcolombet at apple.com] 
>> Sent: Wednesday, November 11, 2015 7:48 PM
>> To: Joseph Tremoulet <jotrem at microsoft.com>
>> Cc: llvm-commits <llvm-commits at lists.llvm.org>
>> Subject: Re: [llvm] r251113 - [CodeGen] Mark setjmp/catchret MBBs address-taken
>> 
>> Hi Joseph,
>> 
>> With shrink-wrapping enabled, the test case added in this commit fails and I need your feedback on the output (see attachments).
>> 
>> The new output seems incorrect to me and assuming it is, could you explain what we should check to teach shrink-wrapping to do the right thing?
>> Alternatively, it may be possible it is just that we miss a proper expansion for CATCHRET.
>> Anyway, please advise.
>> 
>> Thanks,
>> Q.
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151112/8b1a320b/attachment-0001.html>