On 8 June 2018 at 12:41, Kyrill Tkachov <kyrylo.tkac...@foss.arm.com> wrote: > Hi Christophe, > > > On 25/05/18 09:03, Christophe Lyon wrote: >> >> Support additional relocations: TLS_GD32_FDPIC, TLS_LDM32_FDPIC, and >> TLS_IE32_FDPIC. >> >> We do not support the GNU2 TLS dialect. >> >> 2018-XX-XX Christophe Lyon <christophe.l...@st.com> >> Mickaël Guêné <mickael.gu...@st.com> >> >> gcc/ >> * config/arm/arm.c (tls_reloc): Add TLS_GD32_FDPIC, >> TLS_LDM32_FDPIC and TLS_IE32_FDPIC. >> (arm_call_tls_get_addr_fdpic): New. >> (legitimize_tls_address_fdpic): New. >> (legitimize_tls_address_not_fdpic): New. >> (legitimize_tls_address): Add FDPIC support. >> (arm_emit_tls_decoration): Likewise. >> >> Change-Id: I4ea5034ff654540c4658d0a79fb92f70550cdf4a >> >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >> index 2434602..20b8f66 100644 >> --- a/gcc/config/arm/arm.c >> +++ b/gcc/config/arm/arm.c >> @@ -2373,9 +2373,12 @@ char arm_arch_name[] = "__ARM_ARCH_PROFILE__"; >> >> enum tls_reloc { >> TLS_GD32, >> + TLS_GD32_FDPIC, >> TLS_LDM32, >> + TLS_LDM32_FDPIC, >> TLS_LDO32, >> TLS_IE32, >> + TLS_IE32_FDPIC, >> TLS_LE32, >> TLS_DESCSEQ /* GNU scheme */ >> }; >> @@ -8681,6 +8684,30 @@ load_tls_operand (rtx x, rtx reg) >> } >> >> static rtx_insn * >> +arm_call_tls_get_addr_fdpic (rtx x, rtx reg, rtx *valuep, int reloc) >> +{ > > > Please add a function comment > >> + rtx sum; >> + >> + gcc_assert (reloc != TLS_DESCSEQ); >> + start_sequence (); >> + >> + sum = gen_rtx_UNSPEC (Pmode, >> + gen_rtvec (2, x, GEN_INT (reloc)), >> + UNSPEC_TLS); >> + reg = load_tls_operand (sum, reg); >> + emit_insn (gen_addsi3 (reg, reg, gen_rtx_REG (Pmode, 9))); >> + >> + *valuep = emit_library_call_value (get_tls_get_addr (), NULL_RTX, >> + LCT_PURE, /* LCT_CONST? */ >> + Pmode, reg, Pmode); > > > I prefer to avoid comments with such question marks. Is there some ambiguity > on which should be used? > >> + >> + rtx_insn *insns = get_insns (); >> + end_sequence (); >> + >> + return insns; >> +} >> + >> +static rtx_insn * >> arm_call_tls_get_addr (rtx x, rtx reg, rtx *valuep, int reloc) >> { >> rtx label, labelno, sum; >> @@ -8736,8 +8763,84 @@ arm_tls_descseq_addr (rtx x, rtx reg) >> return reg; >> } >> >> -rtx >> -legitimize_tls_address (rtx x, rtx reg) >> +static rtx >> +legitimize_tls_address_fdpic (rtx x, rtx reg) >> +{ > > > Please add a function comment, even if it's just a simple one. > >> + rtx dest, ret, eqv, addend, sum, tp; >> + rtx_insn *insns; >> + unsigned int model = SYMBOL_REF_TLS_MODEL (x); >> + >> + switch (model) >> + { >> + case TLS_MODEL_GLOBAL_DYNAMIC: >> + if (TARGET_GNU2_TLS) >> + { >> + abort (); >> + } > > > Use gcc_unreachable (). > >> + else >> + { >> + /* Original scheme. */ >> + insns = arm_call_tls_get_addr_fdpic (x, reg, &ret, >> TLS_GD32_FDPIC); >> + dest = gen_reg_rtx (Pmode); >> + emit_libcall_block (insns, dest, ret, x); >> + } >> + return dest; >> + break; >> + >> + case TLS_MODEL_LOCAL_DYNAMIC: >> + if (TARGET_GNU2_TLS) >> + { >> + abort (); >> + } > > > Likewise. > >> + else >> + { >> + insns = arm_call_tls_get_addr_fdpic (x, reg, &ret, >> TLS_LDM32_FDPIC); >> + /* Attach a unique REG_EQUIV, to allow the RTL optimizers to >> + share the LDM result with other LD model accesses. */ >> + eqv = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, const1_rtx), >> + UNSPEC_TLS); >> + dest = gen_reg_rtx (Pmode); >> + emit_libcall_block (insns, dest, ret, eqv); >> + >> + /* Load the addend. */ >> + addend = gen_rtx_UNSPEC (Pmode, >> + gen_rtvec (2, x, GEN_INT >> (TLS_LDO32)), >> + UNSPEC_TLS); >> + addend = force_reg (SImode, gen_rtx_CONST (SImode, addend)); > > > Nit I'm guessing, but I think this SImode should be Pmode. > >> + dest = gen_rtx_PLUS (Pmode, dest, addend); >> + } >> + return dest; >> + break; >> + >> + case TLS_MODEL_INITIAL_EXEC: >> + sum = gen_rtx_UNSPEC (Pmode, >> + gen_rtvec (2, x, GEN_INT (TLS_IE32_FDPIC)), >> + UNSPEC_TLS); >> + reg = load_tls_operand (sum, reg); >> + /* FIXME: optimize below? */ > > > Not a fan of such FIXMEs. Let's either optimise it now or leave the comment > out. > >> + emit_insn (gen_addsi3 (reg, reg, gen_rtx_REG (Pmode, 9))); >> + emit_insn (gen_movsi (reg, gen_rtx_MEM (Pmode, reg))); > > > You can use the shorter emit_move_insn (). I think there are other places in > the series > where you can do that as well. > >> + tp = arm_load_tp (NULL_RTX); >> + >> + return gen_rtx_PLUS (Pmode, tp, reg); >> + break; >> + >> + case TLS_MODEL_LOCAL_EXEC: >> + tp = arm_load_tp (NULL_RTX); >> + reg = gen_rtx_UNSPEC (Pmode, >> + gen_rtvec (2, x, GEN_INT (TLS_LE32)), >> + UNSPEC_TLS); >> + reg = force_reg (SImode, gen_rtx_CONST (SImode, reg)); >> + >> + return gen_rtx_PLUS (Pmode, tp, reg); >> + >> + default: >> + abort (); >> + } >> +} >> + >> +static rtx >> +legitimize_tls_address_not_fdpic (rtx x, rtx reg) >> { > > > Likewise, function comment please. > >> rtx dest, tp, label, labelno, sum, ret, eqv, addend; >> rtx_insn *insns; >> @@ -8831,6 +8934,15 @@ legitimize_tls_address (rtx x, rtx reg) >> } >> } >> >> +rtx >> +legitimize_tls_address (rtx x, rtx reg) >> +{ > > > And here. > >> + if (TARGET_FDPIC) >> + return legitimize_tls_address_fdpic (x, reg); >> + else >> + return legitimize_tls_address_not_fdpic (x, reg); >> +} > > > And this is my main gripe. > The new fdpic-specific function looks like it could be integrated with the > existing legitimize_tls_address > function. The structure is the same (switch on model construct and emit some > RTXes). > Can you look to merge the fdpic code more organically rather than splitting > them off? >
OK, I think it was easier to split them during development, I'll merge them. > Thanks, > Kyrill > > >> + >> /* Try machine-dependent ways of modifying an illegitimate address >> to be legitimate. If we find one, return the new, valid address. */ >> rtx >> @@ -28022,15 +28134,24 @@ arm_emit_tls_decoration (FILE *fp, rtx x) >> case TLS_GD32: >> fputs ("(tlsgd)", fp); >> break; >> + case TLS_GD32_FDPIC: >> + fputs ("(tlsgd_fdpic)", fp); >> + break; >> case TLS_LDM32: >> fputs ("(tlsldm)", fp); >> break; >> + case TLS_LDM32_FDPIC: >> + fputs ("(tlsldm_fdpic)", fp); >> + break; >> case TLS_LDO32: >> fputs ("(tlsldo)", fp); >> break; >> case TLS_IE32: >> fputs ("(gottpoff)", fp); >> break; >> + case TLS_IE32_FDPIC: >> + fputs ("(gottpoff_fdpic)", fp); >> + break; >> case TLS_LE32: >> fputs ("(tpoff)", fp); >> break; >> -- >> 2.6.3 >> >