Re: [RFA] A patch for PR 25115

[Ulrich Weigand <uweigand at de dot ibm dot com>]

Paolo Bonzini wrote:

> > I attached a different patch to PR25115.  Since the change to 
> > hash_scan_set uses a note to populate the hash table, my patch uses the 
> > note to figure out the parameter to gen_move_insn.  I'll post the patch 
> > formally after bootstrap/regtest finishes on powerpc-darwin-8.2.0; the 
> > patch fixes the test case on sh.
> 
> Indeed, Kaz confirmed that it fixes the ICE.  It produces better code on 
> a s390 testcase attached to PR25115, and worse code on the original sh 
> testcase.  I'll look at the sh pessimization in the near future; in the 
> meanwhile, ok to commit?

> +  if (!set)
> +    {
> +      rtx note = find_reg_equal_equiv_note (insn);
> +      gcc_assert (note);
> +      new_insn = gen_move_insn (reg, XEXP (note, 0));
> +      new_insn = emit_insn_after (new_insn, insn);
> +
> +      /* Keep register set table up to date.  */
> +      record_one_set (regno, new_insn);
> +    }

I don't understand how this is supposed to work in the general case.

The expression in the note may very well be of a form that *cannot*
be re-computed by a simple gen_move_insn -- there may not be any
single pattern that does the computation, or the pattern may require
additional CLOBBERs etc.

In fact, as I understand it this is the very reason why this piece
of code looks at the orignal insn in the first place -- otherwise
you could just use gen_move_insn (reg, expr->expr) ...

It does happen to work in the current s390 test case because there
happens to be an insn to match -- but this is just coincidence.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com