Re: [RS6000] power8 internal compiler errors

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

David Edelsohn wrote:
> On Mon, Feb 10, 2014 at 8:33 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Mon, Feb 10, 2014 at 07:01:03PM -0500, David Edelsohn wrote:
> >> On Mon, Feb 10, 2014 at 5:18 PM, Alan Modra <amodra@gmail.com> wrote:
> >>
> >> Shouldn't addr_op2 also be set from find_replacement?
> >
> > Sorry, I thought after I sent the email that I should have added some
> > explanation of why certain parts need find_replacement and others
> > don't.  We want just those parts of addresses that might have been
> > reloaded.
> >
> > There's the case of the entire address being reloaded (actually, I'm
> > not sure this one is needed) and then all the ones we do in the rs6000
> > backend in legitimize_reload_address.  I think I found all the
> > required parts but it certainly won't hurt if you check too.  Calling
> > find_replacement when not strictly necessary will slow down gcc a
> > little..
> 
> I cannot tell if either branch of the PLUS could contain an address
> that previously was replaced. I want to avoid slow downs, but I also
> want to avoid leaving a latent bug, either now or a future change to
> reload or IRA.

In general, any part of an address may have been replaced.  In the
most generic case, if we have an address of the form
  (mem (plus (reg1) (reg2)))
for two pseudos, each of the pseudos *could* have been allocated
e.g. to an FPR under unusual circumstances, and then it would
require a replacement (without the whole address itself being
fully replaced).
 
> Is there an ENABLE_CHECKING assert of some form that could test
> addr_op2 to ensure that it does contain part of the address that was
> reloaded when not configured as a Release?

Note that find_replacement itself already recurses into both sides
of a PLUS.  So given the code flow:

-  addr = XEXP (mem, 0);
+  addr = find_replacement (&XEXP (mem, 0));
[...]
       if (GET_CODE (addr) == AND)
 	{
 	  and_op2 = XEXP (addr, 1);
-	  addr = XEXP (addr, 0);
+	  addr = find_replacement (&XEXP (addr, 0));
 	}

      if (GET_CODE (addr) == PRE_MODIFY)
        {
          scratch_or_premodify = XEXP (addr, 0);
          if (!REG_P (scratch_or_premodify))
            rs6000_secondary_reload_fail (__LINE__, reg, mem, scratch, store_p);

          if (GET_CODE (XEXP (addr, 1)) != PLUS)
            rs6000_secondary_reload_fail (__LINE__, reg, mem, scratch, store_p);

          addr = XEXP (addr, 1);
        }

      if (GET_CODE (addr) == PLUS
          && (and_op2 != NULL_RTX
              || !rs6000_legitimate_offset_address_p (PTImode, addr,
                                                      false, true)))
        {
          addr_op1 = XEXP (addr, 0);
          addr_op2 = XEXP (addr, 1);

the only time addr_op1 *or* addr_op2 might need another replacement
is if addr was reset to the inner of a PRE_MODIFY.  So it might be
easier and cheaper overall to just do a find_replacement within
the PRE_MODIFY clause ...


If you really prefer a check, I guess you can always do something like:

#ifdef ENABLE_CHECKING
  gcc_assert (find_replacement (&XEXP (...)) == XEXP (...));
#endif

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com