On Wed, Oct 07, 2015 at 07:22:59AM +0900, Oleg Endo wrote: > 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.
OK. > > 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. OK, I'll try this. > > 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... Perhaps something like the following: #ifdef __SH_FDPIC__ typedef __attribute__((__may_alias__)) uintptr_t sh_aliased_uintptr_t; #define SH_CODE_ADDR(x) (*(sh_aliased_uintptr_t *)(x)) #else #define SH_CODE_ADDR(x) x #endif And then just passing SH_CODE_ADDR(__udiv_qrnnd_16) rather than just __udiv_qrnnd_16 as the input to the asm. Rich