On Sun, 15 Jan 2017, Jakub Jelinek wrote: > On Sat, Jan 14, 2017 at 09:16:11PM -0700, Jeff Law wrote: > > > The force_operand on complex count expression in doloop_modify can invoke > > > various expander routines that are assuming there is rtl unsharing after > > > them (which is the case for expansion). When later optimizations invoke > > > the expander (e.g. expand_mult in this case), they use > > > unshare_all_rtl_in_chain on the sequence. The following patch does that > > > for > > > doloop. The count expression is already known not to be shared with > > > anything else (we do copy_rtx on it first and then create new rtls around > > > it > > > if needed), so for that if it occurs just once in the sequence, we don't > > > need to unshare it. For subexpression of condition I'm not sure, which is > > > why I've forced unsharing even if it occurs just once and is not shareable > > > part of the condition like REG. > > > > > > Bootstrapped/regtested on powerpc64-linux, ok for trunk? > > > > > > 2017-01-14 Jakub Jelinek <ja...@redhat.com> > > > > > > PR target/79080 > > > * loop-doloop.c (doloop_modify): Call unshare_all_rtl_in_chain on > > > sequence. Formatting fixes. > > > (doloop_optimize): Formatting fixes. > > > > > > * gcc.dg/pr79080.c: New test. > > So do we have a more general problem here -- we use force_operand in all > > kinds of contexts, particularly in the RTL loop optimizers. If > > force_operand can introduce undesirable sharing, then isn't more code prone > > to this problem? > > I think it is not as bad. I think the problem is only when you > force_operand 1) after expansion 2) with complicated expression. > I think if you force_operand just with something that appears in some insn, > it is very unlikely it will create something significantly more complex. > It is just the case where something constructs a complex RTX expression and > then produces insns using the force_operand. In the doloop case, it is > desc->niter_expr expression, where possibly several insns in the IL together > are used to compute the niter_expr. Or another case could be when using > content of REG_EQUAL note and trying to force_operand it.
Maybe we want to have a force_operand_and_unshare () function then? Richard. > > So just to be clear, I'm OK with this change as-is, I just worry we have > > other places that are structure sharing assumption problems waiting to > > happen. > > We have rtl sharing verification, while it has been defunct for about 3 > years, it is in effect again for some time already and not everything in the > RTL are changed during the last 3 years, before that it had to pass that > verification too. I think it is fine to just wait if similar RTL sharing > issues are reported next time, it is hard to guess where it would be > actually needed and where it would be just a waste of time. > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
