This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, rs6000][GCC6] Fix PR78543, ICE in push_reload on powerpc64le-linux-gnu


Peter Bergner wrote:

> If we look at rs6000_mode_dependent_address(), it accepts some addresses
> as not being mode dependent:
> 
>     case PLUS:
>       /* Any offset from virtual_stack_vars_rtx and arg_pointer_rtx
>          is considered a legitimate address before reload, so there
>          are no offset restrictions in that case.  Note that this
>          condition is safe in strict mode because any address involving
>          virtual_stack_vars_rtx or arg_pointer_rtx would already have
>          been rejected as illegitimate.  */
>       if (XEXP (addr, 0) != virtual_stack_vars_rtx
>           && XEXP (addr, 0) != arg_pointer_rtx
>           && GET_CODE (XEXP (addr, 1)) == CONST_INT)
>         {
>           unsigned HOST_WIDE_INT val = INTVAL (XEXP (addr, 1));
>           return val + 0x8000 >= 0x10000 - (TARGET_POWERPC64 ? 8 : 12);
>         }
>       break;
> 
> It seems wrong that we accept addresses based off virtual_stack_vars_rtx
> and arg_pointer_rtx, but not stack_pointer_rtx (ie, r1) like we have in
> these test cases.

This check is usually done in legitimate_address_p, and there this
distinction is in fact correct: the problem is that when rewriting
an address based on a virtual register into one based on the stack
(or frame) pointer, the offset will change, and may change significantly.
(E.g. you may end up with "frame size - original offset".)

This means that checking the offset relative to the virtual register
is completely pointless:
(A) If the offset relative to the virtual register is in range, the
    resulting offset relative to the stack pointer may be out of range.
(B) If the offset relative to the virtual register is out of range,
    the resulting offset may still end up *in* range.

Because of (A), reload will re-check offsets after eliminating
registers, and fix up offsets that are now out of range.  But
because if (B), if we were to reject out-of-range offsets relative
to a virtual register already in legitimate_address_p, we'd already
commit to generatcwinge more complex code (e.g. precomputing the address)
and even if the final offset would be in range, nobody would then
clean up that unnecessary code any more.

That is why the legitimate_address_p deliberately and correctly
accepts *any* offset for virtual (and eliminable) registers,
thereby deferring the offset validity check until after the
register has been replaced by a real register.  But for any
actually real register (like the stack pointer), of course we
*do* have to perform the precise check -- nobody would do that
check otherwise, and we'd end up with invalid instructions.


Now, here we are in mode_dependent_address, which in general
should return true if legitimate_address_p on this address might
return different results depending on the mode of the address.

And in fact, for stack-pointer based addresses, legitimate_address_p
*can* return different results depending on the mode, as the comment
in front of mode_dependent_address explains:

/* Go to LABEL if ADDR (a legitimate address expression)
   has an effect that depends on the machine mode it is used for.

   On the RS/6000 this is true of all integral offsets (since AltiVec
   and VSX modes don't allow them) or is a pre-increment or decrement.


On the other hand, for addresses based on a virtual register,
legitimate_address_p does not depend on the mode since those
are special-cased to be always accepted (see the discussion above).

So I'm not sure that the proposed change is in fact correct ...

Bye,
Ulrich

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]