Bug 49423 - [4.8/4.9/5 Regression] [arm] internal compiler error: in push_minipool_fix
Summary: [4.8/4.9/5 Regression] [arm] internal compiler error: in push_minipool_fix
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.6.0
: P2 normal
Target Milestone: 4.8.4
Assignee: cbaylis
URL:
Keywords: ice-on-valid-code
: 49135 52836 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-15 13:49 UTC by philb
Modified: 2014-09-29 17:21 UTC (History)
10 users (show)

See Also:
Host:
Target: arm
Build:
Known to work:
Known to fail: 4.6.0, 4.6.1
Last reconfirmed: 2011-06-29 15:35:12


Attachments
testcase (37.13 KB, application/x-bzip)
2011-06-15 13:50 UTC, philb
Details
testcase for gcc-4.6 and gcc-4.7 (985 bytes, application/octet-stream)
2012-09-25 16:12 UTC, Dinar Temirbulatov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description philb 2011-06-15 13:49:53 UTC
$ arm-oe-linux-gnueabi-gcc -march=armv7-a -O2 -S -mfloat-abi=softfp -mfpu=vfp svga_tgsi_insn.i
svga_tgsi_insn.c: In function 'svga_shader_emit_instructions':
svga_tgsi_insn.c:2969:1: internal compiler error: in push_minipool_fix, at config/arm/arm.c:12138
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
$
Comment 1 philb 2011-06-15 13:50:23 UTC
Created attachment 24537 [details]
testcase
Comment 2 Mikael Pettersson 2011-06-20 11:10:11 UTC
I can reproduce the ICE on armv5tel-linux-gnueabi with gcc-4.6.0 and 4.6-20110617, but not with gcc-4.5.3 or the latest 4.7 snapshot.

The ICE stopped occurring on trunk with r170984 (PR41490 fix).  However that's a generic missed-optimization fix, so I suspect the bug is latent on trunk.
Comment 3 Mikael Pettersson 2011-06-20 17:06:18 UTC
It's caused by r164136:

Author: jamborm
Date: Thu Sep  9 23:38:23 2010
New Revision: 164136

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164136
Log:
2010-09-10  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/44972
	* tree-sra.c: Include toplev.h.
	(build_ref_for_offset): Entirely reimplemented.
...

That makes it probably related to PR49094, although in this PR's test case the one occurrence of attribute((aligned(N)) doesn't matter for the ICE.
Comment 4 Martin Jambor 2011-06-21 12:48:36 UTC
Possibly, yes.  I plan to submit a fix for PR49094 this week, please
try after that.
Comment 5 Ramana Radhakrishnan 2011-06-29 15:35:12 UTC
doesn't seem to occur on 4.5 branch or on trunk. 

Ramana
Comment 6 Martin Jambor 2011-07-11 17:39:09 UTC
I have just committed a fix for PR 49094 to the 4.6 branch.  Please
try again now.  Thanks.
Comment 7 Mikael Pettersson 2011-07-20 12:35:53 UTC
gcc-4.6-20110715 still ICEs on this test case, so unfortunately the PR49094 fix didn't solve this problem.
Comment 8 Martin Jambor 2012-04-17 16:37:26 UTC
I'm sorry I forgot about this bug but I finally remembered to have a
look today.  Unfortunately, on i686 host I was able to reproduce the
ICE also with revisions 164134 and 164135 and so I assumed that Mikael
made a mistake when he tracked the cause of the ICE to my commit in
comment #3.  When verifying that on an x86_64 host, I realized that
the ICE indeed started happening with my commit but it re-appeared
quickly with previous revisions when I added -fno-tree-sra to the
command line options.  Therefore I believe my patch did not cause any
bug but uncovered some other issue elsewhere.

In order to help at least a little, I did my own bisecting, this time
with -fno-tree-sra, and to me it appears the ICE (still present on the
4.6 branch) is introduced by revision 163935:

2010-09-07  Bernd Schmidt  <bernds@codesourcery.com>

        PR target/43137
        * config/arm/iterators.md (qhs_zextenddi_cond, qhs_sextenddi_cond):
        New define_mode_attrs.
        * config/arm/arm.md (zero_extendsidi2, arm_zero_extendsidi2,
        arm_exxtendsidi2, arm_extendsidi2): Delete patterns.
        (zero_extend<mode>di2, extend<mode>di2 and related splits): New.
        (thumb1_zero_extendhisi2): Remove code to handle LABEL_REFs.
        Remove pool_range attribute.
        (arm_zero_extendhisi2, arm_zero_extendhisi2_v6, arm_zero_extendqisi2,
        arm_zero_extendqisi2_v6, thumb1_zero_extendqisi2_v6): Remove
        pool_range and neg_pool_range attributes.
        * config/arm/thumb2.md (thumb2_zero_extendsidi2,
        thumb2_zero_extendhidi2, thumb2_zero_extendqidi2, thumb2_extendsidi2,
        thumb2_extendhidi2, thumb2_extendqidi2): Delete.
Comment 9 Andrew Pinski 2012-09-16 18:10:55 UTC
I get this same ICE while compiling gcc.c-torture/execute/scal-to-vec2.c at -O1 with a 4.7 configured with  --with-arch=armv7-a --with-fpu=vfp --with-float=hard .


(gdb) p debug_rtx(insn)
(insn 357 1038 1041 (set (reg:SI 14 lr)
        (zero_extend:SI (mem/u/c:HI (symbol_ref/u:SI ("*.LC0") [flags 0x2]) [0 S2 A16]))) t.c:49 161 {*arm_zero_extendhisi2_v6}
     (nil))
$4 = void

Both arm_zero_extendhisi2 and arm_zero_extendhisi2_v6 are missing pool_range/neg_pool_range .
Comment 10 Andrew Pinski 2012-09-21 15:23:18 UTC
*** Bug 49135 has been marked as a duplicate of this bug. ***
Comment 11 Andrew Pinski 2012-09-21 15:23:36 UTC
*** Bug 52836 has been marked as a duplicate of this bug. ***
Comment 12 Dinar Temirbulatov 2012-09-25 16:12:13 UTC
Created attachment 28273 [details]
testcase for gcc-4.6 and gcc-4.7

Here is another testcase for gcc-4.6 and gcc-4.7
Comment 13 Dinar Temirbulatov 2012-09-25 16:14:44 UTC
This time it is "arm_zero_extendqisi2_v6" pattern
Comment 14 Dinar Temirbulatov 2012-09-25 16:18:05 UTC
(gdb) call debug_rtx(insn)
(insn:TI 454 460 607 (set (reg:SI 2 r2 [orig:433 buf+2 ] [433])
        (zero_extend:SI (mem/u/c/i:QI (symbol_ref/u:SI ("*.LC0") [flags 0x2])
[0 S1 A8]))) cr_parse-reduced.ii:111 168 {*arm_zero_extendqisi2_v6}
     (nil))
Comment 15 Dinar Temirbulatov 2012-09-26 15:38:33 UTC
here is command and flags to reproduce the second testcase on 4.6 and 4.7 : cc1plus -mcpu=cortex-a15 -mfpu=neon -mfloat-abi=hard -ftree-vectorize -O3 cr_parse-reduced-fsf.ii -o /tmp/1.s
Comment 16 Dinar Temirbulatov 2012-10-11 09:11:27 UTC
this regression after PR43137, also absence of pool range predicates for arm_zero_extendqisi2, arm_zero_extendqisi2_v6, arm_zero_extendhisi2, arm_zero_extendhisi2_v6 caused gcc.c-torture/compile/920928-2.c to fail with ICE by using -Os flag.
Comment 17 Jakub Jelinek 2013-03-26 10:03:51 UTC
Another testcase for -march=armv7-a -O2, ICEs with both trunk and 4.8 branch (reduced from https://bugzilla.redhat.com/show_bug.cgi?id=927565 ):
const int a, b[4][256], c[4][256];
int
foo (unsigned *x)
{
  unsigned d[9];
  x[55] = c[0][d[7] & 0xff] ^ c[3][d[7] & 0xff];
  d[0] = (b[0][d[7] & 0xff] ^ b[1][(d[7] >> 16) & 0xff] ^ b[2][d[7] & 0xff] ^ b[3][d[7] & 0xff]) ^ a;
  x[48] = c[0][d[0] & 0xff] ^ c[1][(d[0] >> 8) & 0xff] ^ c[2][(d[0] >> 16) & 0xff] ^ c[3][d[0] >> 24];
  x[49] = c[0][d[1] & 0xff] ^ c[1][(d[1] >> 8) & 0xff] ^ c[2][d[1] & 0xff] ^ c[3][d[1]];
  d[4] = b[0][0xff] ^ b[1][8] ^ b[3][d[3] >> 24];
  x[44] = c[0][d[4] & 0xff] ^ c[1][(d[4] >> 8) & 0xff] ^ c[2][(d[4] >> 16) & 0xff] ^ c[3][d[4] >> 24];
  d[5] ^= d[4];
  x[45] = c[0][d[5] & 0xff] ^ c[1][d[5]] ^ c[2][(d[5] >> 16) & 0xff] ^ c[3][(d[5] >> 24) & 0xff];
  d[6] ^= d[5];
  x[46] = c[0][d[6] & 0xff] ^ c[3][d[6] & 0xff];
  d[7] ^= d[6];
  x[47] = c[0][d[7]] ^ c[3][(d[7] >> 24) & 0xff];
}

p debug_rtx (fix->insn)
(insn:TI 6 155 156 (set (reg:SI 5 r5 [orig:110 D.4236 ] [110])
        (zero_extend:SI (mem/u/c:QI (symbol_ref/u:SI ("*.LC0") [flags 0x2]) [0 S1 A8]))) rh927565.i:6 171 {*arm_zero_extendqisi2_v6}
     (nil))

If the PR43137 changes removed the pool_range/neg_pool_range attributes from the instructions incorrectly, can those be added?  I mean, for ICE on valid code with so many dups it is ridiculous to keep it unsolved for almost 3 years now, out of which the bug is known for almost 2 years.
Comment 18 Eric Botcazou 2013-03-27 10:26:37 UTC
I just posted a patch about it:
  http://gcc.gnu.org/ml/gcc-patches/2013-03/msg00939.html
Comment 19 Jakub Jelinek 2013-04-12 15:16:21 UTC
GCC 4.6.4 has been released and the branch has been closed.
Comment 20 jules 2013-06-20 10:37:47 UTC
I've posted a new potential fix (and a new testcase which breaks on current mainline) here:

http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01191.html
Comment 21 Yury Gribov 2014-01-20 08:00:35 UTC
Can we accept Julian's patch for 4.9? This bug really hurts.
Comment 22 Eric Botcazou 2014-01-20 08:23:46 UTC
IMO this bug should be moved to P1 as a regression on a primary platform and the RMs should up the pressure on the maintainers to have it fixed.
Comment 23 Jakub Jelinek 2014-01-20 08:42:18 UTC
IMHO a bug that is known for 2.5 years and unfixed shouldn't be all of sudden P1.  That doesn't mean the maintainers should ignore the bug, just that it isn't a release blocker.
Comment 24 Eric Botcazou 2014-01-20 09:23:27 UTC
> IMHO a bug that is known for 2.5 years and unfixed shouldn't be all of
> sudden P1.  That doesn't mean the maintainers should ignore the bug, just
> that it isn't a release blocker.

I'm afraid that history has shown that you won't achieve the former without doing the latter...
Comment 25 Charles Baylis 2014-04-03 15:04:56 UTC
I have proposed a fix for this:

http://gcc.gnu.org/ml/gcc-patches/2014-04/msg00078.html
Comment 26 jules 2014-05-01 12:25:36 UTC
The testcase I previously had for this bug no longer reproduces since LRA was enabled by default on ARM. So, it's possible the bug is now dormant, or indeed fixed. The difference in the postreload dump around the insn which previously failed (136) is, using -mno-lra/-mlra on current trunk:

-(insn 136 7 12 2 (set (reg:SI 11 fp [orig:231 D.6590 ] [231])
-        (zero_extend:SI (mem/u/c:QI (symbol_ref/u:SI ("*.LC0") [flags 0x2]) [0  S1 A8]))) ../../aes/aeskey.c:509 182 {*arm_zero_extendqisi2_v6}
+(insn 952 7 136 2 (set (reg:SI 12 ip [1488])
+        (const_int 0 [0])) ../../aes/aeskey.c:509 666 {*arm_movsi_vfp}
+     (nil))
+(insn 136 952 12 2 (set (reg:SI 11 fp [orig:231 D.6590 ] [231])
+        (zero_extend:SI (reg:QI 12 ip [1488]))) ../../aes/aeskey.c:509 182 {*arm_zero_extendqisi2_v6}

Or, does anyone know of a testcase which still causes this ICE with -mlra enabled? If not, we might be able to consider this fixed. (Insn 136 -- of the form which causes breakage -- was generated by reload.)

Thanks,

Julian
Comment 27 Charles Baylis 2014-05-02 13:59:30 UTC
I suspect this still remains as a latent bug.

The zero/sign extend patterns still allow a memory operand, and there remains a subset of memory operands which will trigger the ICE (ie those which refer to a constant pool). 

I think we shouldn't consider it fixed unless there is a definitive reason to believe that LRA can never rematerialize a constant in this way.
Comment 28 Richard Biener 2014-06-12 13:43:35 UTC
The 4.7 branch is being closed, moving target milestone to 4.8.4.
Comment 29 cbaylis 2014-07-05 11:58:40 UTC
Author: cbaylis
Date: Sat Jul  5 11:58:06 2014
New Revision: 212303

URL: https://gcc.gnu.org/viewcvs?rev=212303&root=gcc&view=rev
Log:
[ARM] PR target/49423

2014-07-05  Charles Baylis  <charles.baylis@linaro.org>

	PR target/49423
	* config/arm/arm-protos.h (arm_legitimate_address_p,
	arm_is_constant_pool_ref): Add prototypes.
	* config/arm/arm.c (arm_legitimate_address_p): Remove static.
	(arm_is_constant_pool_ref) New function.
	* config/arm/arm.md (unaligned_loadhis, arm_zero_extendhisi2_v6,
	arm_zero_extendqisi2_v6): Use Uh constraint for memory operand.
	(arm_extendhisi2, arm_extendhisi2_v6): Use Uh constraint for memory
	operand. Remove pool_range and neg_pool_range attributes.
	(arm_extendqihi_insn, arm_extendqisi, arm_extendqisi_v6): Remove
	pool_range and neg_pool_range attributes.
	* config/arm/constraints.md (Uh): New constraint.
	(Uq): Don't allow constant pool references.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm-protos.h
    trunk/gcc/config/arm/arm.c
    trunk/gcc/config/arm/arm.md
    trunk/gcc/config/arm/constraints.md
Comment 30 ktkachov 2014-07-09 09:30:53 UTC
This testcase now works on trunk 4.10.

Charles, can this be closed? or is there some backporting to be done?
Comment 31 cbaylis 2014-07-09 09:53:12 UTC
I intend to backport to 4.8 and 4.9, once this change has had a week of testing on trunk.
Comment 32 gregory.0xf0 2014-09-26 21:54:18 UTC
(In reply to cbaylis from comment #31)
> I intend to backport to 4.8 and 4.9, once this change has had a week of
> testing on trunk.

Hi Charles, just a gentle reminder that you were planning this.
Comment 33 cbaylis 2014-09-29 16:48:03 UTC
Author: cbaylis
Date: Mon Sep 29 16:47:31 2014
New Revision: 215685

URL: https://gcc.gnu.org/viewcvs?rev=215685&root=gcc&view=rev
Log:
2014-09-29  Charles Baylis  <charles.baylis@linaro.org>

        Backport from mainline r212303
        PR target/49423
        * config/arm/arm-protos.h (arm_legitimate_address_p,
        arm_is_constant_pool_ref): Add prototypes.
        * config/arm/arm.c (arm_legitimate_address_p): Remove static.
        (arm_is_constant_pool_ref) New function.
        * config/arm/arm.md (unaligned_loadhis, arm_zero_extendhisi2_v6,
        arm_zero_extendqisi2_v6): Use Uh constraint for memory operand.
        (arm_extendhisi2, arm_extendhisi2_v6): Use Uh constraint for memory
        operand and remove pool_range and neg_pool_range attributes.
        (arm_extendqihi_insn, arm_extendqisi, arm_extendqisi_v6): Remove
        pool_range and neg_pool_range attributes.
        * config/arm/constraints.md (Uh): New constraint. (Uq): Don't allow
        constant pool references.


Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/arm/arm-protos.h
    branches/gcc-4_9-branch/gcc/config/arm/arm.c
    branches/gcc-4_9-branch/gcc/config/arm/arm.md
    branches/gcc-4_9-branch/gcc/config/arm/constraints.md
Comment 34 cbaylis 2014-09-29 17:08:18 UTC
Author: cbaylis
Date: Mon Sep 29 17:07:47 2014
New Revision: 215686

URL: https://gcc.gnu.org/viewcvs?rev=215686&root=gcc&view=rev
Log:
2014-09-29  Charles Baylis  <charles.baylis@linaro.org>

        Backport from mainline r212303
        PR target/49423
        * config/arm/arm-protos.h (arm_legitimate_address_p,
        arm_is_constant_pool_ref): Add prototypes.
        * config/arm/arm.c (arm_legitimate_address_p): Remove static.
        (arm_is_constant_pool_ref) New function.
        * config/arm/arm.md (unaligned_loadhis, arm_zero_extendhisi2_v6,
        arm_zero_extendqisi2_v6): Use Uh constraint for memory operand.
        (arm_extendhisi2, arm_extendhisi2_v6): Use Uh constraint for memory
        operand and remove pool_range and neg_pool_range attributes.
        (arm_extendqihi_insn, arm_extendqisi, arm_extendqisi_v6): Remove
        pool_range and neg_pool_range attributes.
        * config/arm/constraints.md (Uh): New constraint. (Uq): Don't allow
        constant pool references.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/arm/arm-protos.h
    branches/gcc-4_8-branch/gcc/config/arm/arm.c
    branches/gcc-4_8-branch/gcc/config/arm/arm.md
    branches/gcc-4_8-branch/gcc/config/arm/constraints.md
Comment 35 cbaylis 2014-09-29 17:21:28 UTC
Backports committed to 4.8 and 4.9