Bug 54974 - [4.7 Regression] [ARM] [thumb] Incorrect placement of constant pools
Summary: [4.7 Regression] [ARM] [thumb] Incorrect placement of constant pools
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.8.0
: P2 normal
Target Milestone: 4.8.0
Assignee: mgretton
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2012-10-18 12:58 UTC by Mans Rullgard
Modified: 2014-06-12 13:18 UTC (History)
4 users (show)

See Also:
Host:
Target: arm-linux-gnueabi
Build:
Known to work: 4.6.3, 4.8.0
Known to fail: 4.7.4
Last reconfirmed: 2012-11-28 00:00:00

Attachments
Test case (51.72 KB, application/octet-stream)
2012-10-18 12:58 UTC, Mans Rullgard 		Details
Hack patch (553 bytes, patch)
2012-10-18 13:00 UTC, Mans Rullgard 		Details | Diff
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 Mans Rullgard 2012-10-18 12:58:24 UTC 
Created attachment 28483 [details]
Test case

If the following conditions are true, a constant pool is placed too far from an LDR instruction accessing it:

- Compiling to Thumb2.
- There is no unconditional branch within 4k of the LDR instruction.
- At least one of:
  * The LDR instruction is not at a 4-byte aligned address.
  * There is an instruction boundary 4094 bytes from the value of PC
    at the LDR.

The problem here is twofold:

1. The base address of a PC-relative LDR in Thumb2 is the address of the
   instruction plus 4, rounded down to a multiple of 4.  The calculation
   for the valid range fails to take this rounding into account.

2. The constant pool is (rightly) 4-byte aligned.  When scanning the
   instructions for a suitable location, the possible need for padding
   is not considered.

The problem can be seen by compiling the attached preprocessed source using flags "-mthumb -march=armv7-a -mfpu=vfpv3-d16 -mfloat-abi=hard -O0 -fPIC".
Comment 1 Mans Rullgard 2012-10-18 13:00:48 UTC 
Created attachment 28484 [details]
Hack patch

This hack patch validates the analysis.  A proper fix probably looks different.
Comment 2 Matthias Klose 2012-10-18 13:20:19 UTC 
works with 4.7, fails with trunk
Comment 3 Mikael Pettersson 2012-10-18 13:23:13 UTC 
Also works with gcc-4.6.
Comment 4 Mikael Pettersson 2012-10-18 19:43:02 UTC 
The test case started failing with r189790:
http://gcc.gnu.org/ml/gcc-cvs/2012-07/msg00695.html

That patch merely enabled insn splitting at -O0, so I suspect it exposed a latent problem in the back-end, consistent with Måns' analysis.
Comment 5 Matthias Klose 2012-10-19 12:23:57 UTC 
this patch is now on the 4.7 branch too.
Comment 6 Ramana Radhakrishnan 2012-11-28 03:03:50 UTC 
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01825.html

Patch posted here.
Comment 7 mgretton 2012-11-29 10:02:22 UTC 
Author: mgretton
Date: Thu Nov 29 10:02:16 2012
New Revision: 193930

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193930
Log:
	PR target/54974
	* config/arm/arm.md (thumb2_pool_range, pool_range): Add comment on
	Thumb pool ranges.
	(thumb1_extendhisi2): Reduce Thumb pool range.
	(arm_movdi): Likewise.
	(thumb1_movdi_insn): Likewise.
	(thumb1_movsi_insn): Likewise.
	(pic_load_addr_unified): Likewise.
	(pic_load_addr_32bit): Likewise.
	(pic_load_addr_thumb1): Likewise.
	(thumb1_movhf): Likewise.
	(arm_movsf_soft_insn): Likewise.
	(thumb1_movsf_soft_insn): Likewise.
	(movdf_soft_insn): Likewise.
	(thumb1_movdf_soft_insn): Likewise.
	* config/arm/neon.md (*neon_mov<mode>): Likewise.
	(*neon_mov<mode>): Likwise.
	* config/arm/thumb2.md: (*thumb2_movsi_insn): Likewise.
	(*thumb2_movhi_insn): Likewise.
	(*thumb2_extendqisi_v6): Likewise.
	(*thumb2_zero_extendqisi_v6): Likewise.
	(*thumb2_zero_extendqisi2_v6): Likewise.
	* config/arm/vfp.md: (*thumb2_movsi_vfp): Likewise.
	(*movdi_vfp): Likewise.
	(*movdi_vfp_cortexa8): Likewise.
	(*thumb2_movsf_vfp): Likewise.
	(*thumb2_movdf_vfp): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.md
    trunk/gcc/config/arm/neon.md
    trunk/gcc/config/arm/thumb2.md
    trunk/gcc/config/arm/vfp.md
Comment 8 Richard Biener 2012-12-03 15:58:20 UTC 
Fixed on the trunk I suppose.
Comment 9 Ramana Radhakrishnan 2012-12-11 17:36:34 UTC 
Matt, 

Are you planning on backporting this to 4.7 ? 

regards
Ramana
Comment 10 Richard Biener 2013-04-11 07:59:05 UTC 
GCC 4.7.3 is being released, adjusting target milestone.
Comment 11 Richard Biener 2014-06-12 13:18:58 UTC 
Fixed for 4.8.0.