Hi all,
On 16 January 2017 at 16:34, Kyrill Tkachov <kyrylo.tkac...@foss.arm.com> wrote:
>
> On 13/01/17 16:35, James Greenhalgh wrote:
>>
>> On Wed, Jan 11, 2017 at 04:32:45PM +0000, Kyrill Tkachov wrote:
>>>
>>> Hi all,
>>>
>>> In this PR we generated ADRP/ADD instructions with :lo12: relocations on
>>> symbols even though -mpc-relative-literal-loads is used. This is due to
>>> the
>>> confusing double-negative logic of the nopcrelative_literal_loads aarch64
>>> variable and its relation to the aarch64_nopcrelative_literal_loads
>>> global
>>> variable in the GCC 6 branch.
>>>
>>> Wilco fixed this on trunk as part of a larger patch (r237607) and parts
>>> of
>>> that patch were backported, but other parts weren't and that patch now
>>> doesn't apply cleanly to the branch.
>>
>> As I commented to Jakub at the time he made the first partial backport,
>> I'd much rather just see all of Wilco's patch backported. We're not on
>> the verge of a 6 release now, so please just backport Wilco's patch (as
>> should have been done all along if this had been correctly identified as
>> a fix rather than a clean-up).
>
>
> Unfortunately simply backporting the patch does not fix this PR.
> The aarch64_classify_symbol changes do not have the desired effect
> and the :lo12: relocations are generated.
> I'll look into it, but I believe that would require a bigger change than
> this one-liner.
Here is the backport of Wilco's patch (r237607) along with Kyrill's
one (r244643, which removed the remaining occurences of
aarch64_nopcrelative_literal_loads). To fix the issue the original
patch has to be modified, to keep aarch64_pcrelative_literal_loads
test for large models in aarch64_classify_symbol.
On trunk and gcc-7-branch the :lo12: relocations are not generated
because of Wilco's fix for pr78733 (r243456 and 243486), but my
understanding is that the bug is still present since compiling
gcc.target/aarch64/pr78733.c with -mcmodel=large brings back the
:lo12: relocations (I'll submit a patch to add the test back if my
understanding is correct).
Cross built and regtested without issue for aarch64-linux-gnu,
aarch64-none-elf and aarch64_be-none-elf targets
OK for gcc-6-branch ?
Thanks
Yvan
gcc/ChangeLog
2017-06-22 Yvan Roux <yvan.r...@linaroi.org>
PR target/79041
Backport from mainline
2016-06-20 Wilco Dijkstra <wdijk...@arm.com>
* config/aarch64/aarch64.opt
(mpc-relative-literal-loads): Rename internal option name.
* config/aarch64/aarch64.c
(aarch64_nopcrelative_literal_loads): Rename to
aarch64_pcrelative_literal_loads.
(aarch64_expand_mov_immediate): Likewise.
(aarch64_secondary_reload): Likewise.
(aarch64_can_use_per_function_literal_pools_p): Likewise.
(aarch64_classify_symbol): Likewise.
(aarch64_override_options_after_change_1): Rename and simplify logic.
2016-01-19 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
* config/aarch64/aarch64-protos.h(aarch64_nopcrelative_literal_loads):
Delete.
* config/aarch64/aarch64.md
(aarch64_reload_movcp<GPF_TF:mode><P:mode>): Delete reference to
aarch64_nopcrelative_literal_loads.
(aarch64_reload_movcp<VALL:mode><P:mode>): Likewise.
gcc/testsuite/ChangeLog
2017-06-22 Yvan Roux <yvan.r...@linaroi.org>
PR target/79041
* gcc.target/aarch64/pr79041.c: New test.
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index fa97e29..7b10ff6 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -436,7 +436,6 @@ int aarch64_ccmp_mode_to_code (enum machine_mode mode);
bool extract_base_offset_in_addr (rtx mem, rtx *base, rtx *offset);
bool aarch64_operands_ok_for_ldpstp (rtx *, bool, enum machine_mode);
bool aarch64_operands_adjust_ok_for_ldpstp (rtx *, bool, enum machine_mode);
-extern bool aarch64_nopcrelative_literal_loads;
extern void aarch64_asm_output_pool_epilogue (FILE *, const char *,
tree, HOST_WIDE_INT);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e79165b..9b06320 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -152,7 +152,7 @@ enum aarch64_processor aarch64_tune = cortexa53;
unsigned long aarch64_tune_flags = 0;
/* Global flag for PC relative loads. */
-bool aarch64_nopcrelative_literal_loads;
+bool aarch64_pcrelative_literal_loads;
/* Support for command line parsing of boolean flags in the tuning
structures. */
@@ -1703,7 +1703,7 @@ aarch64_expand_mov_immediate (rtx dest, rtx imm)
we need to expand the literal pool access carefully.
This is something that needs to be done in a number
of places, so could well live as a separate function. */
- if (aarch64_nopcrelative_literal_loads)
+ if (!aarch64_pcrelative_literal_loads)
{
gcc_assert (can_create_pseudo_p ());
base = gen_reg_rtx (ptr_mode);
@@ -4028,7 +4028,7 @@ aarch64_classify_address (struct aarch64_address_info *info,
return ((GET_CODE (sym) == LABEL_REF
|| (GET_CODE (sym) == SYMBOL_REF
&& CONSTANT_POOL_ADDRESS_P (sym)
- && !aarch64_nopcrelative_literal_loads)));
+ && aarch64_pcrelative_literal_loads)));
}
return false;
@@ -5183,7 +5183,7 @@ aarch64_secondary_reload (bool in_p ATTRIBUTE_UNUSED, rtx x,
if (MEM_P (x) && GET_CODE (x) == SYMBOL_REF && CONSTANT_POOL_ADDRESS_P (x)
&& (SCALAR_FLOAT_MODE_P (GET_MODE (x))
|| targetm.vector_mode_supported_p (GET_MODE (x)))
- && aarch64_nopcrelative_literal_loads)
+ && !aarch64_pcrelative_literal_loads)
{
sri->icode = aarch64_constant_pool_reload_icode (mode);
return NO_REGS;
@@ -5517,7 +5517,7 @@ aarch64_uxt_size (int shift, HOST_WIDE_INT mask)
static inline bool
aarch64_can_use_per_function_literal_pools_p (void)
{
- return (!aarch64_nopcrelative_literal_loads
+ return (aarch64_pcrelative_literal_loads
|| aarch64_cmodel == AARCH64_CMODEL_LARGE);
}
@@ -8043,32 +8043,31 @@ aarch64_override_options_after_change_1 (struct gcc_options *opts)
opts->x_align_functions = aarch64_tune_params.function_align;
}
- /* If nopcrelative_literal_loads is set on the command line, this
+ /* We default to no pc-relative literal loads. */
+
+ aarch64_pcrelative_literal_loads = false;
+
+ /* If -mpc-relative-literal-loads is set on the command line, this
implies that the user asked for PC relative literal loads. */
- if (opts->x_nopcrelative_literal_loads == 1)
- aarch64_nopcrelative_literal_loads = false;
+ if (opts->x_pcrelative_literal_loads == 1)
+ aarch64_pcrelative_literal_loads = true;
- /* If it is not set on the command line, we default to no pc
- relative literal loads, unless the workaround for Cortex-A53
- erratum 843419 is in effect. */
/* This is PR70113. When building the Linux kernel with
CONFIG_ARM64_ERRATUM_843419, support for relocations
R_AARCH64_ADR_PREL_PG_HI21 and R_AARCH64_ADR_PREL_PG_HI21_NC is
removed from the kernel to avoid loading objects with possibly
- offending sequences. With nopcrelative_literal_loads, we would
+ offending sequences. Without -mpc-relative-literal-loads we would
generate such relocations, preventing the kernel build from
succeeding. */
- if (opts->x_nopcrelative_literal_loads == 2
- && !TARGET_FIX_ERR_A53_843419)
- aarch64_nopcrelative_literal_loads = true;
+ if (opts->x_pcrelative_literal_loads == 2
+ && TARGET_FIX_ERR_A53_843419)
+ aarch64_pcrelative_literal_loads = true;
- /* In the tiny memory model it makes no sense
- to disallow non PC relative literal pool loads
- as many other things will break anyway. */
- if (opts->x_nopcrelative_literal_loads
- && (aarch64_cmodel == AARCH64_CMODEL_TINY
- || aarch64_cmodel == AARCH64_CMODEL_TINY_PIC))
- aarch64_nopcrelative_literal_loads = false;
+ /* In the tiny memory model it makes no sense to disallow PC relative
+ literal pool loads. */
+ if (aarch64_cmodel == AARCH64_CMODEL_TINY
+ || aarch64_cmodel == AARCH64_CMODEL_TINY_PIC)
+ aarch64_pcrelative_literal_loads = true;
}
/* 'Unpack' up the internal tuning structs and update the options
@@ -9314,7 +9313,7 @@ aarch64_classify_symbol (rtx x, rtx offset)
/* This is alright even in PIC code as the constant
pool reference is always PC relative and within
the same translation unit. */
- if (nopcrelative_literal_loads
+ if (!aarch64_pcrelative_literal_loads
&& CONSTANT_POOL_ADDRESS_P (x))
return SYMBOL_SMALL_ABSOLUTE;
else
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 2c9f48c..8c3e216 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4775,7 +4775,7 @@
[(set (match_operand:GPF_TF 0 "register_operand" "=w")
(mem:GPF_TF (match_operand 1 "aarch64_constant_pool_symref" "S")))
(clobber (match_operand:P 2 "register_operand" "=&r"))]
- "TARGET_FLOAT && aarch64_nopcrelative_literal_loads"
+ "TARGET_FLOAT"
{
aarch64_expand_mov_immediate (operands[2], XEXP (operands[1], 0));
emit_move_insn (operands[0], gen_rtx_MEM (<GPF_TF:MODE>mode, operands[2]));
@@ -4788,7 +4788,7 @@
[(set (match_operand:VALL 0 "register_operand" "=w")
(mem:VALL (match_operand 1 "aarch64_constant_pool_symref" "S")))
(clobber (match_operand:P 2 "register_operand" "=&r"))]
- "TARGET_FLOAT && aarch64_nopcrelative_literal_loads"
+ "TARGET_FLOAT"
{
aarch64_expand_mov_immediate (operands[2], XEXP (operands[1], 0));
emit_move_insn (operands[0], gen_rtx_MEM (<VALL:MODE>mode, operands[2]));
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index c637ff4..bc50ec9 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -146,7 +146,7 @@ EnumValue
Enum(aarch64_abi) String(lp64) Value(AARCH64_ABI_LP64)
mpc-relative-literal-loads
-Target Report Save Var(nopcrelative_literal_loads) Init(2) Save
+Target Report Save Var(pcrelative_literal_loads) Init(2) Save
PC relative literal loads.
mlow-precision-recip-sqrt
diff --git a/gcc/testsuite/gcc.target/aarch64/pr79041.c b/gcc/testsuite/gcc.target/aarch64/pr79041.c
new file mode 100644
index 0000000..9ec97b2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr79041.c
@@ -0,0 +1,26 @@
+/* PR target/79041. Check that we don't generate the LO12 relocations
+ for -mpc-relative-literal-loads. */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcmodel=large -mpc-relative-literal-loads" } */
+
+extern int strcmp(const char *, const char *);
+extern char * strcpy(char *,const char *);
+
+static struct {
+ char *b;
+ char *c;
+} d[] = {
+ { "0", "000000000000000" },
+ { "1", "111111111111111" },
+};
+
+void
+e (const char *b, char *c)
+{
+ int i;
+ for (i = 0; i < 1; ++i)
+ if (!strcmp(d[i].b, b))
+ strcpy(c, d[i].c);
+}
+
+/* { dg-final { scan-assembler-not ":lo12:" } } */
