* [PATCH][AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability
@ 2016-02-25 11:04 Kyrill Tkachov
2016-02-26 15:51 ` James Greenhalgh
0 siblings, 1 reply; 5+ messages in thread
From: Kyrill Tkachov @ 2016-02-25 11:04 UTC (permalink / raw)
To: GCC Patches; +Cc: Marcus Shawcroft, Richard Earnshaw, James Greenhalgh
[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]
Hi all,
Seems like aarch64 is suffering from something similar to PR 69245 as well.
If a target pragma sets the target state to the same as the target_option_default_node
the node is just a pointer to target_option_default_node rather than a distinct identical
node. So we must still restore the target globals even when setting to target_option_default_node
in order to force the midend to recompute the availability of various optabs.
If we don't do it, we can get in a problem like in the testcase where the isa_flags are all
set correctly, but the optab HAVE_* predicates have not been recomputed.
There is also a related issue present when popping/resetting target pragmas for which
I'll send out a patch separately.
Bootstrapped and tested on aarch64.
Ok for trunk?
Thanks,
Kyrill
2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/69245
* config/aarch64/aarch64.c (aarch64_set_current_function): Save/restore
target globals when switching to target_option_default_node.
2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/69245
* gcc.target/aarch64/pr69245_1.c: New test.
[-- Attachment #2: aarch64-set-func-default.patch --]
[-- Type: text/x-patch, Size: 1139 bytes --]
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 2e69e345545e591d1de76c2d175aac476e6e1107..07590bd43ca81e182ab1e2ba9596cf6881173729 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8593,7 +8593,7 @@ aarch64_set_current_function (tree fndecl)
if (old_tree == new_tree)
;
- else if (new_tree && new_tree != target_option_default_node)
+ else if (new_tree)
{
cl_target_option_restore (&global_options,
TREE_TARGET_OPTION (new_tree));
diff --git a/gcc/testsuite/gcc.target/aarch64/pr69245_1.c b/gcc/testsuite/gcc.target/aarch64/pr69245_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..dcc542b2a8686f1b323433624d4df65ca9e96b36
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr69245_1.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=armv8-a+fp -fomit-frame-pointer" } */
+
+#pragma GCC target "arch=armv8-a+nofp"
+long a;
+static void
+fn1 ()
+{
+}
+
+#pragma GCC target "arch=armv8-a+fp"
+float
+fn2 (float a)
+{
+ return a + 2.0;
+}
+
+/* { dg-final { scan-assembler-not "__addsf3" } } */
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability
2016-02-25 11:04 [PATCH][AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability Kyrill Tkachov
@ 2016-02-26 15:51 ` James Greenhalgh
2016-02-29 14:10 ` Christophe Lyon
0 siblings, 1 reply; 5+ messages in thread
From: James Greenhalgh @ 2016-02-26 15:51 UTC (permalink / raw)
To: Kyrill Tkachov; +Cc: GCC Patches, Marcus Shawcroft, Richard Earnshaw
On Thu, Feb 25, 2016 at 11:04:21AM +0000, Kyrill Tkachov wrote:
> Hi all,
>
> Seems like aarch64 is suffering from something similar to PR 69245 as well.
> If a target pragma sets the target state to the same as the
> target_option_default_node the node is just a pointer to
> target_option_default_node rather than a distinct identical node. So we must
> still restore the target globals even when setting to
> target_option_default_node in order to force the midend to recompute the
> availability of various optabs.
>
> If we don't do it, we can get in a problem like in the testcase where the
> isa_flags are all set correctly, but the optab HAVE_* predicates have not
> been recomputed.
>
> There is also a related issue present when popping/resetting target pragmas
> for which I'll send out a patch separately.
>
> Bootstrapped and tested on aarch64.
>
> Ok for trunk?
OK.
Thanks,
James
> Thanks,
> Kyrill
>
>
> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> PR target/69245
> * config/aarch64/aarch64.c (aarch64_set_current_function): Save/restore
> target globals when switching to target_option_default_node.
>
> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> PR target/69245
> * gcc.target/aarch64/pr69245_1.c: New test.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability
2016-02-26 15:51 ` James Greenhalgh
@ 2016-02-29 14:10 ` Christophe Lyon
2016-02-29 14:28 ` Kyrill Tkachov
0 siblings, 1 reply; 5+ messages in thread
From: Christophe Lyon @ 2016-02-29 14:10 UTC (permalink / raw)
To: James Greenhalgh
Cc: Kyrill Tkachov, GCC Patches, Marcus Shawcroft, Richard Earnshaw
On 26 February 2016 at 16:51, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Thu, Feb 25, 2016 at 11:04:21AM +0000, Kyrill Tkachov wrote:
>> Hi all,
>>
>> Seems like aarch64 is suffering from something similar to PR 69245 as well.
>> If a target pragma sets the target state to the same as the
>> target_option_default_node the node is just a pointer to
>> target_option_default_node rather than a distinct identical node. So we must
>> still restore the target globals even when setting to
>> target_option_default_node in order to force the midend to recompute the
>> availability of various optabs.
>>
>> If we don't do it, we can get in a problem like in the testcase where the
>> isa_flags are all set correctly, but the optab HAVE_* predicates have not
>> been recomputed.
>>
>> There is also a related issue present when popping/resetting target pragmas
>> for which I'll send out a patch separately.
>>
>> Bootstrapped and tested on aarch64.
>>
>> Ok for trunk?
>
> OK.
>
Hi Kyrill,
Since this patch, I'm seeing:
gcc.dg/torture/pr52429.c -O2 -flto -fno-use-linker-plugin
-flto-partition=none (internal compiler error)
on target aarch64-none-linux-gnu
The log has:
spawn -ignore SIGHUP
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/gcc/xgcc
-B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc
3/gcc/ /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c
-fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -flto
-fno-use-linker-pl
ugin -flto-partition=none -g -ftree-parallelize-loops=4
-DSTACK_SIZE=16384 -S -o pr52429.s
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c:
In function 'foo':
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c:24:1:
internal compiler error: Segmentation fault
0xade075 crash_signal
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:335
0x91f88e record_operand_costs
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1293
0x91fdba scan_one_insn
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1471
0x91fdba process_bb_for_costs
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1592
0x9214e7 find_costs_and_classes
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1699
0x922552 ira_set_pseudo_classes(bool, _IO_FILE*)
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:2239
0x1061ecd alloc_global_sched_pressure_data
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7244
0x1061ecd sched_init()
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7394
0x10679ed haifa_sched_init()
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7406
0xa84fae schedule_insns()
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3504
0xa85864 rest_of_handle_sched
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3717
0xa85864 execute
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3825
Don't you see this regression on your side?
Thanks,
Christophe.
> Thanks,
> James
>
>> Thanks,
>> Kyrill
>>
>>
>> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>
>> PR target/69245
>> * config/aarch64/aarch64.c (aarch64_set_current_function): Save/restore
>> target globals when switching to target_option_default_node.
>>
>> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>
>> PR target/69245
>> * gcc.target/aarch64/pr69245_1.c: New test.
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability
2016-02-29 14:10 ` Christophe Lyon
@ 2016-02-29 14:28 ` Kyrill Tkachov
2016-02-29 14:33 ` Christophe Lyon
0 siblings, 1 reply; 5+ messages in thread
From: Kyrill Tkachov @ 2016-02-29 14:28 UTC (permalink / raw)
To: Christophe Lyon, James Greenhalgh
Cc: Kyrill Tkachov, GCC Patches, Marcus Shawcroft, Richard Earnshaw
Hi Crhistophe,
On 29/02/16 14:10, Christophe Lyon wrote:
> On 26 February 2016 at 16:51, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> On Thu, Feb 25, 2016 at 11:04:21AM +0000, Kyrill Tkachov wrote:
>>> Hi all,
>>>
>>> Seems like aarch64 is suffering from something similar to PR 69245 as well.
>>> If a target pragma sets the target state to the same as the
>>> target_option_default_node the node is just a pointer to
>>> target_option_default_node rather than a distinct identical node. So we must
>>> still restore the target globals even when setting to
>>> target_option_default_node in order to force the midend to recompute the
>>> availability of various optabs.
>>>
>>> If we don't do it, we can get in a problem like in the testcase where the
>>> isa_flags are all set correctly, but the optab HAVE_* predicates have not
>>> been recomputed.
>>>
>>> There is also a related issue present when popping/resetting target pragmas
>>> for which I'll send out a patch separately.
>>>
>>> Bootstrapped and tested on aarch64.
>>>
>>> Ok for trunk?
>> OK.
>>
> Hi Kyrill,
>
> Since this patch, I'm seeing:
> gcc.dg/torture/pr52429.c -O2 -flto -fno-use-linker-plugin
> -flto-partition=none (internal compiler error)
> on target aarch64-none-linux-gnu
>
> The log has:
> spawn -ignore SIGHUP
> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/gcc/xgcc
> -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc
> 3/gcc/ /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c
> -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -flto
> -fno-use-linker-pl
> ugin -flto-partition=none -g -ftree-parallelize-loops=4
> -DSTACK_SIZE=16384 -S -o pr52429.s
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c:
> In function 'foo':
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c:24:1:
> internal compiler error: Segmentation fault
> 0xade075 crash_signal
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:335
> 0x91f88e record_operand_costs
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1293
> 0x91fdba scan_one_insn
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1471
> 0x91fdba process_bb_for_costs
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1592
> 0x9214e7 find_costs_and_classes
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1699
> 0x922552 ira_set_pseudo_classes(bool, _IO_FILE*)
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:2239
> 0x1061ecd alloc_global_sched_pressure_data
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7244
> 0x1061ecd sched_init()
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7394
> 0x10679ed haifa_sched_init()
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7406
> 0xa84fae schedule_insns()
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3504
> 0xa85864 rest_of_handle_sched
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3717
> 0xa85864 execute
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3825
>
> Don't you see this regression on your side?
I've reproduced it just now.
I had not seen it initially because the test requires a pthread target,
so it was marked UNSUPPORTED when I tested aarch64-none-elf :(
But this looks like a latent bug elsewhere.
I'll try to investigate, can you please open a PR?
Thanks,
Kyrill
> Thanks,
>
> Christophe.
>
>
>
>> Thanks,
>> James
>>
>>> Thanks,
>>> Kyrill
>>>
>>>
>>> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>>
>>> PR target/69245
>>> * config/aarch64/aarch64.c (aarch64_set_current_function): Save/restore
>>> target globals when switching to target_option_default_node.
>>>
>>> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>>
>>> PR target/69245
>>> * gcc.target/aarch64/pr69245_1.c: New test.
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability
2016-02-29 14:28 ` Kyrill Tkachov
@ 2016-02-29 14:33 ` Christophe Lyon
0 siblings, 0 replies; 5+ messages in thread
From: Christophe Lyon @ 2016-02-29 14:33 UTC (permalink / raw)
To: Kyrill Tkachov
Cc: James Greenhalgh, GCC Patches, Marcus Shawcroft, Richard Earnshaw
On 29 February 2016 at 15:28, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi Crhistophe,
>
>
> On 29/02/16 14:10, Christophe Lyon wrote:
>>
>> On 26 February 2016 at 16:51, James Greenhalgh <james.greenhalgh@arm.com>
>> wrote:
>>>
>>> On Thu, Feb 25, 2016 at 11:04:21AM +0000, Kyrill Tkachov wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Seems like aarch64 is suffering from something similar to PR 69245 as
>>>> well.
>>>> If a target pragma sets the target state to the same as the
>>>> target_option_default_node the node is just a pointer to
>>>> target_option_default_node rather than a distinct identical node. So we
>>>> must
>>>> still restore the target globals even when setting to
>>>> target_option_default_node in order to force the midend to recompute the
>>>> availability of various optabs.
>>>>
>>>> If we don't do it, we can get in a problem like in the testcase where
>>>> the
>>>> isa_flags are all set correctly, but the optab HAVE_* predicates have
>>>> not
>>>> been recomputed.
>>>>
>>>> There is also a related issue present when popping/resetting target
>>>> pragmas
>>>> for which I'll send out a patch separately.
>>>>
>>>> Bootstrapped and tested on aarch64.
>>>>
>>>> Ok for trunk?
>>>
>>> OK.
>>>
>> Hi Kyrill,
>>
>> Since this patch, I'm seeing:
>> gcc.dg/torture/pr52429.c -O2 -flto -fno-use-linker-plugin
>> -flto-partition=none (internal compiler error)
>> on target aarch64-none-linux-gnu
>>
>> The log has:
>> spawn -ignore SIGHUP
>>
>> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/gcc/xgcc
>> -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc
>> 3/gcc/
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c
>> -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -flto
>> -fno-use-linker-pl
>> ugin -flto-partition=none -g -ftree-parallelize-loops=4
>> -DSTACK_SIZE=16384 -S -o pr52429.s
>>
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c:
>> In function 'foo':
>>
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/torture/pr52429.c:24:1:
>> internal compiler error: Segmentation fault
>> 0xade075 crash_signal
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:335
>> 0x91f88e record_operand_costs
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1293
>> 0x91fdba scan_one_insn
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1471
>> 0x91fdba process_bb_for_costs
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1592
>> 0x9214e7 find_costs_and_classes
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:1699
>> 0x922552 ira_set_pseudo_classes(bool, _IO_FILE*)
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira-costs.c:2239
>> 0x1061ecd alloc_global_sched_pressure_data
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7244
>> 0x1061ecd sched_init()
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7394
>> 0x10679ed haifa_sched_init()
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/haifa-sched.c:7406
>> 0xa84fae schedule_insns()
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3504
>> 0xa85864 rest_of_handle_sched
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3717
>> 0xa85864 execute
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/sched-rgn.c:3825
>>
>> Don't you see this regression on your side?
>
>
> I've reproduced it just now.
> I had not seen it initially because the test requires a pthread target,
> so it was marked UNSUPPORTED when I tested aarch64-none-elf :(
That's what I noticed too
> But this looks like a latent bug elsewhere.
> I'll try to investigate, can you please open a PR?
Sure, this is PR 70016:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70016
Christophe
> Thanks,
> Kyrill
>
>
>> Thanks,
>>
>> Christophe.
>>
>>
>>
>>> Thanks,
>>> James
>>>
>>>> Thanks,
>>>> Kyrill
>>>>
>>>>
>>>> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>>>
>>>> PR target/69245
>>>> * config/aarch64/aarch64.c (aarch64_set_current_function):
>>>> Save/restore
>>>> target globals when switching to target_option_default_node.
>>>>
>>>> 2016-02-25 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>>>
>>>> PR target/69245
>>>> * gcc.target/aarch64/pr69245_1.c: New test.
>>>
>>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-29 14:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-25 11:04 [PATCH][AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability Kyrill Tkachov
2016-02-26 15:51 ` James Greenhalgh
2016-02-29 14:10 ` Christophe Lyon
2016-02-29 14:28 ` Kyrill Tkachov
2016-02-29 14:33 ` Christophe Lyon
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).