On Tue, 2015-10-06 at 12:52 -0400, Rich Felker wrote:
> > > + if (TARGET_FDPIC)
> > > + {
> > > + rtx a = force_reg (Pmode, plus_constant (Pmode, XEXP (tramp_mem,
> > > 0), 8));
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 0), a);
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 4),
> > > OUR_FDPIC_REG);
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 8),
> > > + gen_int_mode (TARGET_LITTLE_ENDIAN ? 0xd203d302 :
> > > 0xd302d203,
> > > + SImode));
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 12),
> > > + gen_int_mode (TARGET_LITTLE_ENDIAN ? 0x5c216122 :
> > > 0x61225c21,
> > > + SImode));
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 16),
> > > + gen_int_mode (TARGET_LITTLE_ENDIAN ? 0x0009412b :
> > > 0x412b0009,
> > > + SImode));
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 20), cxt);
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 24), fnaddr);
> > > + }
> > > + else
> > > + {
> > > + emit_move_insn (change_address (tramp_mem, SImode, NULL_RTX),
> > > + gen_int_mode (TARGET_LITTLE_ENDIAN ? 0xd301d202 :
> > > 0xd202d301,
> > > + SImode));
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 4),
> > > + gen_int_mode (TARGET_LITTLE_ENDIAN ? 0x0009422b :
> > > 0x422b0009,
> > > + SImode));
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 8), cxt);
> > > + emit_move_insn (adjust_address (tramp_mem, SImode, 12), fnaddr);
> > > + }
> >
> > I think this hunk really needs a comment. It copies machine code from
> > somewhere to somewhere via constant loads... but what exactly are the
> > instructions ...
>
> This is generating trampolines for nested functions. This portion of
> the patch applied without modification from the old patch, so I didn't
> read into it in any more detail; it seems to be the following, which
> makes sense:
>
> 0: .long 1f
> .long gotval
> 1: mov.l 3f,r3
> mov.l 2f,r2
> mov.l @r2,r1
> mov.l @(4,r2),r12
> jmp @r1
> nop
> 3: .long cxt
> 2: .long fnaddr
>
> The corresponding non-FDPIC version is:
>
> mov.l 3f,r3
> mov.l 2f,r2
> jmp @r2
> nop
> 3: .long cxt
> 2: .long fnaddr
>
> Should these go into the source as comments?
Yes, please. And of course some of the descriptive text as above.
> I would think it does, but I've found in the RTL files sometimes extra
> escaping is silently accepted, and I'm not sure if omitting it would
> visibly break. Can I rely on it producing a visible error right away
> if removing it is wrong, or do I need to search the gccint
> documentation to figure out what the right way is?
Just compile some code and look at the generated asm.
> I don't want to turn this into a political battle so we can go with
> whatever is appropriate for upstream gcc. Note however that, at
> present, the only targets this code is useful on are completely
> non-GNU Linux (musl-based and not using any GNU userspace on the
> target). uClibc may also work if someone digs up the old (untouched
> since 2011) superh_fdpic branch.
In this case leave as just "Linux".
> By "better" you mean leaving the self-specs approach in-place but
> explicitly initializing it to 0 with Init(0)? That sounds good to me.
Yes.
> It can't generate the same code either way because, with the patch as
> submitted, there's an extra load inside the asm. I would prefer
> switching to an approach that avoids that (mainly to avoid the ugly
> near-duplication of the asm block, but also to save a couple
> instructions) but short of feedback on acceptable ways to do the
> punning in the C++ I'll just leave it in the asm for now.
Do you have some alternatives to what's currently in the patch? It's
difficult to judge without seeing them...
Cheers,
Oleg
