[PATCH v2] SH FDPIC backend support

[Oleg Endo]

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