On 04/05/17 11:40, Prakhar Bahuguna wrote:
> On 03/05/2017 11:30:13, Richard Earnshaw (lists) wrote:
>> On 20/04/17 10:54, Prakhar Bahuguna wrote:
>>> [ARM] PR71607: Fix ICE when loading constant
>>>
>>> gcc/ChangeLog:
>>>
>>> 2017-04-18 Andre Vieira <[email protected]>
>>> Prakhar Bahuguna <[email protected]>
>>>
>>> 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_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.
>>> (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.
>>>
>>> testsuite/ChangeLog:
>>>
>>> 2017-04-18 Andre Vieira <[email protected]>
>>> Thomas Preud'homme <[email protected]>
>>> Prakhar Bahuguna <[email protected]>
>>>
>>> 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.
>>>
>>> Okay for stage1?
>>>
>>
>> This patch lacks a description of what's going on and why the change is
>> necessary (it should stand alone from the PR data). It's clearly a
>> non-trivial change, so why have you adopted this approach?
>>
>> R.
>>
>
> Hi,
>
> This patch is based off an earlier patch that was applied to the
> embedded-6-branch, and I had neglected to include the full description, which
> is presented below:
>
> This patch tackles the issue reported in PR71607. This patch takes a different
> approach for disabling the creation of literal pools. Instead of disabling the
> patterns that would normally transform the rtl into actual literal pools, it
> disables the creation of this literal pool rtl by making the target hook
> TARGET_CANNOT_FORCE_CONST_MEM return true if arm_disable_literal_pool is true.
> I added patterns to split floating point constants for both SF and DFmode. A
> pattern to handle the addressing of label_refs had to be included as well
> since
> all "memory_operand" patterns are disabled when TARGET_CANNOT_FORCE_CONST_MEM
> returns true. Also the pattern for splitting 32-bit immediates had to be
> changed, it was not accepting unsigned 32-bit unsigned integers with the MSB
> set. I believe const_int_operand expects the mode of the operand to be set to
> VOIDmode and not SImode. I have only changed it in the patterns that were
> affecting this code, though I suggest looking into changing it in the rest of
> the ARM backend.
>
> Additionally, the use of thread-local storage is disabled if literal pools are
> disabled, as there are no relocations for TLS variables and incorrect code is
> generated as a result. The patch now emits a diagnostic in TLS-enabled
> toolchains if a TLS symbol is found when -mpure-code or -mslow-flash-data are
> enabled.
>
Thanks, that helps a lot.
+ {
+ /* ARM currently does not provide relocations to encode TLS variables
ARM ELF does not define relocations ...
+ /* Make sure we do not attempt to create a literal pool even though
it should
+ no longer be necessary to create any. */
+ if (arm_disable_literal_pool)
+ return ;
+
It would be safer to run through the code and then assert that fixups
aren't needed; though that would cost a little computation time. I
think you could put such an assert at the start of push_minipool_fix.
OK with those changes.
R.
