[Bug middle-end/56524] New: Compiler ICE when compiling with -mips16

[Bug middle-end/56524] New: Compiler ICE when compiling with -mips16

[sje at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

             Bug #: 56524
           Summary: Compiler ICE when compiling with -mips16
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: sje@gcc.gnu.org
                CC: aldyh@redhat.com
            Target: mips*-*-*


This test program (from glibc) generates an ICE when compiled with -mips16. 
The internal error we are hitting was part of the fix for pr52555.  The test
program is a bit finicky, I think it is dependent on the order that GCC
processes the functions.  I don't know if this is a bug in the generic code
or in mips specific code and how mips handles the mips16 flag.


% cat mktime.i
unsigned int __bswap_32 (unsigned int __bsx)
{ return __builtin_bswap32 (__bsx); }

#pragma GCC optimize ("wrapv")

inline int leapyear (long long int year)
{
  return
    ((year & 3) == 0
     && (year % 100 != 0
  || ((year / 100) & 3) == (- (1900 / 100) & 3)));
}

mips-mti-linux-gnu-gcc -c -mips16 mktime.i
mktime.i:7:1: internal compiler error: in save_optabs_if_changed, at
optabs.c:6221
 {
 ^
0x8198e5 save_optabs_if_changed(tree_node*)
    /local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/optabs.c:6221
0x4b090b decl_attributes(tree_node**, tree_node*, int)
    /local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/attribs.c:545
0x4cf728 start_function(c_declspecs*, c_declarator*, tree_node*)
    /local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/c/c-decl.c:7727
0x557114 c_common_parse_file()
    /local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/c-family/c-opts.c:1046
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-03-05
                 CC|                            |aldyh at gcc dot gnu.org
   Target Milestone|---                         |4.8.0
            Summary|Compiler ICE when compiling |[4.8 Regression] Compiler
                   |with -mips16                |ICE when compiling with
                   |                            |-mips16
     Ever Confirmed|0                           |1

--- Comment #1 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-05 16:23:34 UTC ---
Confirmed.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #2 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-05 16:24:35 UTC ---
Created attachment 29588
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29588
reduced testcase

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #3 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-06 17:54:41 UTC ---
This is ICEing here:

  /* ?? If this fails, we should temporarily restore the default
     target first (set_cfun (NULL) ??), do the rest of this function,
     and then restore it.  */
  gcc_assert (this_target_optabs == &default_target_optabs);

In this case, this_target_optabs is being set in:

  mips_set_mips16_mode() ->
    save_target_globals_default_opts() ->
      save_target_globals() ->
        restore_target_globals() 
             ...
             this_target_optabs = g->optabs;

We can't do exactly as the comment above the ICE says, since cfun isn't even
set at this point (we are being called from the parser).

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[rsandifo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> 2013-03-06 20:59:01 UTC ---
Created attachment 29601
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29601
Tentative patch

How about this patch?  Only lightly tested.  It fixes the testcase
and the pr43564.c failure that I'm seeing on MIPS16 runs.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #5 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-06 23:01:26 UTC ---
(In reply to comment #4)
> Created attachment 29601 [details]
> Tentative patch
> 
> How about this patch?  Only lightly tested.  It fixes the testcase
> and the pr43564.c failure that I'm seeing on MIPS16 runs.

Boy, this is getting quite confusing.  Do you think perhaps we could document
the plethora of target optabs, fn_optabs, base_optabs, etc.

And BTW, thanks for the patch Richard.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-07 08:14:17 UTC ---
I don't like the patch too much.  Why are you removing the cfun->optabs stuff?
set_cfun is called pretty often, e.g. during IPA switching to various functions
and back, when you remove cfun->optabs, it will constantly need to recompute
the optabs (quite expensive operation) if switching between mips16 vs.
non-mips16 functions with different optimization attributes.  The base_optabs
stuff is fine, but I'd just remove the ggc_free and in
invoke_set_current_function_hook
restore the saving of the computed optabs into cfun->optabs and using it if
already set.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[sje at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #7 from Steve Ellcey <sje at gcc dot gnu.org> 2013-03-07 15:56:19 UTC ---
I included the patch in my nightly build and test and encountered no problems
with GCC or GLIBC.  I was able to completely build GLIBC in mips16 mode.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[rsandifo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> 2013-03-07 18:40:59 UTC ---
(In reply to comment #6)
> I don't like the patch too much.  Why are you removing the cfun->optabs stuff?

Because Aldy says, it's getting confusing having all these optabs
flying around.

> set_cfun is called pretty often, e.g. during IPA switching to various functions
> and back, when you remove cfun->optabs, it will constantly need to recompute
> the optabs (quite expensive operation) if switching between mips16 vs.
> non-mips16 functions with different optimization attributes.

I don't think that's true, is it?  It would only recompute the flags
if switching between mips16 and nomips16 functions that have the
_same non-default_ optimisation flags.  TBH I think that's so rare
that it's not worth worrying about for 4.8.

I think this optabs stuff is only a stopgap.  As I said in the thread
on list, and as you said in the original PR, I think we should seriously
consider switching the full target_globals structure instead.  We can
see how things look then as to whether it's worth caching in both cfun
and the optimisation node.

Richard

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-07 19:02:14 UTC ---
It didn't seem to be too confusing and would require only a few extra lines of
code (that have been written already).
Anyway, I guess I don't care much, so if this works, go ahead and check it in.
If you want, you might want to get rid of that ggc_free followed (almost)
immediately by ggc_atomic_cleared.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[rsandifo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #10 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> 2013-03-07 19:13:07 UTC ---
(In reply to comment #9)
> It didn't seem to be too confusing and would require only a few extra lines of
> code (that have been written already).
> Anyway, I guess I don't care much, so if this works, go ahead and check it in.
> If you want, you might want to get rid of that ggc_free followed (almost)
> immediately by ggc_atomic_cleared.

Yeah, I'll fix that before submitting, thanks.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[rsandifo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #11 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> 2013-03-09 07:54:08 UTC ---
Author: rsandifo
Date: Sat Mar  9 07:54:02 2013
New Revision: 196570

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196570
Log:
gcc/
    PR middle-end/56524
    * tree.h (tree_optimization_option): Rename target_optabs to optabs.
    Add base_optabs.
    (TREE_OPTIMIZATION_OPTABS): Update after previous field change.
    (TREE_OPTIMIZATION_BASE_OPTABS): New macro.
    (save_optabs_if_changed): Replace with...
    (init_tree_optimization_optabs): ...this.
    * optabs.c (save_optabs_if_changed): Rename to...
    (init_tree_optimization_optabs): ...this.  Take the optimization node
    as argument.  Do nothing if the base optabs are already correct.
    Reuse the existing TREE_OPTIMIZATION_OPTABS memory if we need
    to recompute optabs.
    * function.h (function): Remove optabs field.
    * function.c (invoke_set_current_function_hook): Call
    init_tree_optimization_optabs.  Use the result to initialize
    this_fn_optabs.

gcc/c-family/
    PR middle-end/56524
    * c-common.c (handle_optimize_attribute): Don't call
    save_optabs_if_changed.

gcc/testsuite/
    PR middle-end/56524
    * gcc.target/mips/pr56524.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/mips/pr56524.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/function.c
    trunk/gcc/function.h
    trunk/gcc/optabs.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree.h

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[rsandifo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

rsandifo@gcc.gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> 2013-03-09 08:01:54 UTC ---
Patch applied.

[Bug middle-end/56524] [4.8 Regression] Compiler ICE when compiling with -mips16

[clm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56524

--- Comment #13 from clm at gcc dot gnu.org 2013-03-13 15:38:37 UTC ---
Author: clm
Date: Wed Mar 13 15:37:36 2013
New Revision: 196635

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196635
Log:
2013-03-13  Catherine Moore  <clm@codesourcery.com>

        Merge from Mainline:
        2013-03-09  Richard Sandiford  <rdsandiford@googlemail.com>
        PR middle-end/56524


Added:
    branches/clm-micromips/gcc/testsuite/gcc.target/mips/pr56524.c
Modified:
    branches/clm-micromips/gcc/ChangeLog
    branches/clm-micromips/gcc/c-family/ChangeLog
    branches/clm-micromips/gcc/c-family/c-common.c
    branches/clm-micromips/gcc/function.c
    branches/clm-micromips/gcc/function.h
    branches/clm-micromips/gcc/optabs.c
    branches/clm-micromips/gcc/tree.h