Segher Boessenkool <seg...@kernel.crashing.org> writes: > On Tue, Jan 30, 2018 at 09:41:47AM +0000, Richard Sandiford wrote: >> Segher Boessenkool <seg...@kernel.crashing.org> writes: >> > 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). >> >> Is that needed even in DECL_RTL? > > When it is copied to the RTL stream, it has to yes. No sharing allowed, > even if nothing inside this is ever modified.
Right. But DECL_RTL isn't in the RTL stream. It needs to be copied before being used there. And this code is looking at DECL_RTL, not at rtl stream (insn patterns). >> >> 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). >> >> But in that case, what does the copying? > > I don't know. Aaron will look at it, but timezones etc. :-) > >> That's what seems strange. I can see why we'd have two nested >> pluses with the inner plus being pointer-equal to internal_arg_ptr. >> And I can see why we'd have a single canonical plus (which IMO would >> be better, but I agree it's not stage 4 material). It's having the two >> nested pluses without maintaining pointer equality that seems strange. > > The inner plus is *not* pointer-equal, that is the problem. Something > did copy_rtx (or such) on it, many things do. We can tell you what > exactly later today. Yeah, I realise that. And I'm saying that's what seems strange :-) If something is deliberately avoiding folding the plus so that we can still see internal_arg_ptr, why does it end up copying the internal_arg_ptr regardless? Thanks, Richard