Guys, on second thought... doesn't making the exception registers live from the InvokeInst to the LandingpadInst create problems for critical-edge-splitting ? if a landingpad-edge is critical and needs to be split, won't we be creating and inserting a new BB between the "invoke-block" and the "landingpad-block", and if we do then isn't there the possibility of the register allocator spilling the contents of the exception registers from within the newly created block --- but this block won't ever get executed because the _Unwind_ / _personality_ functions will cause control flow to go directly to the block with the LandingpadInst ? If you really want to split a landingpad-edge won't you have to move the LandingpadInst up into the new block ? if this is true (and I seem to be making a lot of logic errors lately, so maybe reread and proof-read the above a few times...!...) then don't we need to add another invariant to Bill's list: *) there can be no code between an InvokeInst and its LandingpadInst other than possibly PHINodes at the beginning of the landingpad-block. It seems that if you really do need to split a landingpad-edge then you have to move the LandingpadInst up into (the beginning of) the new block. However it seems that if a landingpad-block has multiple predecessors (often the case, multiple InvokeInst in the main body of a try-statement all go to the same landingpad- block), then you cannot move the LandingpadInst in order to break a critical edge unless you do it for _all_ landingpad-block predecessor edges simultaneously, but that seems to be a messy conclusion (being forced to split other edges that don't need to be split). my first guess is that all the nuances of whether it ever makes sense and/or is even logically possible to split a critical landingpad-edge won't be discovered except by painful trial-and-error, and that it might be best to at first disallow it until proven doable by someone working in an isolated branch -- although proving it works may be difficult, since so little code actually uses exceptions (only TableGen in llvm ?). -Peter Lawrence. On Aug 4, 2011, at 4:16 PM, Andrew Trick wrote:> Hi Peter, > > Thanks for pointing this out. Some us who are concerned with > codegen have discussed the problem. Although the details aren't > decided, you can be sure that at the MachineInstr level we won't > have a landindpadInst to model liveness of exception values. Any > physical registers set by the personality function will be > considered live immediately after the call on the unwind path. > > -Andy > > On Aug 4, 2011, at 4:04 PM, Peter Lawrence wrote: > >> Chris, >> it is goodness that the LandingpadInst will be pinned >> to the beginning >> of a BasicBlock,... except for the possibility of PHINode >> instructions that _must_ >> come even earlier.?. >> >> I can't exactly put my finger on what's going to go wrong with this, >> but it sure smells fishy... >> >> >> my current understanding is that the LandingpadInst will "define" >> some hard >> registers which will be used by following code to switch to the >> corresponding >> catch-clause >> >> the lifetimes of these hard registers ostensibly starts at the >> LandingpadInst, >> but for purposes of PHI lowering and Register Allocation they >> _must_ actually >> start at the beginning of the BasicBlock -- since that is where >> control flow will >> return to from the _Unwind_RaiseException / __gcc_personality_v0 >> calls, >> and it is the _Unwind_ and _personality_ functions that physically >> set those >> hard registers, not the "LandingpadInst". >> >> Somehow PHI lowering and register allocation need to be prohibited >> from >> using those hard registers for spill code at the beginning of a >> "landing pad block", >> but I don't see how that will "fall out" of the current design.?. >> >> >> >> -Peter Lawrence. >> >> >> >> >> On Aug 3, 2011, at 12:35 AM, llvmdev-request at cs.uiuc.edu wrote: >> >>> >>> I agree with Bill in this case. The reason for landingpad to be >>> an instruction is a) make it clear that it is magic in several >>> ways (e.g. pinned to the start of a block), and b) so that >>> LandingPadInst can have a bunch of useful accessors on it. >>> >>> -Chris >>> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110805/05541c99/attachment.html>
On Aug 5, 2011, at 10:57 AM, Peter Lawrence wrote:> However it seems that if a landingpad-block has multiple predecessors (often the case, > multiple InvokeInst in the main body of a try-statement all go to the same landingpad- > block), then you cannot move the LandingpadInst in order to break a critical edge unless > you do it for _all_ landingpad-block predecessor edges simultaneously, but that seems > to be a messy conclusion (being forced to split other edges that don't need to be split). > > > my first guess is that all the nuances of whether it ever makes sense and/or is even > logically possible to split a critical landingpad-edge won't be discovered except by > painful trial-and-error, and that it might be best to at first disallow it until proven doable > by someone working in an isolated branch -- although proving it works may be difficult, > since so little code actually uses exceptions (only TableGen in llvm ?).Peter, I think this will be done lazily to avoid excessive splitting as in: Call1 -> LP Call2 -> LP Call3 -> LP => split Call1 Call1 -> LP-split -> LP-remainder Call2 -> LP-split-merge -> LP-remainder Call3 -> LP-split-merge -> LP-remainder But John will know best. -Andy -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110805/d164804f/attachment.html>
On Aug 5, 2011, at 10:57 AM, Peter Lawrence wrote:> Guys, > on second thought... > > doesn't making the exception registers live from the InvokeInst to the LandingpadInst > create problems for critical-edge-splitting ? > > if a landingpad-edge is critical and needs to be split, won't we be creating and inserting > a new BB between the "invoke-block" and the "landingpad-block", and if we do then > isn't there the possibility of the register allocator spilling the contents of the exception > registers from within the newly created block --- but this block won't ever get executed > because the _Unwind_ / _personality_ functions will cause control flow to go directly > to the block with the LandingpadInst ? If you really want to split a landingpad-edge > won't you have to move the LandingpadInst up into the new block ?You cannot split critical edges to landing pads, but you can duplicate landing pads and distribute the predecessors any way you like. The requirement is that all unwind edges go to landing pads and all non-unwind edges go to non-landing pads. That will also be required in the code generator.> if this is true (and I seem to be making a lot of logic errors lately, so maybe reread and > proof-read the above a few times...!...) then don't we need to add another invariant to > Bill's list: > > *) there can be no code between an InvokeInst and its LandingpadInst other than > possibly PHINodes at the beginning of the landingpad-block.I think this covers it: A landing pad block must have a 'landingpad' instruction as its first non-PHI instruction. There can be only one 'landingpad' instruction within the landing pad block. A basic block that is not a landing pad block may not include a 'landingpad' instruction. /jakob -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110805/69a05c77/attachment.html>
On Aug 5, 2011, at 11:06 AM, Andrew Trick wrote:> On Aug 5, 2011, at 10:57 AM, Peter Lawrence wrote: >> However it seems that if a landingpad-block has multiple predecessors (often the case, >> multiple InvokeInst in the main body of a try-statement all go to the same landingpad- >> block), then you cannot move the LandingpadInst in order to break a critical edge unless >> you do it for _all_ landingpad-block predecessor edges simultaneously, but that seems >> to be a messy conclusion (being forced to split other edges that don't need to be split).Yes, this is why we're not going to have SplitCriticalEdge succeed on landing pad edges, at least by default. It's not conceptually impossible, but it is significantly more invasive than a normal edge-splitting, and pretty much every client would need to be updated to handle it.> I think this will be done lazily to avoid excessive splitting as in: > > Call1 -> LP > Call2 -> LP > Call3 -> LP > > => split Call1 > > Call1 -> LP-split -> LP-remainder > Call2 -> LP-split-merge -> LP-remainder > Call3 -> LP-split-merge -> LP-remainderYes, this is precisely the transformation required for splitting a critical edge to a landing pad. If LP has n predecessors and k phis, such that looks like this: LP: %phi_i = phi [ %value_{i,1}, %call_1 ] .. [ %value_{i,n}, %call_n ] %lpad = landingpad LP-args LP-instructions... then the three new blocks look like this: LP-split: %lpad-split = landingpad LP-args br label %LP-remainder LP-split-merge: %phi-split-merge_i = phi [ %value_{i,2}, %call_2 ] .. [ %value_{i,n}, %call_n ] %lpad-split-merge = landingpad LP-args br label %LP-remainder LP-remainder: %phi_i = phi [ %value_{i,1}, %LP-split ], [ %phi-split-merge, %LP-split-merge ] %lpad = phi [ %lpad-split, %LP-split ], [ %lpad-split-merge, %LP-split-merge ] LP-instructions... In practice, we would want to recognize that LP has the form of an LP-split-merge, with associated LP-remainder, and we would just re-use that as our new LP-split-merge block, adding incoming values to the phis in the LP-remainder as appropriate. John. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110805/4fa80171/attachment.html>