Merge lp:~michaelh1/gcc-linaro/lp723185 into lp:gcc-linaro/4.6
- lp723185
- Merge into 4.6
Status: | Merged |
---|---|
Approved by: | Richard Sandiford |
Approved revision: | no longer in the source branch. |
Merged at revision: | 106761 |
Proposed branch: | lp:~michaelh1/gcc-linaro/lp723185 |
Merge into: | lp:gcc-linaro/4.6 |
Diff against target: |
197 lines (+150/-18) 3 files modified
ChangeLog.linaro (+21/-0) gcc/config/arm/arm.c (+128/-17) gcc/config/arm/arm.md (+1/-1) |
To merge this branch: | bzr merge lp:~michaelh1/gcc-linaro/lp723185 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Richard Sandiford | Approve | ||
Review via email: mp+63965@code.launchpad.net |
This proposal supersedes a proposal from 2011-06-08.
Commit message
Description of the change
Backport r172297 from trunk to fix LP: #723185. Now pulls in the offset limit check.
Linaro Toolchain Builder (cbuild) wrote : Posted in a previous version of this proposal | # |
Linaro Toolchain Builder (cbuild) wrote : Posted in a previous version of this proposal | # |
cbuild successfully built this on i686-lucid-
The build results are available at:
http://
The test suite results were unchanged compared to the branch point lp:gcc-linaro/4.6+bzr106754.
The full testsuite results are at:
http://
cbuild-checked: i686-lucid-
Linaro Toolchain Builder (cbuild) wrote : Posted in a previous version of this proposal | # |
cbuild successfully built this on x86_64-
The build results are available at:
http://
The test suite results were unchanged compared to the branch point lp:gcc-linaro/4.6+bzr106754.
The full testsuite results are at:
http://
cbuild-checked: x86_64-
Richard Sandiford (rsandifo) wrote : Posted in a previous version of this proposal | # |
I think you also need:
2011-04-20 Chung-Lin Tang <email address hidden>
* config/arm/arm.c (arm_legitimize
quad-word modes, reduce to 9-bit index range when above 1016 limit.
(fixes http://
Ramana Radhakrishnan (ramana) wrote : Posted in a previous version of this proposal | # |
On 9 June 2011 09:07, Richard Sandiford <email address hidden> wrote:
> Review: Needs Fixing
> I think you also need:
FTR, I had tried to backport the above mentioned fix to the FSF 4.6
branch and saw a few ICEs in the testsuite that I've posted about at
http://
cheers
Ramana
>
> 2011-04-20 Chung-Lin Tang <email address hidden>
>
> * config/arm/arm.c (arm_legitimize
> quad-word modes, reduce to 9-bit index range when above 1016 limit.
>
> (fixes http://
>
>
> --
> https:/
> Your team Linaro Toolchain Developers is subscribed to branch lp:gcc-linaro/4.6.
>
Michael Hope (michaelh1) wrote : | # |
I'm running this to see if the ICEs that Ramana saw occur.
Linaro Toolchain Builder (cbuild) wrote : | # |
cbuild has taken a snapshot of this branch at r106756 and queued it for build.
The snapshot is available at:
http://
and will be built on the following builders:
a9-builder i686 x86_64
You can track the build queue at:
http://
cbuild-snapshot: gcc-linaro-
cbuild-ancestor: lp:gcc-linaro/4.6+bzr106754
cbuild-state: check
Linaro Toolchain Builder (cbuild) wrote : | # |
cbuild successfully built this on x86_64-
The build results are available at:
http://
The test suite results were unchanged compared to the branch point lp:gcc-linaro/4.6+bzr106754.
The full testsuite results are at:
http://
cbuild-checked: x86_64-
Linaro Toolchain Builder (cbuild) wrote : | # |
cbuild successfully built this on armv7l-
The build results are available at:
http://
The test suite results were unchanged compared to the branch point lp:gcc-linaro/4.6+bzr106754.
The full testsuite results are at:
http://
cbuild-checked: armv7l-
Linaro Toolchain Builder (cbuild) wrote : | # |
cbuild successfully built this on i686-lucid-
The build results are available at:
http://
The test suite results were unchanged compared to the branch point lp:gcc-linaro/4.6+bzr106754.
The full testsuite results are at:
http://
cbuild-checked: i686-lucid-
Michael Hope (michaelh1) wrote : | # |
The -mthumb -mfpu=neon build bootstrapped fine with no regressions. I'm running a -marm -mfpu=neon and -mthumb -fpu=vfpv3-d16 as well.
Chung-Lin Tang (cltang) wrote : | # |
On 2011/6/10 07:51 AM, Michael Hope wrote:
> The -mthumb -mfpu=neon build bootstrapped fine with no regressions. I'm running a -marm -mfpu=neon and -mthumb -fpu=vfpv3-d16 as well.
This patch actually only affects ARM mode. The original ICE should not
have occurred in Thumb-2, IIRC.
Michael Hope (michaelh1) wrote : | # |
Cortex-A9+NEON+ARM bootstraps fine. The testsuite comes out cleaner due to being in ARM mode.
Cortex-
Linaro Toolchain Builder (cbuild) wrote : | # |
cbuild successfully built this on armv7l-
The build results are available at:
http://
The test suite was not checked as the branch point lp:gcc-linaro/4.6+bzr106754 has nothing to compare against.
The full testsuite results are at:
http://
cbuild-checked: armv7l-
Linaro Toolchain Builder (cbuild) wrote : | # |
cbuild successfully built this on armv7l-
The build results are available at:
http://
The test suite was not checked as the branch point lp:gcc-linaro/4.6+bzr106754 has nothing to compare against.
The full testsuite results are at:
http://
cbuild-checked: armv7l-
Michael Hope (michaelh1) wrote : | # |
Any thoughts on this? GCC bootstraps in Thumb-2 NEON, Thumb-2 VFP, and ARM NEON modes.
Richard Sandiford (rsandifo) wrote : | # |
Looks good. Sorry for the slow response.
Preview Diff
1 | === modified file 'ChangeLog.linaro' |
2 | --- ChangeLog.linaro 2011-06-07 11:18:20 +0000 |
3 | +++ ChangeLog.linaro 2011-06-09 09:02:19 +0000 |
4 | @@ -1,3 +1,24 @@ |
5 | +2011-06-09 Michael Hope <michael.hope@linaro.org> |
6 | + |
7 | + gcc/ |
8 | + Backport from mainline: |
9 | + |
10 | + Chung-Lin Tang <cltang@codesourcery.com> |
11 | + Richard Earnshaw <rearnsha@arm.com> |
12 | + |
13 | + PR target/48250 |
14 | + * config/arm/arm.c (arm_legitimize_reload_address): Update cases |
15 | + to use sign-magnitude offsets. Reject unsupported unaligned |
16 | + cases. Add detailed description in comments. |
17 | + * config/arm/arm.md (reload_outdf): Disable for ARM mode; change |
18 | + condition from TARGET_32BIT to TARGET_ARM. |
19 | + |
20 | + Chung-Lin Tang <cltang@codesourcery.com> |
21 | + |
22 | + * config/arm/arm.c (arm_legitimize_reload_address): For NEON |
23 | + quad-word modes, reduce to 9-bit index range when above 1016 |
24 | + limit. |
25 | + |
26 | 2011-06-07 Andrew Stubbs <ams@codesourcery.com> |
27 | |
28 | Backport from FSF: |
29 | |
30 | === modified file 'gcc/config/arm/arm.c' |
31 | --- gcc/config/arm/arm.c 2011-05-11 14:49:48 +0000 |
32 | +++ gcc/config/arm/arm.c 2011-06-09 09:02:19 +0000 |
33 | @@ -6419,23 +6419,134 @@ |
34 | HOST_WIDE_INT val = INTVAL (XEXP (*p, 1)); |
35 | HOST_WIDE_INT low, high; |
36 | |
37 | - if (mode == DImode || (mode == DFmode && TARGET_SOFT_FLOAT)) |
38 | - low = ((val & 0xf) ^ 0x8) - 0x8; |
39 | - else if (TARGET_MAVERICK && TARGET_HARD_FLOAT) |
40 | - /* Need to be careful, -256 is not a valid offset. */ |
41 | - low = val >= 0 ? (val & 0xff) : -((-val) & 0xff); |
42 | - else if (mode == SImode |
43 | - || (mode == SFmode && TARGET_SOFT_FLOAT) |
44 | - || ((mode == HImode || mode == QImode) && ! arm_arch4)) |
45 | - /* Need to be careful, -4096 is not a valid offset. */ |
46 | - low = val >= 0 ? (val & 0xfff) : -((-val) & 0xfff); |
47 | - else if ((mode == HImode || mode == QImode) && arm_arch4) |
48 | - /* Need to be careful, -256 is not a valid offset. */ |
49 | - low = val >= 0 ? (val & 0xff) : -((-val) & 0xff); |
50 | - else if (GET_MODE_CLASS (mode) == MODE_FLOAT |
51 | - && TARGET_HARD_FLOAT && TARGET_FPA) |
52 | - /* Need to be careful, -1024 is not a valid offset. */ |
53 | - low = val >= 0 ? (val & 0x3ff) : -((-val) & 0x3ff); |
54 | + /* Detect coprocessor load/stores. */ |
55 | + bool coproc_p = ((TARGET_HARD_FLOAT |
56 | + && (TARGET_VFP || TARGET_FPA || TARGET_MAVERICK) |
57 | + && (mode == SFmode || mode == DFmode |
58 | + || (mode == DImode && TARGET_MAVERICK))) |
59 | + || (TARGET_REALLY_IWMMXT |
60 | + && VALID_IWMMXT_REG_MODE (mode)) |
61 | + || (TARGET_NEON |
62 | + && (VALID_NEON_DREG_MODE (mode) |
63 | + || VALID_NEON_QREG_MODE (mode)))); |
64 | + |
65 | + /* For some conditions, bail out when lower two bits are unaligned. */ |
66 | + if ((val & 0x3) != 0 |
67 | + /* Coprocessor load/store indexes are 8-bits + '00' appended. */ |
68 | + && (coproc_p |
69 | + /* For DI, and DF under soft-float: */ |
70 | + || ((mode == DImode || mode == DFmode) |
71 | + /* Without ldrd, we use stm/ldm, which does not |
72 | + fair well with unaligned bits. */ |
73 | + && (! TARGET_LDRD |
74 | + /* Thumb-2 ldrd/strd is [-1020,+1020] in steps of 4. */ |
75 | + || TARGET_THUMB2)))) |
76 | + return false; |
77 | + |
78 | + /* When breaking down a [reg+index] reload address into [(reg+high)+low], |
79 | + of which the (reg+high) gets turned into a reload add insn, |
80 | + we try to decompose the index into high/low values that can often |
81 | + also lead to better reload CSE. |
82 | + For example: |
83 | + ldr r0, [r2, #4100] // Offset too large |
84 | + ldr r1, [r2, #4104] // Offset too large |
85 | + |
86 | + is best reloaded as: |
87 | + add t1, r2, #4096 |
88 | + ldr r0, [t1, #4] |
89 | + add t2, r2, #4096 |
90 | + ldr r1, [t2, #8] |
91 | + |
92 | + which post-reload CSE can simplify in most cases to eliminate the |
93 | + second add instruction: |
94 | + add t1, r2, #4096 |
95 | + ldr r0, [t1, #4] |
96 | + ldr r1, [t1, #8] |
97 | + |
98 | + The idea here is that we want to split out the bits of the constant |
99 | + as a mask, rather than as subtracting the maximum offset that the |
100 | + respective type of load/store used can handle. |
101 | + |
102 | + When encountering negative offsets, we can still utilize it even if |
103 | + the overall offset is positive; sometimes this may lead to an immediate |
104 | + that can be constructed with fewer instructions. |
105 | + For example: |
106 | + ldr r0, [r2, #0x3FFFFC] |
107 | + |
108 | + This is best reloaded as: |
109 | + add t1, r2, #0x400000 |
110 | + ldr r0, [t1, #-4] |
111 | + |
112 | + The trick for spotting this for a load insn with N bits of offset |
113 | + (i.e. bits N-1:0) is to look at bit N; if it is set, then chose a |
114 | + negative offset that is going to make bit N and all the bits below |
115 | + it become zero in the remainder part. |
116 | + |
117 | + The SIGN_MAG_LOW_ADDR_BITS macro below implements this, with respect |
118 | + to sign-magnitude addressing (i.e. separate +- bit, or 1's complement), |
119 | + used in most cases of ARM load/store instructions. */ |
120 | + |
121 | +#define SIGN_MAG_LOW_ADDR_BITS(VAL, N) \ |
122 | + (((VAL) & ((1 << (N)) - 1)) \ |
123 | + ? (((VAL) & ((1 << ((N) + 1)) - 1)) ^ (1 << (N))) - (1 << (N)) \ |
124 | + : 0) |
125 | + |
126 | + if (coproc_p) |
127 | + { |
128 | + low = SIGN_MAG_LOW_ADDR_BITS (val, 10); |
129 | + |
130 | + /* NEON quad-word load/stores are made of two double-word accesses, |
131 | + so the valid index range is reduced by 8. Treat as 9-bit range if |
132 | + we go over it. */ |
133 | + if (TARGET_NEON && VALID_NEON_QREG_MODE (mode) && low >= 1016) |
134 | + low = SIGN_MAG_LOW_ADDR_BITS (val, 9); |
135 | + } |
136 | + else if (GET_MODE_SIZE (mode) == 8) |
137 | + { |
138 | + if (TARGET_LDRD) |
139 | + low = (TARGET_THUMB2 |
140 | + ? SIGN_MAG_LOW_ADDR_BITS (val, 10) |
141 | + : SIGN_MAG_LOW_ADDR_BITS (val, 8)); |
142 | + else |
143 | + /* For pre-ARMv5TE (without ldrd), we use ldm/stm(db/da/ib) |
144 | + to access doublewords. The supported load/store offsets are |
145 | + -8, -4, and 4, which we try to produce here. */ |
146 | + low = ((val & 0xf) ^ 0x8) - 0x8; |
147 | + } |
148 | + else if (GET_MODE_SIZE (mode) < 8) |
149 | + { |
150 | + /* NEON element load/stores do not have an offset. */ |
151 | + if (TARGET_NEON_FP16 && mode == HFmode) |
152 | + return false; |
153 | + |
154 | + if (TARGET_THUMB2) |
155 | + { |
156 | + /* Thumb-2 has an asymmetrical index range of (-256,4096). |
157 | + Try the wider 12-bit range first, and re-try if the result |
158 | + is out of range. */ |
159 | + low = SIGN_MAG_LOW_ADDR_BITS (val, 12); |
160 | + if (low < -255) |
161 | + low = SIGN_MAG_LOW_ADDR_BITS (val, 8); |
162 | + } |
163 | + else |
164 | + { |
165 | + if (mode == HImode || mode == HFmode) |
166 | + { |
167 | + if (arm_arch4) |
168 | + low = SIGN_MAG_LOW_ADDR_BITS (val, 8); |
169 | + else |
170 | + { |
171 | + /* The storehi/movhi_bytes fallbacks can use only |
172 | + [-4094,+4094] of the full ldrb/strb index range. */ |
173 | + low = SIGN_MAG_LOW_ADDR_BITS (val, 12); |
174 | + if (low == 4095 || low == -4095) |
175 | + return false; |
176 | + } |
177 | + } |
178 | + else |
179 | + low = SIGN_MAG_LOW_ADDR_BITS (val, 12); |
180 | + } |
181 | + } |
182 | else |
183 | return false; |
184 | |
185 | |
186 | === modified file 'gcc/config/arm/arm.md' |
187 | --- gcc/config/arm/arm.md 2011-06-02 15:58:33 +0000 |
188 | +++ gcc/config/arm/arm.md 2011-06-09 09:02:19 +0000 |
189 | @@ -6245,7 +6245,7 @@ |
190 | [(match_operand:DF 0 "arm_reload_memory_operand" "=o") |
191 | (match_operand:DF 1 "s_register_operand" "r") |
192 | (match_operand:SI 2 "s_register_operand" "=&r")] |
193 | - "TARGET_32BIT" |
194 | + "TARGET_THUMB2" |
195 | " |
196 | { |
197 | enum rtx_code code = GET_CODE (XEXP (operands[0], 0)); |
cbuild has taken a snapshot of this branch at r106755 and queued it for build.
The snapshot is available at: ex.seabright. co.nz/snapshots /gcc-linaro- 4.6+bzr106755~ michaelh1~ lp723185. tar.xdelta3. xz
http://
and will be built on the following builders:
a9-builder i686 x86_64
You can track the build queue at: ex.seabright. co.nz/helpers/ scheduler
http://
cbuild-snapshot: gcc-linaro- 4.6+bzr106755~ michaelh1~ lp723185
cbuild-ancestor: lp:gcc-linaro/4.6+bzr106754
cbuild-state: check