Christophe Lyon <christophe.l...@linaro.org> writes:
> @@ -3485,6 +3485,14 @@ arm_option_override (void)
> if (flag_pic && TARGET_VXWORKS_RTP)
> arm_pic_register = 9;
>
> + /* If in FDPIC mode then force arm_pic_register to be r9. */
> + if (TARGET_FDPIC)
> + {
> + arm_pic_register = FDPIC_REGNUM;
> + if (TARGET_THUMB1)
> + sorry ("FDPIC mode is not supported in Thumb-1 mode.");
Should be no "." at the end.
> + }
> +
> if (arm_pic_register_string != NULL)
> {
> int pic_register = decode_reg_name (arm_pic_register_string);
> [...]
> @@ -7295,6 +7303,21 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
> if (cfun->machine->sibcall_blocked)
> return false;
>
> + if (TARGET_FDPIC)
> + {
> + /* In FDPIC, never tailcall something for which we have no decl:
> + the target function could be in a different module, requiring
> + a different FDPIC register value. */
> + if (decl == NULL)
> + return false;
> +
> + /* Don't tailcall if we go through the PLT since the FDPIC
> + register is then corrupted and we don't restore it after
> + static function calls. */
> + if (!targetm.binds_local_p (decl))
> + return false;
> + }
> +
> /* Never tailcall something if we are generating code for Thumb-1. */
> if (TARGET_THUMB1)
> return false;
Is this still needed after you removed the optimisation to avoid
restoring r9? (Not really a review comment, just curious.)
> [...]
> @@ -7780,28 +7812,132 @@ arm_load_pic_register (unsigned long saved_regs
> ATTRIBUTE_UNUSED, rtx pic_reg)
> emit_use (pic_reg);
> }
>
> +/* Try to determine whether an object, referenced via ORIG, will be
> + placed in the text or data segment. This is used in FDPIC mode, to
> + decide which relocations to use when accessing ORIG. *IS_READONLY
> + is set to true if ORIG is a read-only location, false otherwise.
> + Return true if we could determine the location of ORIG, false
> + otherwise. *IS_READONLY is valid only when we return true. */
> +static bool
> +arm_is_segment_info_known (rtx orig, bool *is_readonly)
> +{
> + *is_readonly = false;
> +
> + if (GET_CODE (orig) == LABEL_REF)
> + {
> + *is_readonly = true;
> + return true;
> + }
> +
> + if (SYMBOL_REF_P (orig))
> + {
> + if (CONSTANT_POOL_ADDRESS_P (orig))
> + {
> + *is_readonly = true;
> + return true;
> + }
> + else if (SYMBOL_REF_LOCAL_P (orig)
> + && !SYMBOL_REF_EXTERNAL_P (orig)
> + && SYMBOL_REF_DECL (orig)
> + && (!DECL_P (SYMBOL_REF_DECL (orig))
> + || !DECL_COMMON (SYMBOL_REF_DECL (orig))))
This can just be an "if".
> + {
> + tree decl = SYMBOL_REF_DECL (orig);
> + tree init = (TREE_CODE (decl) == VAR_DECL)
> + ? DECL_INITIAL (decl) : (TREE_CODE (decl) == CONSTRUCTOR)
> + ? decl : 0;
> + int reloc = 0;
> + bool named_section, readonly;
> +
> + if (init && init != error_mark_node)
> + reloc = compute_reloc_for_constant (init);
> +
> + named_section = TREE_CODE (decl) == VAR_DECL
> + && lookup_attribute ("section", DECL_ATTRIBUTES (decl));
> + readonly = decl_readonly_section (decl, reloc);
> +
> + /* We don't know where the link script will put a named
> + section, so return false in such a case. */
> + if (named_section)
> + return false;
> +
> + *is_readonly = readonly;
> + return true;
> + }
> + else
> + {
> + /* We don't know. */
> + return false;
> + }
> + }
> + else
> + gcc_unreachable ();
> +
> + return false;
Then this can end with:
/* We don't know. */
return false;
}
gcc_unreachable ();
}
> +}
> +
> /* Generate code to load the address of a static var when flag_pic is set.
> */
> static rtx_insn *
> arm_pic_static_addr (rtx orig, rtx reg)
> {
> rtx l1, labelno, offset_rtx;
> + rtx_insn *insn;
>
> gcc_assert (flag_pic);
>
> - /* We use an UNSPEC rather than a LABEL_REF because this label
> - never appears in the code stream. */
> - labelno = GEN_INT (pic_labelno++);
> - l1 = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, labelno), UNSPEC_PIC_LABEL);
> - l1 = gen_rtx_CONST (VOIDmode, l1);
> + bool is_readonly = false;
> + bool info_known = false;
> +
> + if (TARGET_FDPIC
> + && SYMBOL_REF_P (orig)
> + && !SYMBOL_REF_FUNCTION_P (orig))
> + info_known = arm_is_segment_info_known (orig, &is_readonly);
> +
> + if (TARGET_FDPIC
> + && SYMBOL_REF_P (orig)
> + && !SYMBOL_REF_FUNCTION_P (orig)
> + && !info_known)
> + {
> + /* We don't know where orig is stored, so we have be
> + pessimistic and use a GOT relocation. */
> + rtx pic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> +
> + insn = calculate_pic_address_constant (reg, pic_reg, orig);
> + }
> + else if (TARGET_FDPIC
> + && SYMBOL_REF_P (orig)
> + && (SYMBOL_REF_FUNCTION_P (orig)
> + || (info_known && !is_readonly)))
The info_known check is redundant here. I think it's actually clearer
without, since it's then more obvious that the final "else" is handling:
!SYMBOL_REF_FUNCTION_P (orig) && is_readonly
(Initially I misread the condition and was wondering why it was safe to
drop to the "else" when "!info_known". But it doesn't do that of course.)
> + {
> + /* We use the GOTOFF relocation. */
> + rtx pic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> +
> + rtx l1 = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, orig), UNSPEC_PIC_SYM);
> + emit_insn (gen_movsi (reg, l1));
> + insn = emit_insn (gen_addsi3 (reg, reg, pic_reg));
> + }
> + else
> + {
> + /* Not FDPIC, not SYMBOL_REF_P or readonly: we can use
> + PC-relative access. */
> + /* We use an UNSPEC rather than a LABEL_REF because this label
> + never appears in the code stream. */
> + labelno = GEN_INT (pic_labelno++);
> + l1 = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, labelno), UNSPEC_PIC_LABEL);
> + l1 = gen_rtx_CONST (VOIDmode, l1);
> +
> + /* On the ARM the PC register contains 'dot + 8' at the time of the
> + addition, on the Thumb it is 'dot + 4'. */
> + offset_rtx = plus_constant (Pmode, l1, TARGET_ARM ? 8 : 4);
> + offset_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, orig, offset_rtx),
> + UNSPEC_SYMBOL_OFFSET);
> + offset_rtx = gen_rtx_CONST (Pmode, offset_rtx);
>
> - /* On the ARM the PC register contains 'dot + 8' at the time of the
> - addition, on the Thumb it is 'dot + 4'. */
> - offset_rtx = plus_constant (Pmode, l1, TARGET_ARM ? 8 : 4);
> - offset_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, orig, offset_rtx),
> - UNSPEC_SYMBOL_OFFSET);
> - offset_rtx = gen_rtx_CONST (Pmode, offset_rtx);
> + insn = emit_insn (gen_pic_load_addr_unified (reg, offset_rtx,
> + labelno));
> + }
>
> - return emit_insn (gen_pic_load_addr_unified (reg, offset_rtx, labelno));
> + return insn;
> }
>
> /* Return nonzero if X is valid as an ARM state addressing register. */
> @@ -8510,7 +8646,7 @@ load_tls_operand (rtx x, rtx reg)
> static rtx_insn *
> arm_call_tls_get_addr (rtx x, rtx reg, rtx *valuep, int reloc)
> {
> - rtx label, labelno, sum;
> + rtx label, labelno = NULL_RTX, sum;
>
> gcc_assert (reloc != TLS_DESCSEQ);
> start_sequence ();
Looks like this might be a stray change (not mentioned in the changelog).
> [...]
> @@ -23069,9 +23234,37 @@ arm_assemble_integer (rtx x, unsigned int size, int
> aligned_p)
> && (!SYMBOL_REF_LOCAL_P (x)
> || (SYMBOL_REF_DECL (x)
> ? DECL_WEAK (SYMBOL_REF_DECL (x)) : 0))))
> - fputs ("(GOT)", asm_out_file);
> + {
> + if (TARGET_FDPIC && SYMBOL_REF_FUNCTION_P (x))
> + fputs ("(GOTFUNCDESC)", asm_out_file);
> + else
> + fputs ("(GOT)", asm_out_file);
> + }
> else
> - fputs ("(GOTOFF)", asm_out_file);
> + {
> + if (TARGET_FDPIC && SYMBOL_REF_FUNCTION_P (x))
> + fputs ("(GOTOFFFUNCDESC)", asm_out_file);
> + else
> + {
> + bool is_readonly;
> +
> + if (arm_is_segment_info_known (x, &is_readonly))
> + fputs ("(GOTOFF)", asm_out_file);
> + else
> + fputs ("(GOT)", asm_out_file);
> + }
It looks like this changes behaviour for non-FDPIC. Is that intentional?
Or should it be:
if (!TARGET_FDPIC
|| arm_is_segment_info_known (x, &is_readonly))
?
> + }
> + }
> +
> + /* For FDPIC we also have to mark symbol for .data section. */
> + if (TARGET_FDPIC
> + && NEED_GOT_RELOC
> + && flag_pic
> + && !making_const_table
> + && SYMBOL_REF_P (x))
> + {
> + if (SYMBOL_REF_FUNCTION_P (x))
> + fputs ("(FUNCDESC)", asm_out_file);
> }
> fputc ('\n', asm_out_file);
> return true;
Given:
> > Can NEED_GOT_RELOC or flag_pic be false for TARGET_FDPIC?
> No.
>
> > Is !flag_pic TARGET_FDPIC supported?
> No; flag_pic is false when we use -mno-fdpic, so we revert to the "usual" abi
> then
the flag_pic and NEED_GOT_RELOC checks look redundant.
Might as well put the SYMBOL_REF_FUNCTION_P (x) in the main "if"
statement rather than split it out.
> [...]
> @@ -8151,10 +8156,33 @@
> pat = gen_call_internal (operands[0], operands[1], operands[2]);
> arm_emit_call_insn (pat, XEXP (operands[0], 0), false);
> }
> +
> + /* Restore FDPIC register (r9) after call. */
> + if (TARGET_FDPIC)
> + {
> + rtx fdpic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> + rtx initial_fdpic_reg =
> + get_hard_reg_initial_val (Pmode, FDPIC_REGNUM);
Formatting nit: "=" should be on the next line.
> +
> + emit_insn (gen_restore_pic_register_after_call (fdpic_reg,
> + initial_fdpic_reg));
> + }
> +
> DONE;
> }"
> )
>
> [...]
> @@ -8240,6 +8273,18 @@
> operands[2], operands[3]);
> arm_emit_call_insn (pat, XEXP (operands[1], 0), false);
> }
> +
> + /* Restore FDPIC register (r9) after call. */
> + if (TARGET_FDPIC)
> + {
> + rtx fdpic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> + rtx initial_fdpic_reg =
> + get_hard_reg_initial_val (Pmode, FDPIC_REGNUM);
Same here.
Looks good otherwise, thanks.
Richard
