Re: [PATCH] Adjust/remove REG_EQ{UAL,UIV} notes in REE (PR rtl-optimization/63659)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Oct 29, 2014 at 09:47:33PM -0600, Jeff Law wrote:
> On 10/27/14 14:47, Jakub Jelinek wrote:
> >Hi!
> >
> >The following testcase is miscompiled in 4.8+ on x86_64 at -O2+,
> >because REE widens for ZERO_EXTEND mode on
> >(set (reg:QI ax) (const_int -1)) instruction to SImode, but doesn't
> >adjust REG_EQUAL note of (const_int -1) also to (const_int 0xff)
> >like it changes the pattern.
> >
> >The following patch attempts to adjust the notes if they are CONST_INT,
> >or drop otherwise (not sure if it would be desirable to emit
> >a ZERO_EXTEND of the previous content of the note), and both through
> >validate_change, so that if we give up later on, it is undone.
> >
> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >What about 4.9.2 (after release) and 4.8.x?
> >
> >2014-10-27  Jakub Jelinek  <jakub@redhat.com>
> >
> >	PR rtl-optimization/63659
> >	* ree.c (update_reg_equal_equiv_notes): New function.
> >	(combine_set_extension, transform_ifelse): Use it.
> >
> >	* gcc.c-torture/execute/pr63659.c: New test.
> Does this do the right thing in the case where there's multiple reaching
> definitions and one of those reaching definitions turns out to be
> non-combinable, while some other has a fixable REG_EQUAL note?

The changes to REG_EQUAL or REG_EQUIV notes (either adjustment of CONST_INT
in it, or its removal) is tied to the changing of the insn holding it
in the patch.  So, if we revert the changes to the instruction, so it is
again the original mode, we revert the changes to the notes too (either put
back the old CONST_INT, or re-add the note).
The granularity of changes in REE is combine_reaching_defs - each cand
separately, if any insn needed to be changed for some cand fails to be
changed, we revert all the changes for the cand in question (it is not
possible to have only a subset of setters adjusted if we want to adjust
the user(s)).  So IMHO the right thing is what the patch does, tie it to
the apply_change_group/cancel_changes that verifies or undoes the insn
changes.

> Don't we have to separate the check for a fixable note from the actual
> changing of the note?

	Jakub