Evgeny Karpov <evgeny.kar...@microsoft.com> writes:
> The current binutils implementation does not support offset up to 4GB in
> IMAGE_REL_ARM64_PAGEBASE_REL21 relocation and is limited to 1MB.
> This is related to differences in ELF and COFF relocation records.
> There are ways to fix this. This work on relocation change will be extracted
> to
> a separate binutils patch series and discussion.
>
> To unblock the current patch series, the IMAGE_REL_ARM64_PAGEBASE_REL21
> relocation will remain unchanged, and the workaround below will be applied to
> bypass the 1MB offset limitation.
>
> Regards,
> Evgeny
>
>
> The patch will be replaced by this change.
Seems like a reasonable workarond to me FWIW, but some comments on the
implementation below:
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 03362a975c0..5f17936df1f 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -2896,7 +2896,30 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
> if (can_create_pseudo_p ())
> tmp_reg = gen_reg_rtx (mode);
>
> - emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, copy_rtx (imm)));
> + do
> + {
> + if (TARGET_PECOFF)
> + {
> + poly_int64 offset;
> + HOST_WIDE_INT const_offset;
> + strip_offset (imm, &offset);
> +
> + if (offset.is_constant (&const_offset)
> + && abs(const_offset) >= 1 << 20)
abs_hwi (const_offset) (since const_offset has HOST_WIDE_INT type).
> + {
> + rtx const_int = imm;
> + const_int = XEXP (const_int, 0);
> + XEXP (const_int, 1) = GEN_INT(const_offset % (1 << 20));
CONST_INTs are shared objects, so we can't modify their value in-place.
It might be easier to pass base and const_offset from the caller
(aarch64_expand_mov_immediate). We are then guaranteed that the
offset is constant and don't need to worry about the SVE case.
The new SYM+OFF expression can be calculated using plus_constant.
I think it'd be worth asserting that the offset fits in 32 bits,
since if by some bug the offset is larger, we'd generate silent
wrong code (in the sense that the compiler would truncate the offset
before the assembler sees it).
> +
> + emit_move_insn (tmp_reg, gen_rtx_HIGH (mode,
> copy_rtx(imm)));
> + emit_insn (gen_add_hioffset (tmp_reg,
> GEN_INT(const_offset)));
I think the normal addition patterns can handle this, if we pass the
result of the ~0xfffff calculation. There should be no need for a
dedicated pattern.
> + break;
> + }
> + }
> +
> + emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, copy_rtx (imm)));
> + } while(0);
I think it'd be clearer to duplicate the gen_add_losym and avoid the
do...while(0)
Thanks,
Richard
> +
> emit_insn (gen_add_losym (dest, tmp_reg, imm));
> return;
> }
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 665a333903c..072110f93e7 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -7405,6 +7405,13 @@
> DONE;
> })
>
> +(define_insn "add_hioffset"
> + [(match_operand 0 "register_operand")
> + (match_operand 1 "const_int_operand")]
> + ""
> + "add %0, %0, (%1 & ~0xfffff) >> 12, lsl #12"
> +)
> +
> (define_insn "add_losym_<mode>"
> [(set (match_operand:P 0 "register_operand" "=r")
> (lo_sum:P (match_operand:P 1 "register_operand" "r")
