On Mon, Jan 29, 2018 at 11:41:32PM +0100, Jakub Jelinek wrote: > On Mon, Jan 29, 2018 at 04:26:50PM -0600, Segher Boessenkool wrote: > > > The code actually meant pointer comparison, the question is what is > > > different on powerpc* that you end up with a different REG. > > > >From what I can see, function.c uses crtl->args.internal_arg_pointer > > > directly rather than a REG with the same REGNO. > > > Where does it become something different and why? > > > > There is a lot of code that copies any RTX that isn't obviously unique. > > Here we have a PLUS of some things, which always needs copying, can > > never be shared. > > Yes, PLUS needs to be unshared. > So it ought to be handled by the PLUS handling code. > || (GET_CODE (XEXP (incoming, 0)) == PLUS > && XEXP (XEXP (incoming, 0), 0) > == crtl->args.internal_arg_pointer > && CONST_INT_P (XEXP (XEXP (incoming, 0), 1))))) > Here we are talking about MEMs on the PARM_DECL DECL_RTLs, those are not > instantiated as normal IL in the RTL stream is.
This is a PLUS of something with the internal_arg_pointer. Our internal_arg_pointer _itself_ is a PLUS, so it should be copy_rtx'd everywhere it is used (or risk RTL sharing). > I'm not saying the var-tracking.c change is wrong, but I'd like to see > analysis on what is going on. The rtx_equal_p is equal to the == for anything that uses just a single register. For us it makes the compiler bootstrap again (with Go enabled), not unnice to have in stage4 ;-) I agree the code does not seem to be set up for PLUS to work here. > Is the patch for -fsplit-stack where rs6000_internal_arg_pointer > returns a PLUS of some pseudo and some offset? Yeah. > In that case I wonder how does the patch with rtx_equal_p actually work, > because function.c then uses plus_constant on this result, and if there are Not everywhere, in places it does gen_rtx_PLUS (Pmode, stack_parm, offset_rtx); (so we end up with non-canonical RTL). > 2 non-zero offsets added, the result is a PLUS with a single offset based > on that pseudo. That would mean var-tracking.c would need special code to > handle crtl->args.internal_arg_pointer that is a PLUS, and handle it > differently from crtl->args.internal_arg_pointer that is a pseudo REG. Yeah. Not entirely pleasant. Not super hard for just reg+off, but for general expressions it isn't very funny. If we want to force internal_arg_pointer to be just one reg the doc in target.def should be updated: /* ??? Documenting this hook requires a GFDL license grant. */ DEFHOOK_UNDOC (internal_arg_pointer, "Return an rtx for the argument pointer incoming to the\ current function.", rtx, (void), default_internal_arg_pointer) Segher
