On Wed, Nov 23, 2016 at 05:25:44AM +0000, Hurugalawadi, Naveen wrote:
> Hi,
>
> Please find attached the patch that fixes PR71112.
>
> The current implementation that handles SYMBOL_SMALL_GOT_28K in
> aarch64_load_symref_appropriately access the high part of RTX for Big-Endian
> mode which results in ICE for ILP32.
>
> The attached patch modifies it by accessing the lower part for both Endian
> and fixes the issue.
>
> Please review the patch and let me know if its okay?
>
>
> 2016-11-23 Andrew PInski <apin...@cavium.com>
>
> gcc
> * config/aarch64/aarch64.c (aarch64_load_symref_appropriately):
> Access the lower part of RTX appropriately.
>
> gcc/testsuite
> * gcc.target/aarch64/pr71112.c : New Testcase.
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index efcba83..4d87953 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -1298,7 +1298,8 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
> emit_move_insn (gp_rtx, gen_rtx_HIGH (Pmode, s));
>
> if (mode != GET_MODE (gp_rtx))
> - gp_rtx = simplify_gen_subreg (mode, gp_rtx, GET_MODE (gp_rtx), 0);
> + gp_rtx = gen_lowpart (mode, gp_rtx);
> +
> }
>
> if (mode == ptr_mode)
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr71112.c
> b/gcc/testsuite/gcc.target/aarch64/pr71112.c
> new file mode 100644
> index 0000000..5bb9dee
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr71112.c
> @@ -0,0 +1,12 @@
> +/* PR target/71112 */
> +/* { dg-do compile } */
> +/* { dg-options "-mabi=ilp32 -mbig-endian -fpie" } */
Why limit the ABI and endianness here, and if you do plan to do that
should you not also check the effective target to make sure these options
are OK to add?
i.e.
/* { dg-require-effective-target pie } */
/* { dg-require-effective-target aarch64_big_endian } */
And probably one for ilp32 too?
As this testcase should pass across all ABI variants and all endinaness
the right thing to do is probably to drop the extra options.
The same comment applies to the testcase for PR78382.
I'm concerned we get this right early, as the trouble caused in the ARM
back-end by testcases assuming they can add options, then FAILing (for a
variety of reasons) is very painful, and makes it hard to read test output
on ARM - I'd rather we didn't introduce the same issues on AArch64.
Thanks,
James
