Bug 71607 - [6 Regression] [ARM] ice due to forbidden enabled attribute dependency on instruction operands
Summary: [6 Regression] [ARM] ice due to forbidden enabled attribute dependency on ins...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.0
: P2 normal
Target Milestone: 7.2
Assignee: Thomas Preud'homme
URL:
Keywords: ice-on-valid-code
: 79237 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-06-21 10:40 UTC by Mickael Guene
Modified: 2018-10-26 10:57 UTC (History)
3 users (show)

See Also:
Host:
Target: ARM
Build:
Known to work: 4.9.4
Known to fail: 5.4.1, 6.1.1, 7.0
Last reconfirmed: 2016-06-21 00:00:00

Attachments
reduce test case that ice (164 bytes, text/x-csrc)
2016-06-21 10:40 UTC, Mickael Guene 		Details
ice backtrace (386 bytes, text/plain)
2016-06-21 10:41 UTC, Mickael Guene 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mickael Guene 2016-06-21 10:40:42 UTC 
Created attachment 38738 [details]
reduce test case that ice

There is a rule in the 'gcc internals' document that states that 'enabled'
attribute must not depend on the current insn operands.

 But unfortunately such a dependency exists due to 'use_literal_pool' attribute
dependency on operands[1] and 'use_literal_pool' usage in 'enabled'
attribute definition (see arm.md).

 This leads to ice on check_bool_attrs() assertion in recog.c that exactly 
checks that 'enabled', 'preferred_for_size' or 'preferred_for_speed'
are static properties of the subtarget (as stated in gcc internals document
and in check_bool_attrs() comment).

 Find attached a test case that triggers this ice.
>> arm-none-eabi-gcc -c simple.c -mslow-flash-data -O0 -mfloat-abi=hard -march=armv7-m -mthumb -mfpu=vfpv3
will trigger an ice whereas
>> arm-none-eabi-gcc -c simple.c -mno-slow-flash-data -O0 -mfloat-abi=hard -march=armv7-m -mthumb -mfpu=vfpv3
will not.

Mickael
Comment 1 Mickael Guene 2016-06-21 10:41:14 UTC 
Created attachment 38739 [details]
ice backtrace
Comment 2 ktkachov 2016-06-21 10:45:22 UTC 
Confirned, doesn't ICE on 4.9
Comment 3 Thomas Preud'homme 2016-06-22 16:28:47 UTC 
I have a patch for this but it has not had much testing so far. It also includes code to fix -mslow-flash-data with floating-point constants. I'll see to split the patch in two as testing the integer part should be simpler.

Best regards.
Comment 4 Christophe Monat 2016-09-01 14:11:53 UTC 
(In reply to Thomas Preud'homme from comment #3)

Thomas,

I am seeing that the assignee name has been reset : does it mean that you definitely disengage from looking at this problem ?

If this is the case, and if nobody officially takes over, would you share your current patch ?

Best regards,
--C
Comment 5 Thomas Preud'homme 2016-09-01 14:38:04 UTC 
(In reply to Christophe Monat from comment #4)
> (In reply to Thomas Preud'homme from comment #3)
> 
> Thomas,

Hi Christophe,

> 
> I am seeing that the assignee name has been reset : does it mean that you
> definitely disengage from looking at this problem ?

Yes but only because a colleague took over. He is trying a new approach for -mslow-flash-data altogether which should prove cleaner.

> 
> If this is the case, and if nobody officially takes over, would you share
> your current patch ?

I'd prefer not since my colleague Andre took over and is actively working on it. He'll keep this bug updated of any progress.

Best regards,

Thomas
Comment 6 Thomas Preud'homme 2016-09-01 14:38:27 UTC 
Ticket is currently unassigned
Comment 7 avieira 2016-10-06 14:00:07 UTC 
Got a patch up for review on gcc-patches that fixes this, see https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00377.html
Comment 8 avieira 2016-12-05 17:36:35 UTC 
Author: avieira
Date: Mon Dec  5 17:36:03 2016
New Revision: 243266

URL: https://gcc.gnu.org/viewcvs?rev=243266&root=gcc&view=rev
Log:
[ARM] PR71607: New approach to arm_disable_literal_pool

gcc/ChangeLog.arm:
2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	PR target/71607
	* config/arm/arm.md (use_literal_pool): Removes.
	(64-bit immediate split): No longer takes cost into consideration
	if 'arm_disable_literal_pool' is enabled.
	* config/arm/arm.c (arm_use_blocks_for_constant_p): New.
	(TARGET_USE_BLOCKS_FOR_CONSTANT_P): Define.
	(arm_max_const_double_inline_cost): Remove use of
	arm_disable_literal_pool.
	* config/arm/vfp.md (no_literal_pool_df_immediate): New.
	(no_literal_pool_sf_immediate): New.

gcc/testsuite/ChangeLog.arm:
2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
	    Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR target/71607
	* gcc.target/arm/thumb2-slow-flash-data.c: Renamed to ...
	* gcc.target/arm/thumb2-slow-flash-data-1.c: ... this.
	* gcc.target/arm/thumb2-slow-flash-data-2.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-3.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-4.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-5.c: New.


Added:
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
Removed:
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data.c
Modified:
    branches/ARM/embedded-6-branch/gcc/ChangeLog.arm
    branches/ARM/embedded-6-branch/gcc/config/arm/arm.c
    branches/ARM/embedded-6-branch/gcc/config/arm/arm.md
    branches/ARM/embedded-6-branch/gcc/config/arm/vfp.md
    branches/ARM/embedded-6-branch/gcc/testsuite/ChangeLog.arm
Comment 9 Ramana Radhakrishnan 2017-01-26 13:06:44 UTC 
*** Bug 79237 has been marked as a duplicate of this bug. ***
Comment 10 Christophe Monat 2017-04-07 13:52:45 UTC 
(In reply to Ramana Radhakrishnan from comment #9)

Hello Ramana,

Is there a plan to have this patch delivered upstream at some point in the near future ?

Best regards,
--C
Comment 11 Tejas Belagod 2017-04-07 14:28:18 UTC 
(In reply to Christophe Monat from comment #10)
> (In reply to Ramana Radhakrishnan from comment #9)
> 
> Hello Ramana,
> 
> Is there a plan to have this patch delivered upstream at some point in the
> near future ?
> 
> Best regards,
> --C

Hi Christophe,

We're working on a fix for this. We'll keep you updated on the progress.

Thanks,
Tejas.
Comment 12 Prakhar Bahuguna 2017-04-20 10:02:40 UTC 
(In reply to Christophe Monat from comment #10)
> (In reply to Ramana Radhakrishnan from comment #9)
> 
> Hello Ramana,
> 
> Is there a plan to have this patch delivered upstream at some point in the
> near future ?
> 
> Best regards,
> --C

Hi Christophe,

The patch has now been posted to the mailing list: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00872.html

Regards,

Prakhar
Comment 13 Christophe Monat 2017-05-03 15:33:49 UTC 
(In reply to Prakhar Bahuguna from comment #12)

Hi Prakar,

> The patch has now been posted to the mailing list:
> https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00872.html

Thanks for the work, and the kind notification.

I really have high hopes that it will be accepted soon (Ramana, could you please...?), since I am getting high internal pressure to have it fixed, to move forward to deliver our own work.
--C
Comment 14 Thomas Preud'homme 2017-05-05 15:42:00 UTC 
Author: thopre01
Date: Fri May  5 15:41:28 2017
New Revision: 247640

URL: https://gcc.gnu.org/viewcvs?rev=247640&root=gcc&view=rev
Log:
[ARM] PR71607: Fix ICE when loading constant

2017-05-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
            Prakhar Bahuguna  <prakhar.bahuguna@arm.com>

    gcc/
    PR target/71607
    * config/arm/arm.md (use_literal_pool): Remove.
    (64-bit immediate split): No longer takes cost into consideration
    if arm_disable_literal_pool is enabled.
    * config/arm/arm.c (arm_tls_referenced_p): Add diagnostic if TLS is
    used when arm_disable_literal_pool is enabled.
    (arm_max_const_double_inline_cost): Remove use of
    arm_disable_literal_pool.
    (push_minipool_fix): Add assert.
    (arm_reorg): Add return if arm_disable_literal_pool is enabled.
    * config/arm/vfp.md (no_literal_pool_df_immediate): New.
    (no_literal_pool_sf_immediate): New.

2017-05-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
        Thomas Preud'homme  <thomas.preudhomme@arm.com>
        Prakhar Bahuguna  <prakhar.bahuguna@arm.com>

    gcc/testsuite/
    PR target/71607
    * gcc.target/arm/thumb2-slow-flash-data.c: Renamed to ...
    * gcc.target/arm/thumb2-slow-flash-data-1.c: ... this.
    * gcc.target/arm/thumb2-slow-flash-data-2.c: New.
    * gcc.target/arm/thumb2-slow-flash-data-3.c: New.
    * gcc.target/arm/thumb2-slow-flash-data-4.c: New.
    * gcc.target/arm/thumb2-slow-flash-data-5.c: New.
    * gcc.target/arm/tls-disable-literal-pool.c: New.

Added:
    trunk/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
      - copied, changed from r247638, trunk/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data.c
    trunk/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
    trunk/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
    trunk/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
    trunk/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
    trunk/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
Removed:
    trunk/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c
    trunk/gcc/config/arm/arm.md
    trunk/gcc/config/arm/vfp.md
    trunk/gcc/testsuite/ChangeLog
Comment 15 prakhar 2017-06-02 11:19:48 UTC 
Author: prakhar
Date: Fri Jun  2 11:19:16 2017
New Revision: 248822

URL: https://gcc.gnu.org/viewcvs?rev=248822&root=gcc&view=rev
Log:
PR71607: Fix ICE when loading constant

2017-06-02  Prakhar Bahuguna  <prakhar.bahuguna@arm.com>

	Backport from mainline
	2017-05-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
		    Prakhar Bahuguna  <prakhar.bahuguna@arm.com>

	gcc/
	PR target/71607
	* config/arm/arm.md (use_literal_pool): Remove.
	(64-bit immediate split): No longer takes cost into consideration
	if arm_disable_literal_pool is enabled.
	* config/arm/arm.c (arm_tls_referenced_p): Add diagnostic if TLS is
	used when arm_disable_literal_pool is enabled.
	(arm_max_const_double_inline_cost): Remove use of
	arm_disable_literal_pool.
	(push_minipool_fix): Add assert.
	(arm_reorg): Add return if arm_disable_literal_pool is enabled.
	* config/arm/vfp.md (no_literal_pool_df_immediate): New.
	(no_literal_pool_sf_immediate): New.

	2017-05-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
		    Thomas Preud'homme  <thomas.preudhomme@arm.com>
		    Prakhar Bahuguna  <prakhar.bahuguna@arm.com>

	gcc/testsuite/
	PR target/71607
	* gcc.target/arm/thumb2-slow-flash-data.c: Renamed to ...
	* gcc.target/arm/thumb2-slow-flash-data-1.c: ... this.
	* gcc.target/arm/thumb2-slow-flash-data-2.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-3.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-4.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-5.c: New.
	* gcc.target/arm/tls-disable-literal-pool.c: New.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/arm/arm.c
    branches/gcc-7-branch/gcc/config/arm/arm.md
    branches/gcc-7-branch/gcc/config/arm/vfp.md
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
Comment 16 Thomas Preud'homme 2017-06-19 15:01:49 UTC 
Author: thopre01
Date: Mon Jun 19 15:01:11 2017
New Revision: 249372

URL: https://gcc.gnu.org/viewcvs?rev=249372&root=gcc&view=rev
Log:
PR71607: Fix ICE when loading constant

2017-06-19  Prakhar Bahuguna  <prakhar.bahuguna@arm.com>

	Backport from mainline
	2017-05-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
		    Prakhar Bahuguna  <prakhar.bahuguna@arm.com>

	gcc/
	PR target/71607
	* config/arm/arm.md (use_literal_pool): Remove.
	(64-bit immediate split): No longer takes cost into consideration
	if arm_disable_literal_pool is enabled.
	* config/arm/arm.c (arm_tls_referenced_p): Add diagnostic if TLS is
	used when arm_disable_literal_pool is enabled.
	(arm_max_const_double_inline_cost): Remove use of
	arm_disable_literal_pool.
	(push_minipool_fix): Add assert.
	(arm_reorg): Add return if arm_disable_literal_pool is enabled.
	* config/arm/vfp.md (no_literal_pool_df_immediate): New.
	(no_literal_pool_sf_immediate): New.

	gcc/testsuite/
	PR target/71607
	* gcc.target/arm/thumb2-slow-flash-data.c: Renamed to ...
	* gcc.target/arm/thumb2-slow-flash-data-1.c: ... this.
	* gcc.target/arm/thumb2-slow-flash-data-2.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-3.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-4.c: New.
	* gcc.target/arm/thumb2-slow-flash-data-5.c: New.
	* gcc.target/arm/tls-disable-literal-pool.c: New.


Added:
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
Modified:
    branches/ARM/embedded-6-branch/gcc/ChangeLog.arm
    branches/ARM/embedded-6-branch/gcc/config/arm/arm.c
    branches/ARM/embedded-6-branch/gcc/config/arm/vfp.md
    branches/ARM/embedded-6-branch/gcc/testsuite/ChangeLog.arm
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
    branches/ARM/embedded-6-branch/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
Comment 17 Jakub Jelinek 2017-10-10 13:27:30 UTC 
GCC 5 branch is being closed
Comment 18 Jakub Jelinek 2017-10-11 10:30:59 UTC 
I believe this should now be fixed for 7.2+.
Comment 19 Jakub Jelinek 2018-10-26 10:57:47 UTC 
GCC 6 branch is being closed, fixed in 7.x.