[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #17 from ktkachov at gcc dot gnu.org --- Author: ktkachov Date: Fri Mar 11 15:27:24 2016 New Revision: 234141 URL: https://gcc.gnu.org/viewcvs?rev=234141&root=gcc&view=rev Log: [AArch64] PR target/70002: Make aarch64_set_current_function play nice with pragma resetting PR target/70002 * config/aarch64/aarch64-protos.h (aarch64_save_restore_target_globals): New prototype. * config/aarch64/aarch64-c.c (aarch64_pragma_target_parse): Call the above when popping pragma. * config/aarch64/aarch64.c (aarch64_save_restore_target_globals): New function. (aarch64_set_current_function): Rewrite using the above. PR target/70002 PR target/69245 * gcc.target/aarch64/pr69245_2.c: New test. Added: trunk/gcc/testsuite/gcc.target/aarch64/pr69245_2.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/aarch64/aarch64-c.c trunk/gcc/config/aarch64/aarch64-protos.h trunk/gcc/config/aarch64/aarch64.c trunk/gcc/testsuite/ChangeLog
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #16 from ktkachov at gcc dot gnu.org --- Author: ktkachov Date: Fri Feb 26 16:02:21 2016 New Revision: 233745 URL: https://gcc.gnu.org/viewcvs?rev=233745&root=gcc&view=rev Log: [AArch64] Set TREE_TARGET_GLOBALS in aarch64_set_current_function when new tree is the default node to recalculate optab availability PR target/69245 * config/aarch64/aarch64.c (aarch64_set_current_function): Save/restore target globals when switching to target_option_default_node. * gcc.target/aarch64/pr69245_1.c: New test. Added: trunk/gcc/testsuite/gcc.target/aarch64/pr69245_1.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/aarch64/aarch64.c trunk/gcc/testsuite/ChangeLog
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 Jakub Jelinek changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #15 from Jakub Jelinek --- Assuming fixed.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #14 from chrbr at gcc dot gnu.org --- Author: chrbr Date: Wed Jan 27 13:03:45 2016 New Revision: 232872 URL: https://gcc.gnu.org/viewcvs?rev=232872&root=gcc&view=rev Log: 2016-01-20 Christian Bruel PR target/69245 * config/arm/arm-c.c (arm_pragma_target_parse): Add comments. Move arm_reset_previous_fndecl and set_target_option_current_node in the conditional part. Call save_restore_target_globals. * config/arm/arm.c (arm_set_current_function): Refactor to better support #pragma target and attribute mix. Call save_restore_target_globals. * config/arm/arm-protos.h (save_restore_target_globals): New function. Added: trunk/gcc/testsuite/gcc.target/arm/pr69245.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/arm/arm-c.c trunk/gcc/config/arm/arm-protos.h trunk/gcc/config/arm/arm.c trunk/gcc/testsuite/ChangeLog
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245
--- Comment #13 from James Greenhalgh ---
This is a similar case I reduced from the Ubuntu rebuild failures, hitting the
"max" idiom recognition:
---
#pragma GCC push_options
#pragma GCC target("fpu=crypto-neon-fp-armv8")
static void
foo (void)
{
}
#pragma GCC pop_options
float
max_idiom (float x, float y)
{
return x > y ? x : y;
}
---
Compile with -march=armv7-a -mfloat-abi=hard -mfpu=neon -O2 -ffast-math
Presumably the same root cause.
bug.c:12:1: error: unrecognizable insn:
}
^
(insn 7 4 8 2 (set (reg:SF 113)
(smax:SF (reg/v:SF 112 [ y ])
(reg/v:SF 111 [ x ]))) bug2.c:11 -1
(nil))
bug.c:12:1: internal compiler error: in extract_insn, at recog.c:2286
0xa411e9 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
.../gcc/rtl-error.c:108
0xa41208 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
.../gcc/rtl-error.c:116
0xa1404d extract_insn(rtx_insn*)
.../gcc/recog.c:2286
0x82b664 instantiate_virtual_regs_in_insn
.../gcc/function.c:1582
0x82b664 instantiate_virtual_regs
.../gcc/function.c:1950
0x82b664 execute
.../gcc/function.c:1999
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See for instructions.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 chrbr at gcc dot gnu.org changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |chrbr at gcc dot gnu.org --- Comment #12 from chrbr at gcc dot gnu.org --- Created attachment 37357 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37357&action=edit new arm_set_current_function implementation A complete refactoring of arm_set_current_function. Much simpler. Also fixes pr68896 due to discrepancies between #pragma GCC target and set_current_function global_options cl_target_option_restore restores. Still want to play a little bit with it before sending it to the list.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 chrbr at gcc dot gnu.org changed: What|Removed |Added Attachment #37332|0 |1 is obsolete|| --- Comment #11 from chrbr at gcc dot gnu.org --- Created attachment 37333 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37333&action=edit factorize restoration code better with the patch
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 chrbr at gcc dot gnu.org changed: What|Removed |Added Attachment #37329|0 |1 is obsolete|| --- Comment #10 from chrbr at gcc dot gnu.org --- Created attachment 37332 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37332&action=edit factorize restoration code
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #9 from chrbr at gcc dot gnu.org --- Created attachment 37329 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37329&action=edit factorize restoration code works for this case but fully untested
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #8 from chrbr at gcc dot gnu.org --- Probably the same latent bug in the else part of the pragma_parse, for the case where arm_valid_target_attribute_tree returns NULL.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #7 from chrbr at gcc dot gnu.org --- also it could be wrong to wait for arm_set_current_function when handling a pragma GCC target because we could need the state in the global scope.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #6 from chrbr at gcc dot gnu.org --- (In reply to ktkachov from comment #5) > The problem is we never end up setting TREE_TARGET_GLOBALS for fn2. > From what I can gather that's TARGET_SET_CURRENT_FUNCTION's job (although > the documentation for these things is scarce :( ) > > When we call arm_set_current_function for fn2 the old_tree is NULL because > arm_pragma_target_parse will have reset arm_previous_fndecl when popping the > target pragma. > > new_tree will also be NULL because DECL_FUNCTION_SPECIFIC_TARGET for fndecl > is only supposed to be set in the target attribute parsing code, but not the > pragma parsing code (again, documentation here is scarce). > > So we end up bypassing all the content in arm_set_current_function and not > setting TREE_TARGET_GLOBALS which means the midend doesn't try to reset the > optabs for fn2. > > x86 gets away with this because ix86_reset_previous_fndecl besides clearing > the previous fndecl cache also sets TREE_TARGET_GLOBALS for > target_option_current_node. > > rs6000 seems to never reset its previous fndecl (rs6000_previous_fndecl) so > its version old_tree will always point to some tree (AFAICT). > > Ugh. So for arm my thoughts are we should not exit early on the old_tree == > new_tree condition if both trees are NULL but rather follow the second arm > of the if statement in arm_set_current_function where we set new_tree to > target_option_current_code. > > Hacking a prototype for that idea fixes the testcase for me. > Christian, what do you think? arm_reset_current_function assumes indeed that all the state switches are done, so it would be more consistent to call restore_target_globals in pragma_target_parse as well instead of splitting the work in 2 different places. Maybe factorizing the common code from both ?
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #5 from ktkachov at gcc dot gnu.org --- The problem is we never end up setting TREE_TARGET_GLOBALS for fn2. >From what I can gather that's TARGET_SET_CURRENT_FUNCTION's job (although the documentation for these things is scarce :( ) When we call arm_set_current_function for fn2 the old_tree is NULL because arm_pragma_target_parse will have reset arm_previous_fndecl when popping the target pragma. new_tree will also be NULL because DECL_FUNCTION_SPECIFIC_TARGET for fndecl is only supposed to be set in the target attribute parsing code, but not the pragma parsing code (again, documentation here is scarce). So we end up bypassing all the content in arm_set_current_function and not setting TREE_TARGET_GLOBALS which means the midend doesn't try to reset the optabs for fn2. x86 gets away with this because ix86_reset_previous_fndecl besides clearing the previous fndecl cache also sets TREE_TARGET_GLOBALS for target_option_current_node. rs6000 seems to never reset its previous fndecl (rs6000_previous_fndecl) so its version old_tree will always point to some tree (AFAICT). Ugh. So for arm my thoughts are we should not exit early on the old_tree == new_tree condition if both trees are NULL but rather follow the second arm of the if statement in arm_set_current_function where we set new_tree to target_option_current_code. Hacking a prototype for that idea fixes the testcase for me. Christian, what do you think?
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #4 from Jakub Jelinek --- I'd recommend writing similar testcase for x86_64 with #pragma GCC target "fma" instead and debug side by side what is different in the target pragma handling.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 --- Comment #3 from ktkachov at gcc dot gnu.org --- Hmm... The code in tree-ssa-math-opts.c that creates the FMA_EXPR avoids doing it if the target doesn't support an fma optab. So the optab handler should have rejected it. The arm fma optab in vfp.md has the condition TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA. TARGET_FMA is defined as (TARGET_VFP && TARGET_FPU_REV >= 4) and TARGET_FPU_REV is (all_fpus[arm_fpu_index].rev) at the point in convert_mult_to_fma in fn2 when the FMA_EXPR is created if I print in gdb the expression: all_fpus[global_options.x_arm_fpu_index].rev I get '3'. So TARGET_FMA should be properly set to 0 at this point. Something's weird going on.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245
ktkachov at gcc dot gnu.org changed:
What|Removed |Added
CC||chrbr at gcc dot gnu.org
--- Comment #2 from ktkachov at gcc dot gnu.org ---
So the fpu given on the command line does not support vfma but the global
variables are enclosed in an fpu pragma that does.
But the fma itself occurs outside the scope of fpu=crypto-neon-fp-armv8.
More C-friendly testcase:
#pragma GCC push_options
#pragma GCC target "fpu=crypto-neon-fp-armv8"
int a, c, d;
float b;
static int fn1 ()
{
return 0;
}
#pragma GCC pop_options
void fn2 ()
{
d = b * c + a;
}
interestingly, deleting the definition of fn1 we don't ICE.
[Bug target/69245] [6 Regression] ICE in extract_insn, at recog.c:2286 on arm-linux-gnueabihf
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69245 ktkachov at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2016-01-12 Target Milestone|--- |6.0 Ever confirmed|0 |1 --- Comment #1 from ktkachov at gcc dot gnu.org --- Confirmed.
