On Mon, 13 Feb 2017, Jeff Law wrote: > On 02/10/2017 05:24 AM, Richard Biener wrote: > > > > It turns out the SSA var defs/uses generated by the gimplifier can break > > apart in a way no longer satisfying the dominance requirement of SSA > > uses vs. their defs by means of CFG construction adding abnormal edges > > for stuff like setjmp (but also non-local gotos I guess). > > > > This would be quite costly to overcome in gimplification - one needs > > to check whether a (part?) of an expression to be gimplified may > > produce such edges and disable SSA name generation for them. With > > the recursive nature of the gimplifier plus the general complexity > > of GENERIC I can't see how to do this. > ISTM there is no apriori way to know if any object is live across a call which > would create these abnormal edges. How in the world did this work in the > past?
It worked in the past because we didn't use SSA names for temporaries in the gimplifiers and regular decls just got effectively uninitialized uses across such edges (by into-SSA and PHI insertion, exactly how I fix this "after the fact") > Sigh. Because the problem starts at gimplification, we can't even do > something like pre-scan the block for problem calls -- we don't have a CFG. > > Can this only happen during gimplification of an expression where a > sub-expression has a problematic call? How bad do you think things would blow > up if we scanned for a call and avoided using SSA_NAMEs when there's a > problematical call within the expression? Or is there no way structurally to > do that within the gimplifier? Well, the only thing I could imagine is doing walk_tree on all GENERIC where it matters (all?). > Are setjmp/longjump the only problems here, or does EH tickle these issues as > well (if so, then ISTM you'd need a different test). The only issue is when CFG construction adds backedges which only happens for abnormals. Only backedges can make uses no longer dominate defs. > > > > Thus the following patch "recovers" from the extra abnormal edges > > by effectively treating SSA vars pre into-SSA as "non-SSA" and thus > > doing PHI insertion for them when rewriting the function into SSA. > > Implementation-wise the easiest thing was to re-write the affected > > SSA vars out-of-SSA (replace them by a decl). > If we can't do the right thing from the start, then "recovery" seems to be the > only option. I think it's a question of cost and maintainability. It for sure must be possible to fix this in the gimplifier but I think the costs are too high. > > > > > The out-of-SSA rewriting is placed in insert_phi_nodes because thats > > the first point in time we have immediate uses present (otherwise > > it could be done at any point after CFG construction). > THe earlier the better. So as soon as we have a CFG & SSA we have to fixup. > So location seems right. Yeah, if we'd have immediate uses at CFG construction then I'd fix it at the time we introduce those abnormal edges. For GCC 8 we might want to consider this (building SSA operands earlier). > > > > I'm not 100% happy with this but after trying for a day I can't come > > up with a better solution - it has the chance of papering over > > bogus gimplifications but OTOH that we only do this for functions > > calling setjmp or having non-local labels would make those trigger > > SSA verification in the other cases. > > > > Bootstrap and regtest running on x86_64-unknown-linux-gnu. > > > > Any comments / ideas? Anyone with good arguments that this is > > even conceptually the correct thing to do? > > > > I'm bootstrapping this with the calls_setjmp/has_nonlocal_label > > short-cutted with a 1 || as well as without (to increase testing > > coverage). > > > > Thanks, > > Richard. > > > > 2017-02-10 Richard Biener <rguent...@suse.de> > > > > PR middle-end/79432 > > * tree-into-ssa.c (insert_phi_nodes): When the function can > > have abnormal edges rewrite SSA names with broken use-def > > dominance out of SSA and register them for PHI insertion. > > > > * gcc.dg/torture/pr79432.c: New testcase. > It's gross. But unless we have a reasonable way to do this during > gimplification, I don't see anything better. Ok. I've applied the patch for now. If anybody comes up with a cheap enough idea to cover these in the gimplifier we can reconsider. Richard. > jeff > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
