On Jul 23, 2011, at 2:00 AM, Jakob Stoklund Olesen wrote:> On Jul 23, 2011, at 1:11 AM, Bill Wendling wrote: > >> On Jul 22, 2011, at 11:44 PM, Jakob Stoklund Olesen wrote: >>> Could we add: >>> >>> - A landing pad block is not the destination of any other kind of terminator. Only unwind edges are allowed. >>> >>> - The landingpad instruction must only appear at the top of a landing pad. It cannot appear in any other block, or following non-phi instructions. >>> >> Most of this is covered by 2&3. But it would be good to explicitly state that a landingpad instruction can appear only in a landing pad block. > > Right. I think we agree on the intention. I wanted to make it completely clear that: > > - All unwind edges go to a block with a landingpad instruction. > - All non-unwind edges go to a block without a landingpad instruction. > > That then implies that unwind edges can't be split and landingpad instructions can't be moved. > >>> Why won't SplitCriticalEdge work for landing pads? Does it require more than splitting the landing pad after the landingpad instruction, and duplicating the top half? Alternatively, could we get a SplitLandingPad function? >> >> Splitting a critical edge, especially in this case, isn't necessary for correctness. It's an optimization. > > Yes. You scared me with 'requires considerable care'. Does that mean anything other than 'you have to duplicate the landing pad instead of splitting the unwind edge'. Is special magic required to duplicate a landingpad instruction? >There shouldn't be any special magic involved. As you pointed out, we'd have to duplicate the landingpad instruction into each of the critical edge blocks.>> In general, the use of a value in the catch blocks will access that value via memory. > > Because the register allocator will spill, or because mem2reg fails? Won't there be a lot of IR values live across an unwind edge after mem2reg? >The register allocator should be spilling most variables across the unwind edge. The only ones which can stick around are those that are placed in non-volatile registers. This is all opaque to the IR, which doesn't know that the unwind edge is special. This is why we can have PHI nodes in the landing pad. I'm assuming that it gets it "right" because it's keeping values alive across the invoke.>> It also complicates the SplitCriticalEdge function, like you outlined. > > Yes, I agree. Duplicating a landing pad is different than splitting a critical edge. > >>> Will it be possible to split landing pads during codegen? >> >> Split a landing pad or split the critical edges to a landing pad? > > Sorry, I meant duplicating a landing pad. > > Landing pads notoriously collect critical edges. We should make sure there is some way of dealing with that, unlike indirectbr edges. The possibility of splitting and duplicating landing pads would help. >One way to get around this is to do what Andy suggested to me at one point (not on email). We could have one landing pad block per invoke instruction. So no two invokes could ever share a landing pad (and of course the landingpad instruction). (My apologies to Andy if I misrepresented his idea.) I think of this as overkill, but if you feel strongly that not being able to break critical edges is going to hamper code-gen significantly we can discuss this. -bw
On Jul 23, 2011, at 4:15 PM, Bill Wendling wrote:> On Jul 23, 2011, at 2:00 AM, Jakob Stoklund Olesen wrote:>> Yes. You scared me with 'requires considerable care'. Does that mean anything other than 'you have to duplicate the landing pad instead of splitting the unwind edge'. Is special magic required to duplicate a landingpad instruction? >> > There shouldn't be any special magic involved. As you pointed out, we'd have to duplicate the landingpad instruction into each of the critical edge blocks.That sounds good to me. It's not necessary that SplitCriticalEdge can do it, I just want to be able to duplicate a landing pad without knowing the details of any particular personality function.>>> In general, the use of a value in the catch blocks will access that value via memory. >> >> Because the register allocator will spill, or because mem2reg fails? Won't there be a lot of IR values live across an unwind edge after mem2reg? >> > The register allocator should be spilling most variables across the unwind edge. The only ones which can stick around are those that are placed in non-volatile registers. This is all opaque to the IR, which doesn't know that the unwind edge is special. This is why we can have PHI nodes in the landing pad. I'm assuming that it gets it "right" because it's keeping values alive across the invoke.Yep, this should all just work. Unwind edges are just like any other critical edge, and the unwinder preserves the non-volatiles. There is an optimization issue because once you stick a value in a non-volatile register across an unwind edge, that value must be in the same register before every call that shares the landing pad. That limits what live range splitting can do. But this is really the register allocator's problem. I just want to make sure that it will be possible to duplicate those landing pads when they cause problems.>>> It also complicates the SplitCriticalEdge function, like you outlined. >> >> Yes, I agree. Duplicating a landing pad is different than splitting a critical edge. >> >>>> Will it be possible to split landing pads during codegen? >>> >>> Split a landing pad or split the critical edges to a landing pad? >> >> Sorry, I meant duplicating a landing pad. >> >> Landing pads notoriously collect critical edges. We should make sure there is some way of dealing with that, unlike indirectbr edges. The possibility of splitting and duplicating landing pads would help. >> > One way to get around this is to do what Andy suggested to me at one point (not on email). We could have one landing pad block per invoke instruction. So no two invokes could ever share a landing pad (and of course the landingpad instruction). (My apologies to Andy if I misrepresented his idea.) I think of this as overkill, but if you feel strongly that not being able to break critical edges is going to hamper code-gen significantly we can discuss this.It would certainly make many things easier, but I don't think we properly understand the code size impact of doing that. Low-frequency blocks without critical edges are like catnip to the splitter. It will put all the spill code in there, and we may not be able to merge those landing pads again. I would like to keep the option, but I don't think we should aggressively duplicate landing pads. I want to be able to split and duplicate landing pads on demand in IR and in MI. That is just as good as splitting critical edges. It sounds like your proposal allows this. We can discuss the MI representation when the time comes. /jakob
On Jul 23, 2011, at 5:00 PM, Jakob Stoklund Olesen wrote:> It would certainly make many things easier, but I don't think we properly understand the code size impact of doing that. Low-frequency blocks without critical edges are like catnip to the splitter. It will put all the spill code in there, and we may not be able to merge those landing pads again. > > I would like to keep the option, but I don't think we should aggressively duplicate landing pads. > > I want to be able to split and duplicate landing pads on demand in IR and in MI. That is just as good as splitting critical edges. It sounds like your proposal allows this. We can discuss the MI representation when the time comes. >It should be possible to manually break the critical edges, split the landing pad, dup the landingpad instruction, etc. So yes, this is a happy medium. Sorry for the initial confusion. :-) -bw
On Jul 23, 2011, at 5:00 PM, Jakob Stoklund Olesen wrote:> > On Jul 23, 2011, at 4:15 PM, Bill Wendling wrote: > >> On Jul 23, 2011, at 2:00 AM, Jakob Stoklund Olesen wrote: > >>> Yes. You scared me with 'requires considerable care'. Does that mean anything other than 'you have to duplicate the landing pad instead of splitting the unwind edge'. Is special magic required to duplicate a landingpad instruction? >>> >> There shouldn't be any special magic involved. As you pointed out, we'd have to duplicate the landingpad instruction into each of the critical edge blocks. > > That sounds good to me. It's not necessary that SplitCriticalEdge can do it, I just want to be able to duplicate a landing pad without knowing the details of any particular personality function.You can definitely do that. As you surmised, the only problem is the constraint that a landing pad not be the target of a non-unwind edge, which, in the general case, requires the rest of the edges to change, which is likely to be surprising to generic clients of SplitCriticalEdge. I'd be in favor of letting callers of SplitCriticalEdge just specifically opt in to splitting unwind edges, or maybe providing a different entrypoint for it. John.
On Jul 23, 2011, at 5:00 PM, Jakob Stoklund Olesen wrote:> > On Jul 23, 2011, at 4:15 PM, Bill Wendling wrote: >>> Landing pads notoriously collect critical edges. We should make sure there is some way of dealing with that, unlike indirectbr edges. The possibility of splitting and duplicating landing pads would help. >>> >> One way to get around this is to do what Andy suggested to me at one point (not on email). We could have one landing pad block per invoke instruction. So no two invokes could ever share a landing pad (and of course the landingpad instruction). (My apologies to Andy if I misrepresented his idea.) I think of this as overkill, but if you feel strongly that not being able to break critical edges is going to hamper code-gen significantly we can discuss this. > > It would certainly make many things easier, but I don't think we properly understand the code size impact of doing that. Low-frequency blocks without critical edges are like catnip to the splitter. It will put all the spill code in there, and we may not be able to merge those landing pads again. > > I would like to keep the option, but I don't think we should aggressively duplicate landing pads. > > I want to be able to split and duplicate landing pads on demand in IR and in MI. That is just as good as splitting critical edges. It sounds like your proposal allows this. We can discuss the MI representation when the time comes. > > /jakobSorry I neglected this. My original point to Bill was that we're taking on extra complexity in order to support shared landing pads. Yet the only obvious benefit is pretty IR, with some questionable benefit in compacting the bitcode. The extra complexity is clear when you look at edge splitting. John explained it well. Clients of the edge splitter don't expect other edges to change. In general I think it's a bit harder to reason about shared landing pads. For example, it's strange to me to have phi nodes between the invoke and landing pad. But now that I see how verbose the landing pads have become (because they're summarizing the cleanups and handlers too!), I think Bill did the right thing at the IR level. It would just be too gross to clone them everywhere. Codegen is more interesting. We (hopefully) won't need any special EH placeholder instructions, and we'd like to be able to split edges anywhere, anytime prior to branch optimization. I'm philisophically opposed to the argument that split landing pads gives the splitter too much freedom. They simply allow the splitter to do exactly what it was designed to do. I understand that we're currently using the presence of critical edges as a kind of heuristic to balance static code size with performance (note that with split edges, the splitter is reducing dynamic code size at the cost of more almost-never-taken unconditional branches). My concern is that the problem is broader than just EH code and we're avoiding a general solution. For example, I think it would be nice to have a branch optimizer run after regalloc and push instructions across edges to remove unconditional branches only when it makes sense for a particular subtarget. I would prefer to focus the effort on improving edge-splitting heuristics in that pass, rather than rely on each codegen pass to implement its own pathetic attempt at those heuristics. So, it would be nice to keep in mind that in the long-term Codegen doesn't *need* to support shared landing pads prior to branch optimization. -Andy -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110803/7150d257/attachment.html>