* [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
@ 2017-01-11 16:32 Kyrill Tkachov
2017-01-13 16:36 ` James Greenhalgh
0 siblings, 1 reply; 10+ messages in thread
From: Kyrill Tkachov @ 2017-01-11 16:32 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Earnshaw, James Greenhalgh, Marcus Shawcroft
[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]
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.
The actual bug here is that aarch64_classify_symbol uses nopcrelative_literal_loads instead of the correct aarch64_nopcrelative_literal_loads.
nopcrelative_literal_loads gets set to 1 if the user specifies -mpc-relative-literal-loads(!) whereas aarch64_nopcrelative_literal_loads gets
set to false, so that is the variable we want to check.
So this is the minimal patch that fixes this.
Bootstrapped and tested on aarch64-none-linux-gnu on the GCC 6 branch.
Ok for the branch?
Thanks,
Kyrill
2017-01-11 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/79041
* config/aarch64/aarch64.c (aarch64_classify_symbol): Use
aarch64_nopcrelative_literal_loads instead of
nopcrelative_literal_loads.
2017-01-11 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/79041
* gcc.target/aarch64/pr79041.c: New test.
[-- Attachment #2: tmp.patch --]
[-- Type: text/x-patch, Size: 1385 bytes --]
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83dbd57..fa61289 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9324,7 +9324,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_nopcrelative_literal_loads
&& CONSTANT_POOL_ADDRESS_P (x))
return SYMBOL_SMALL_ABSOLUTE;
else
diff --git a/gcc/testsuite/gcc.target/aarch64/pr79041.c b/gcc/testsuite/gcc.target/aarch64/pr79041.c
new file mode 100644
index 0000000..a23b1ae
--- /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:" } } */
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-01-11 16:32 [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol Kyrill Tkachov
@ 2017-01-13 16:36 ` James Greenhalgh
2017-01-16 15:34 ` Kyrill Tkachov
0 siblings, 1 reply; 10+ messages in thread
From: James Greenhalgh @ 2017-01-13 16:36 UTC (permalink / raw)
To: Kyrill Tkachov; +Cc: GCC Patches, Richard Earnshaw, Marcus Shawcroft, nd
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).
Thanks,
James
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-01-13 16:36 ` James Greenhalgh
@ 2017-01-16 15:34 ` Kyrill Tkachov
2017-06-22 18:42 ` Yvan Roux
0 siblings, 1 reply; 10+ messages in thread
From: Kyrill Tkachov @ 2017-01-16 15:34 UTC (permalink / raw)
To: James Greenhalgh; +Cc: GCC Patches, Richard Earnshaw, Marcus Shawcroft
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.
Thanks,
Kyri
> Thanks,
> James
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-01-16 15:34 ` Kyrill Tkachov
@ 2017-06-22 18:42 ` Yvan Roux
2017-06-27 9:17 ` Yvan Roux
2017-06-27 10:53 ` Wilco Dijkstra
0 siblings, 2 replies; 10+ messages in thread
From: Yvan Roux @ 2017-06-22 18:42 UTC (permalink / raw)
To: GCC Patches
Cc: Kyrill Tkachov, Wilco Dijkstra, James Greenhalgh, Richard Earnshaw
[-- Attachment #1: Type: text/plain, Size: 3428 bytes --]
Hi all,
On 16 January 2017 at 16:34, Kyrill Tkachov <kyrylo.tkachov@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.roux@linaroi.org>
PR target/79041
Backport from mainline
2016-06-20 Wilco Dijkstra <wdijkstr@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.tkachov@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.roux@linaroi.org>
PR target/79041
* gcc.target/aarch64/pr79041.c: New test.
[-- Attachment #2: pr79041.patch --]
[-- Type: text/x-patch, Size: 7661 bytes --]
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:" } } */
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-06-22 18:42 ` Yvan Roux
@ 2017-06-27 9:17 ` Yvan Roux
2017-06-27 10:53 ` Wilco Dijkstra
1 sibling, 0 replies; 10+ messages in thread
From: Yvan Roux @ 2017-06-27 9:17 UTC (permalink / raw)
To: GCC Patches
Cc: Kyrill Tkachov, Wilco Dijkstra, James Greenhalgh, Richard Earnshaw
Hi,
On 22 June 2017 at 20:42, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi all,
>
> On 16 January 2017 at 16:34, Kyrill Tkachov <kyrylo.tkachov@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 ?
Sorry for the fast ping, but since a 6.4 rc1 is planned for tomorrow,
do you think that this fix can be part of it ?
> Thanks
> Yvan
>
> gcc/ChangeLog
> 2017-06-22 Yvan Roux <yvan.roux@linaroi.org>
>
> PR target/79041
> Backport from mainline
> 2016-06-20 Wilco Dijkstra <wdijkstr@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.tkachov@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.roux@linaroi.org>
>
> PR target/79041
> * gcc.target/aarch64/pr79041.c: New test.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-06-22 18:42 ` Yvan Roux
2017-06-27 9:17 ` Yvan Roux
@ 2017-06-27 10:53 ` Wilco Dijkstra
2017-06-27 11:14 ` Yvan Roux
1 sibling, 1 reply; 10+ messages in thread
From: Wilco Dijkstra @ 2017-06-27 10:53 UTC (permalink / raw)
To: Yvan Roux, GCC Patches
Cc: Kyrill Tkachov, James Greenhalgh, Richard Earnshaw, nd
Hi Yvan,
> 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.
The patch looks good to me, however I can't approve it.
> 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).
You're right, eventhough -mpc-relative-literal-loads doesn't make much sense
in the large memory model, it seems best to keep the option orthogonal to
enable the workaround. I've prepared a patch to fix this on trunk/GCC7.
It also adds a test which we should add to your changes to GCC6 too.
Wilco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-06-27 10:53 ` Wilco Dijkstra
@ 2017-06-27 11:14 ` Yvan Roux
2017-07-03 10:48 ` Yvan Roux
0 siblings, 1 reply; 10+ messages in thread
From: Yvan Roux @ 2017-06-27 11:14 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Kyrill Tkachov, James Greenhalgh, Richard Earnshaw, nd
Hi Wilco
On 27 June 2017 at 12:53, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Hi Yvan,
>
>> 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.
>
> The patch looks good to me, however I can't approve it.
ok thanks for the review.
>> 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).
>
> You're right, eventhough -mpc-relative-literal-loads doesn't make much sense
> in the large memory model, it seems best to keep the option orthogonal to
> enable the workaround. I've prepared a patch to fix this on trunk/GCC7.
> It also adds a test which we should add to your changes to GCC6 too.
ok, I think it is what kugan's proposed earlier today in:
https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html
I agree that -mpc-relative-literal-loads and large memory model
doesn't make much sense, now it is what is used in kernel build
system, but if you handle that in a bigger fix already, that's awesome
:)
Thanks
Yvan
> Wilco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-06-27 11:14 ` Yvan Roux
@ 2017-07-03 10:48 ` Yvan Roux
2017-07-11 10:26 ` Yvan Roux
0 siblings, 1 reply; 10+ messages in thread
From: Yvan Roux @ 2017-07-03 10:48 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Kyrill Tkachov, James Greenhalgh, Richard Earnshaw, nd
On 27 June 2017 at 13:14, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi Wilco
>
> On 27 June 2017 at 12:53, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>> Hi Yvan,
>>
>>> 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.
>>
>> The patch looks good to me, however I can't approve it.
>
> ok thanks for the review.
>
>>> 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).
>>
>> You're right, eventhough -mpc-relative-literal-loads doesn't make much sense
>> in the large memory model, it seems best to keep the option orthogonal to
>> enable the workaround. I've prepared a patch to fix this on trunk/GCC7.
>> It also adds a test which we should add to your changes to GCC6 too.
>
> ok, I think it is what kugan's proposed earlier today in:
>
> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html
>
> I agree that -mpc-relative-literal-loads and large memory model
> doesn't make much sense, now it is what is used in kernel build
> system, but if you handle that in a bigger fix already, that's awesome
> :)
ping?
https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01708.html
> Thanks
> Yvan
>
>> Wilco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-07-03 10:48 ` Yvan Roux
@ 2017-07-11 10:26 ` Yvan Roux
2017-08-04 13:50 ` Yvan Roux
0 siblings, 1 reply; 10+ messages in thread
From: Yvan Roux @ 2017-07-11 10:26 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Kyrill Tkachov, James Greenhalgh, Richard Earnshaw, nd
On 3 July 2017 at 12:48, Yvan Roux <yvan.roux@linaro.org> wrote:
> On 27 June 2017 at 13:14, Yvan Roux <yvan.roux@linaro.org> wrote:
>> Hi Wilco
>>
>> On 27 June 2017 at 12:53, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>>> Hi Yvan,
>>>
>>>> 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.
>>>
>>> The patch looks good to me, however I can't approve it.
>>
>> ok thanks for the review.
>>
>>>> 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).
>>>
>>> You're right, eventhough -mpc-relative-literal-loads doesn't make much sense
>>> in the large memory model, it seems best to keep the option orthogonal to
>>> enable the workaround. I've prepared a patch to fix this on trunk/GCC7.
>>> It also adds a test which we should add to your changes to GCC6 too.
>>
>> ok, I think it is what kugan's proposed earlier today in:
>>
>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html
>>
>> I agree that -mpc-relative-literal-loads and large memory model
>> doesn't make much sense, now it is what is used in kernel build
>> system, but if you handle that in a bigger fix already, that's awesome
>> :)
>
> ping?
> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01708.html
ping
>> Thanks
>> Yvan
>>
>>> Wilco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
2017-07-11 10:26 ` Yvan Roux
@ 2017-08-04 13:50 ` Yvan Roux
0 siblings, 0 replies; 10+ messages in thread
From: Yvan Roux @ 2017-08-04 13:50 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, Kyrill Tkachov, James Greenhalgh, Richard Earnshaw, nd
On 11 July 2017 at 12:26, Yvan Roux <yvan.roux@linaro.org> wrote:
> On 3 July 2017 at 12:48, Yvan Roux <yvan.roux@linaro.org> wrote:
>> On 27 June 2017 at 13:14, Yvan Roux <yvan.roux@linaro.org> wrote:
>>> Hi Wilco
>>>
>>> On 27 June 2017 at 12:53, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>>>> Hi Yvan,
>>>>
>>>>> 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.
>>>>
>>>> The patch looks good to me, however I can't approve it.
>>>
>>> ok thanks for the review.
>>>
>>>>> 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).
>>>>
>>>> You're right, eventhough -mpc-relative-literal-loads doesn't make much sense
>>>> in the large memory model, it seems best to keep the option orthogonal to
>>>> enable the workaround. I've prepared a patch to fix this on trunk/GCC7.
>>>> It also adds a test which we should add to your changes to GCC6 too.
>>>
>>> ok, I think it is what kugan's proposed earlier today in:
>>>
>>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html
>>>
>>> I agree that -mpc-relative-literal-loads and large memory model
>>> doesn't make much sense, now it is what is used in kernel build
>>> system, but if you handle that in a bigger fix already, that's awesome
>>> :)
>>
>> ping?
>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01708.html
>
> ping
ping^3
I can include the new testcase added recently on trunk by Wilco
(gcc.target/aarch64/pr79041-2.c) if needed.
>>> Thanks
>>> Yvan
>>>
>>>> Wilco
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-08-04 13:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 16:32 [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol Kyrill Tkachov
2017-01-13 16:36 ` James Greenhalgh
2017-01-16 15:34 ` Kyrill Tkachov
2017-06-22 18:42 ` Yvan Roux
2017-06-27 9:17 ` Yvan Roux
2017-06-27 10:53 ` Wilco Dijkstra
2017-06-27 11:14 ` Yvan Roux
2017-07-03 10:48 ` Yvan Roux
2017-07-11 10:26 ` Yvan Roux
2017-08-04 13:50 ` Yvan Roux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).