Re: [PATCH]: PRE can handle stuff set in jumps

[law at redhat dot com]

  In message <87n14tvvzt.fsf@cgsoftware.com>you write:
  > Let's run through this again, in case you don't get it.
I get it.  But then again, I had the benefit of looking at Geoff's
code and working through it for an hour or so on the phone with Geoff.
I've also had the benefit of working quite a bit more on the gcse pass
than you've done over the years as well as about 10 more years of GCC
experience than you've got.

Part of the problem may be that Geoff hasn't explained things all that
well.

  > He says handle_avail_expr is inserting in the wrong place.
No.  Maybe that's the crux of your misunderstanding.


  > He changes hash_scan_set to not mark the expression as available in
  > the first place.
Yes.

  > Also note that people outside of redhat have no testcase where it fails at
  > all, for either classic  GCSE or PRE. So nobody except redhat people
  > can even say *what* part is broken, and thus, i have to rely on the
  > info given.
Yes.  It's unfortunate, but this problem arises on a part that is still
subject to NDA restrictions.

Consider a conditional jump insn which has one or more additional side
effects (other than the change of flow control).  For example, pretend 
the jump performs an arithmetic operation on two registers and stores
its result into a register.


Let's pretend that we try to expose this capability for various reasons during
RTL generation and thus gcse is potentially presented with such insns.

Roughly they might look like

(parallel [(set (pc) (if_then_else (cond) (label) (pc))
           (set (treg0) (arith_op (sreg0) (sreg1))])



Now assume (set (treg1) (arith_op) (sreg0) (sreg1))) appears elsewhere by
itself as the pattern of a normal insn.

Now also assume that there is a path from the jump to that second occurence
of (arith_op (sreg0) (sreg1)) and that sreg0 and sreg1 are not killed on
that path.

PRE will attempt to remove the later redundant evaluation of that expression.
Instead the later evaluation will be turned into something like

(set (treg1) (reaching_reg))


Of course, we have to go back and insert a copy from treg0 to reaching_reg
after the jump insn.

That happens via pre_insert_copy_insns.

pre_insert_copy_insn isn't prepared to deal with anything that is not a
single_set.  And thus we abort.


While we could make pre_insert_copy_insn handle this case, it hardly seems
worth the effort.  So instead, I recommended to Geoff that we simply not
record any additional expressions inside jump insns.

jeff