Bug 52555 - [4.6/4.7/4.8 Regression] ICE unrecognizable insn with -ffast-math and __attribute__((optimize(xx)))
Summary: [4.6/4.7/4.8 Regression] ICE unrecognizable insn with -ffast-math and __attri...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.8.0
: P2 normal
Target Milestone: 4.6.4
Assignee: Aldy Hernandez
URL:
Keywords: ice-on-valid-code
Depends on: 37565
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-11 05:23 UTC by Roman Kononov
Modified: 2013-02-24 18:54 UTC (History)
4 users (show)

See Also:
Host:
Target: i?86-*-* x86_64-linux-gnu
Build:
Known to work: 4.5.3
Known to fail: 4.6.4, 4.7.0
Last reconfirmed: 2013-01-04 00:00:00


Attachments
untested patch (2.06 KB, patch)
2013-02-09 19:07 UTC, Aldy Hernandez
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roman Kononov 2012-03-11 05:23:48 UTC
$ cat test.cpp 
typedef unsigned long value_t;

struct foo {
    static float _factor;
    value_t _val;

    foo()
    :  _val(7)
    {
        _val=__builtin_ceil(_val * _factor);
    }
};

struct bar {
    void operator()(foo&);
};

void
__attribute__((optimize("O")))
test() {
    foo f;
    bar b;
    b(f);
}

int main() {
    test();
}

$ g++ -c -O2 -ffast-math test.cpp
test.cpp: In function ‘void test()’:
test.cpp:24:1: error: unrecognizable insn:
(insn 9 8 10 3 (parallel [
            (set (reg:XF 67)
                (unspec:XF [
                        (reg:XF 68)
                    ] UNSPEC_FRNDINT_CEIL))
            (clobber (reg:CC 17 flags))
        ]) test.cpp:10 -1
     (nil))
test.cpp:24:1: internal compiler error: in extract_insn, at recog.c:2109
Please submit a full bug report,

This happens with x86 and x86_64.
Comment 1 Roman Kononov 2012-03-12 12:51:20 UTC
It broke in r165823.
Comment 2 Uroš Bizjak 2012-03-12 17:11:35 UTC
(In reply to comment #1)
> It broke in r165823.

Author: jsm28
Date: Fri Oct 22 12:14:45 2010
New Revision: 165823

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=165823
Log:
	* target.h (enum opt_levels, struct default_options): New.
	* target.def (handle_ofast): Remove hook.
	(target_option.optimization): Change to
	target_option.optimization_table.
	...

Adding CC.
Comment 3 Jakub Jelinek 2012-08-15 14:01:34 UTC
I'd say it is wrong to clear the fast math flags when switching from global state -ffast-math to optimize attrubute -O1 or similar, of course with no outer -ffast-math or -Ofast attribute optimize Ofast should turn fast math options on.
How global Ofast with O optimize attribute should behave is another question.

Anyway, what happens here is that the ceil[sdx]f2 expanders and others use flag_unsafe_math_optimizations and flag_trapping_math options in their conditions (i.e. HAVE_ceil[sdx]f2 uses those and init_all_optabs sets it).  But apparently init_all_optabs is evaluated just once (several times only for really multi targettish targets like mips).  Thus it is evaluated when fast math options are on, but during expansion they are off.

In this light I'd say it is a target bug to use options that can change between different functions in insn conditions.
Comment 4 Jakub Jelinek 2012-08-15 14:21:54 UTC
Another option is to switch this_target_optabs not just for SWITCHABLE_TARGETs, but also when switching between cfun with different optimize/target attributes (at the time when the optimization/target node is created for a function it would try to init_all_optabs into a newly allocated array, and if that differed from the default, it would stick it into the node and switch at set_cfun time).

I actually wonder how could target attribute work the way it is implemented right now so far.
Comment 5 H.J. Lu 2012-08-15 14:34:53 UTC
(In reply to comment #4)
> I actually wonder how could target attribute work the way it is implemented
> right now so far.

It works by miracle.  See PR 37565.
Comment 6 Andrew Pinski 2013-01-04 03:09:43 UTC
Confirmed.
Comment 7 Aldy Hernandez 2013-02-07 22:43:25 UTC
I'll take a look.
Comment 8 Aldy Hernandez 2013-02-07 22:44:17 UTC
I'll take a look.
Comment 9 Aldy Hernandez 2013-02-07 23:24:42 UTC
Further reduced testcase, reproducible on both C and C++:

float farg;
unsigned val;

void __attribute__((optimize("O")))
test()
{
  val = __builtin_ceilf(farg);
}
Comment 10 Aldy Hernandez 2013-02-09 19:07:44 UTC
Created attachment 29404 [details]
untested patch

Untested proposed patch.
Comment 11 Jakub Jelinek 2013-02-11 09:00:50 UTC
Thanks for working on this.  I wonder why don't you store a struct target_optabs *
filed directly into the optimization node, instead of using a hash table for it.
Perhaps NULL could mean the default (&default_target_optabs), so that SWITCHABLE_TARGETS can still do something about it.  init_all_optabs is pretty expensive, so if we have say two functions with __attribute__((__optimize__(3))),
it would be better to just use the -O3 optabs the second time.  And, I wouldn't compare the result of init_all_optabs against saved copy of this_target_optabs, but &default_target_optabs.
Comment 12 Aldy Hernandez 2013-02-19 00:04:59 UTC
Author: aldyh
Date: Tue Feb 19 00:04:49 2013
New Revision: 196129

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196129
Log:
	PR target/52555
	* genopinit.c (raw_optab_handler): Use this_fn_optabs.
	(swap_optab_enable): Same.
	(init_all_optabs): Use argument instead of global.
	* tree.h (struct tree_optimization_option): New field
	target_optabs.
	* expr.h (init_all_optabs): Add argument to prototype.
	(TREE_OPTIMIZATION_OPTABS): New.
	(save_optabs_if_changed): Protoize.
	* optabs.h: Declare this_fn_optabs.
	* optabs.c (save_optabs_if_changed): New.
	Declare this_fn_optabs.
	(init_optabs): Add argument to init_all_optabs() call.
	* function.c (invoke_set_current_function_hook): Handle per
	function optabs.
	* function.h (struct function): New field optabs.
	* config/mips/mips.c (mips_set_mips16_mode): Handle when
	optimization_current_node has changed.
	* target-globals.h (save_target_globals_default_opts): Protoize.
	* target-globals.c (save_target_globals_default_opts): New.
c-family/
	* c-common.c (handle_optimize_attribute): Call
	save_optabs_if_changed.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr52555.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/config/mips/mips.c
    trunk/gcc/expr.h
    trunk/gcc/function.c
    trunk/gcc/function.h
    trunk/gcc/genopinit.c
    trunk/gcc/optabs.c
    trunk/gcc/optabs.h
    trunk/gcc/target-globals.c
    trunk/gcc/target-globals.h
    trunk/gcc/tree.h
Comment 13 Aldy Hernandez 2013-02-19 00:14:33 UTC
fixed on trunk
Comment 14 Jakub Jelinek 2013-02-24 18:54:42 UTC
Author: jakub
Date: Sun Feb 24 18:54:37 2013
New Revision: 196245

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196245
Log:
	PR target/52555
	* target-globals.c (save_target_globals): For init_reg_sets and
	target_reinit remporarily set this_fn_optabs to this_target_optabs.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/target-globals.c