| Message ID | 20170420095447.c3sls25mqkr7bwh3@e107464-lin.cambridge.arm.com |
|---|---|
| State | New |
| Headers | show |
On 20/04/17 10:54, Prakhar Bahuguna wrote: > [ARM] PR71607: Fix ICE when loading constant > > gcc/ChangeLog: > > 2017-04-18 Andre Vieira <andre.simoesdiasvieira@arm.com> > Prakhar Bahuguna <prakhar.bahuguna@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_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 <andre.simoesdiasvieira@arm.com> > Thomas Preud'homme <thomas.preudhomme@arm.com> > Prakhar Bahuguna <prakhar.bahuguna@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. > * 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. > -- > > Prakhar Bahuguna > > > 0001-ARM-PR71607-Fix-ICE-when-loading-constant.patch > > > From 985100bbf8f168ab9a88ca29869453844eb6b58e Mon Sep 17 00:00:00 2001 > From: Prakhar Bahuguna <prakhar.bahuguna@arm.com> > Date: Tue, 18 Apr 2017 14:16:46 +0100 > Subject: [PATCH] [ARM] PR71607: Fix ICE when loading constant > > --- > gcc/config/arm/arm.c | 20 +++++++++--- > gcc/config/arm/arm.md | 9 ++---- > gcc/config/arm/vfp.md | 37 ++++++++++++++++++++++ > ...low-flash-data.c => thumb2-slow-flash-data-1.c} | 0 > .../gcc.target/arm/thumb2-slow-flash-data-2.c | 28 ++++++++++++++++ > .../gcc.target/arm/thumb2-slow-flash-data-3.c | 25 +++++++++++++++ > .../gcc.target/arm/thumb2-slow-flash-data-4.c | 26 +++++++++++++++ > .../gcc.target/arm/thumb2-slow-flash-data-5.c | 14 ++++++++ > .../gcc.target/arm/tls-disable-literal-pool.c | 15 +++++++++ > 9 files changed, 163 insertions(+), 11 deletions(-) > rename gcc/testsuite/gcc.target/arm/{thumb2-slow-flash-data.c => thumb2-slow-flash-data-1.c} (100%) > create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c > create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c > create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c > create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c > create mode 100644 gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index a2d80cfd645..01d8c52d8c5 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -8633,7 +8633,16 @@ arm_tls_referenced_p (rtx x) > { > const_rtx x = *iter; > if (GET_CODE (x) == SYMBOL_REF && SYMBOL_REF_TLS_MODEL (x) != 0) > - return true; > + { > + /* ARM currently does not provide relocations to encode TLS variables > + into AArch32 instructions, only data, so there is no way to > + currently implement these if a literal pool is disabled. */ > + if (arm_disable_literal_pool) > + sorry ("accessing thread-local storage is not currently supported " > + "with -mpure-code or -mslow-flash-data"); > + > + return true; > + } > > /* Don't recurse into UNSPEC_TLS looking for TLS symbols; these are > TLS offsets, not real symbol references. */ > @@ -16391,10 +16400,6 @@ push_minipool_fix (rtx_insn *insn, HOST_WIDE_INT address, rtx *loc, > int > arm_max_const_double_inline_cost () > { > - /* Let the value get synthesized to avoid the use of literal pools. */ > - if (arm_disable_literal_pool) > - return 99; > - > return ((optimize_size || arm_ld_sched) ? 3 : 4); > } > > @@ -17341,6 +17346,11 @@ arm_reorg (void) > if (!optimize) > split_all_insns_noflow (); > > + /* 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 ; > + > minipool_fix_head = minipool_fix_tail = NULL; > > /* The first insn must always be a note, or the code below won't > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > index 21cfe3a4c31..f9365cde504 100644 > --- a/gcc/config/arm/arm.md > +++ b/gcc/config/arm/arm.md > @@ -233,10 +233,6 @@ > (match_test "arm_restrict_it")) > (const_string "no") > > - (and (eq_attr "use_literal_pool" "yes") > - (match_test "arm_disable_literal_pool")) > - (const_string "no") > - > (eq_attr "arch_enabled" "no") > (const_string "no")] > (const_string "yes"))) > @@ -5878,8 +5874,9 @@ > (match_operand:ANY64 1 "immediate_operand" ""))] > "TARGET_32BIT > && reload_completed > - && (arm_const_double_inline_cost (operands[1]) > - <= arm_max_const_double_inline_cost ())" > + && (arm_disable_literal_pool > + || (arm_const_double_inline_cost (operands[1]) > + <= arm_max_const_double_inline_cost ()))" > [(const_int 0)] > " > arm_split_constant (SET, SImode, curr_insn, > diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md > index befdea9edd9..d8f77e2ffe4 100644 > --- a/gcc/config/arm/vfp.md > +++ b/gcc/config/arm/vfp.md > @@ -2079,3 +2079,40 @@ > ;; fmdhr et al (VFPv1) > ;; Support for xD (single precision only) variants. > ;; fmrrs, fmsrr > + > +;; Split an immediate DF move to two immediate SI moves. > +(define_insn_and_split "no_literal_pool_df_immediate" > + [(set (match_operand:DF 0 "s_register_operand" "") > + (match_operand:DF 1 "const_double_operand" ""))] > + "TARGET_THUMB2 && arm_disable_literal_pool > + && !(TARGET_HARD_FLOAT && TARGET_VFP_DOUBLE > + && vfp3_const_double_rtx (operands[1]))" > + "#" > + "&& !reload_completed" > + [(set (subreg:SI (match_dup 1) 0) (match_dup 2)) > + (set (subreg:SI (match_dup 1) 4) (match_dup 3)) > + (set (match_dup 0) (match_dup 1))] > + " > + long buf[2]; > + real_to_target (buf, CONST_DOUBLE_REAL_VALUE (operands[1]), DFmode); > + operands[2] = GEN_INT ((int) buf[0]); > + operands[3] = GEN_INT ((int) buf[1]); > + operands[1] = gen_reg_rtx (DFmode); > + ") > + > +;; Split an immediate SF move to one immediate SI move. > +(define_insn_and_split "no_literal_pool_sf_immediate" > + [(set (match_operand:SF 0 "s_register_operand" "") > + (match_operand:SF 1 "const_double_operand" ""))] > + "TARGET_THUMB2 && arm_disable_literal_pool > + && !(TARGET_HARD_FLOAT && vfp3_const_double_rtx (operands[1]))" > + "#" > + "&& !reload_completed" > + [(set (subreg:SI (match_dup 1) 0) (match_dup 2)) > + (set (match_dup 0) (match_dup 1))] > + " > + long buf; > + real_to_target (&buf, CONST_DOUBLE_REAL_VALUE (operands[1]), SFmode); > + operands[2] = GEN_INT ((int) buf); > + operands[1] = gen_reg_rtx (SFmode); > + ") > diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c > similarity index 100% > rename from gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data.c > rename to gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c > diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c > new file mode 100644 > index 00000000000..6e76043daee > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c > @@ -0,0 +1,28 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target arm_cortex_m } */ > +/* { dg-require-effective-target arm_thumb2_ok } */ > +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ > +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ > +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */ > + > +float f (float); > + > +const float max = 0.01f; > + > +int > +g (float in) > +{ > + if (f (in) + f (in) < max) > + return 0; > + return 1; > +} > + > +double foo (void) > +{ > + return 0xF1EC7A5239123AF; > +} > + > +double bar (void) > +{ > + return 0.0f; > +} > diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c > new file mode 100644 > index 00000000000..fe7a12bf99b > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c > @@ -0,0 +1,25 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target arm_cortex_m } */ > +/* { dg-require-effective-target arm_thumb2_ok } */ > +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ > +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ > +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -mthumb -mslow-flash-data" } */ > + > +/* From PR71607 */ > + > +float b; > +void fn1 (); > + > +float > +fn2 () > +{ > + return 1.1f; > +} > + > +void > +fn3 () > +{ > + float a[2]; > + a[1] = b; > + fn1 (a); > +} > diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c > new file mode 100644 > index 00000000000..cc5aea4437f > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c > @@ -0,0 +1,26 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target arm_cortex_m } */ > +/* { dg-require-effective-target arm_thumb2_ok } */ > +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ > +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ > +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */ > + > +double __attribute__ ((target ("fpu=fpv5-d16"))) > +foo (void) > +{ > + return 1.0f; > +} > + > +float __attribute__ ((target ("fpu=fpv5-d16"))) > +bar (void) > +{ > + return 1.0f; > +} > + > +float __attribute__ ((target ("fpu=fpv5-sp-d16"))) > +baz (void) > +{ > + return 1.0f; > +} > + > +/* { dg-final { scan-assembler-times "#1\\.0e\\+0" 3 } } */ > diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c > new file mode 100644 > index 00000000000..b9161c4a4b4 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target arm_cortex_m } */ > +/* { dg-require-effective-target arm_thumb2_ok } */ > +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ > +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ > +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */ > + > +double __attribute__ ((target ("fpu=fpv5-sp-d16"))) > +foo (void) > +{ > + return 1.0f; > +} > + > +/* { dg-final { scan-assembler-not "#1\\.0e\\+0" } } */ > diff --git a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c > new file mode 100644 > index 00000000000..fe14a6b132c > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c > @@ -0,0 +1,15 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target tls } */ > +/* { dg-require-effective-target arm_cortex_m } */ > +/* { dg-require-effective-target arm_thumb2_ok } */ > +/* { dg-options "-mslow-flash-data" } */ > + > +__thread int x = 0; > + > +int > +bar () > +{ > + return x; > +} > + > +/* { dg-error "accessing thread-local storage is not currently supported with -mpure-code or -mslow-flash-data" "" { target *-*-* } 12 } */ >
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 <andre.simoesdiasvieira@arm.com> > > Prakhar Bahuguna <prakhar.bahuguna@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_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 <andre.simoesdiasvieira@arm.com> > > Thomas Preud'homme <thomas.preudhomme@arm.com> > > Prakhar Bahuguna <prakhar.bahuguna@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. > > * 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.
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 <andre.simoesdiasvieira@arm.com> >>> Prakhar Bahuguna <prakhar.bahuguna@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_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 <andre.simoesdiasvieira@arm.com> >>> Thomas Preud'homme <thomas.preudhomme@arm.com> >>> Prakhar Bahuguna <prakhar.bahuguna@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. >>> * 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.
On 04/20/2017 11:54 AM, Prakhar Bahuguna wrote: > diff --git a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c > new file mode 100644 > index 00000000000..fe14a6b132c > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c > @@ -0,0 +1,15 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target tls } */ > +/* { dg-require-effective-target arm_cortex_m } */ > +/* { dg-require-effective-target arm_thumb2_ok } */ > +/* { dg-options "-mslow-flash-data" } */ > + > +__thread int x = 0; > + > +int > +bar () > +{ > + return x; > +} > + > +/* { dg-error "accessing thread-local storage is not currently supported with -mpure-code or -mslow-flash-data" "" { target *-*-* } 12 } */ Absolute line numbers make the testsuite harder to maintain, and in this case it's not necessary. Please put the dg-error: - on the same line as the return, and remove the line number, or - on the line after the return, and use relative line number .-1 Thanks, - Tom
Hi, On 5 May 2017 at 15:19, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote: > 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 <andre.simoesdiasvieira@arm.com> >>>> Prakhar Bahuguna <prakhar.bahuguna@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_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 <andre.simoesdiasvieira@arm.com> >>>> Thomas Preud'homme <thomas.preudhomme@arm.com> >>>> Prakhar Bahuguna <prakhar.bahuguna@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. >>>> * gcc.target/arm/tls-disable-literal-pool.c: New. I've noticed that the last new test (tls-disable-literal-pool.c) fails on arm-eabi --with-mode=thumb --with-cpu=cortex-m3: no error message is generated. Thanks, Christophe >>>> >>>> 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.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index a2d80cfd645..01d8c52d8c5 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -8633,7 +8633,16 @@ arm_tls_referenced_p (rtx x) { const_rtx x = *iter; if (GET_CODE (x) == SYMBOL_REF && SYMBOL_REF_TLS_MODEL (x) != 0) - return true; + { + /* ARM currently does not provide relocations to encode TLS variables + into AArch32 instructions, only data, so there is no way to + currently implement these if a literal pool is disabled. */ + if (arm_disable_literal_pool) + sorry ("accessing thread-local storage is not currently supported " + "with -mpure-code or -mslow-flash-data"); + + return true; + } /* Don't recurse into UNSPEC_TLS looking for TLS symbols; these are TLS offsets, not real symbol references. */ @@ -16391,10 +16400,6 @@ push_minipool_fix (rtx_insn *insn, HOST_WIDE_INT address, rtx *loc, int arm_max_const_double_inline_cost () { - /* Let the value get synthesized to avoid the use of literal pools. */ - if (arm_disable_literal_pool) - return 99; - return ((optimize_size || arm_ld_sched) ? 3 : 4); } @@ -17341,6 +17346,11 @@ arm_reorg (void) if (!optimize) split_all_insns_noflow (); + /* 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 ; + minipool_fix_head = minipool_fix_tail = NULL; /* The first insn must always be a note, or the code below won't diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 21cfe3a4c31..f9365cde504 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -233,10 +233,6 @@ (match_test "arm_restrict_it")) (const_string "no") - (and (eq_attr "use_literal_pool" "yes") - (match_test "arm_disable_literal_pool")) - (const_string "no") - (eq_attr "arch_enabled" "no") (const_string "no")] (const_string "yes"))) @@ -5878,8 +5874,9 @@ (match_operand:ANY64 1 "immediate_operand" ""))] "TARGET_32BIT && reload_completed - && (arm_const_double_inline_cost (operands[1]) - <= arm_max_const_double_inline_cost ())" + && (arm_disable_literal_pool + || (arm_const_double_inline_cost (operands[1]) + <= arm_max_const_double_inline_cost ()))" [(const_int 0)] " arm_split_constant (SET, SImode, curr_insn, diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md index befdea9edd9..d8f77e2ffe4 100644 --- a/gcc/config/arm/vfp.md +++ b/gcc/config/arm/vfp.md @@ -2079,3 +2079,40 @@ ;; fmdhr et al (VFPv1) ;; Support for xD (single precision only) variants. ;; fmrrs, fmsrr + +;; Split an immediate DF move to two immediate SI moves. +(define_insn_and_split "no_literal_pool_df_immediate" + [(set (match_operand:DF 0 "s_register_operand" "") + (match_operand:DF 1 "const_double_operand" ""))] + "TARGET_THUMB2 && arm_disable_literal_pool + && !(TARGET_HARD_FLOAT && TARGET_VFP_DOUBLE + && vfp3_const_double_rtx (operands[1]))" + "#" + "&& !reload_completed" + [(set (subreg:SI (match_dup 1) 0) (match_dup 2)) + (set (subreg:SI (match_dup 1) 4) (match_dup 3)) + (set (match_dup 0) (match_dup 1))] + " + long buf[2]; + real_to_target (buf, CONST_DOUBLE_REAL_VALUE (operands[1]), DFmode); + operands[2] = GEN_INT ((int) buf[0]); + operands[3] = GEN_INT ((int) buf[1]); + operands[1] = gen_reg_rtx (DFmode); + ") + +;; Split an immediate SF move to one immediate SI move. +(define_insn_and_split "no_literal_pool_sf_immediate" + [(set (match_operand:SF 0 "s_register_operand" "") + (match_operand:SF 1 "const_double_operand" ""))] + "TARGET_THUMB2 && arm_disable_literal_pool + && !(TARGET_HARD_FLOAT && vfp3_const_double_rtx (operands[1]))" + "#" + "&& !reload_completed" + [(set (subreg:SI (match_dup 1) 0) (match_dup 2)) + (set (match_dup 0) (match_dup 1))] + " + long buf; + real_to_target (&buf, CONST_DOUBLE_REAL_VALUE (operands[1]), SFmode); + operands[2] = GEN_INT ((int) buf); + operands[1] = gen_reg_rtx (SFmode); + ") diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c similarity index 100% rename from gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data.c rename to gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c new file mode 100644 index 00000000000..6e76043daee --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_cortex_m } */ +/* { dg-require-effective-target arm_thumb2_ok } */ +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */ + +float f (float); + +const float max = 0.01f; + +int +g (float in) +{ + if (f (in) + f (in) < max) + return 0; + return 1; +} + +double foo (void) +{ + return 0xF1EC7A5239123AF; +} + +double bar (void) +{ + return 0.0f; +} diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c new file mode 100644 index 00000000000..fe7a12bf99b --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_cortex_m } */ +/* { dg-require-effective-target arm_thumb2_ok } */ +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -mthumb -mslow-flash-data" } */ + +/* From PR71607 */ + +float b; +void fn1 (); + +float +fn2 () +{ + return 1.1f; +} + +void +fn3 () +{ + float a[2]; + a[1] = b; + fn1 (a); +} diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c new file mode 100644 index 00000000000..cc5aea4437f --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_cortex_m } */ +/* { dg-require-effective-target arm_thumb2_ok } */ +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */ + +double __attribute__ ((target ("fpu=fpv5-d16"))) +foo (void) +{ + return 1.0f; +} + +float __attribute__ ((target ("fpu=fpv5-d16"))) +bar (void) +{ + return 1.0f; +} + +float __attribute__ ((target ("fpu=fpv5-sp-d16"))) +baz (void) +{ + return 1.0f; +} + +/* { dg-final { scan-assembler-times "#1\\.0e\\+0" 3 } } */ diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c new file mode 100644 index 00000000000..b9161c4a4b4 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_cortex_m } */ +/* { dg-require-effective-target arm_thumb2_ok } */ +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */ +/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */ +/* { dg-options "-march=armv7e-m -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */ + +double __attribute__ ((target ("fpu=fpv5-sp-d16"))) +foo (void) +{ + return 1.0f; +} + +/* { dg-final { scan-assembler-not "#1\\.0e\\+0" } } */ diff --git a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c new file mode 100644 index 00000000000..fe14a6b132c --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target tls } */ +/* { dg-require-effective-target arm_cortex_m } */ +/* { dg-require-effective-target arm_thumb2_ok } */ +/* { dg-options "-mslow-flash-data" } */ + +__thread int x = 0; + +int +bar () +{ + return x; +} + +/* { dg-error "accessing thread-local storage is not currently supported with -mpure-code or -mslow-flash-data" "" { target *-*-* } 12 } */
[ARM] PR71607: Fix ICE when loading constant gcc/ChangeLog: 2017-04-18 Andre Vieira <andre.simoesdiasvieira@arm.com> Prakhar Bahuguna <prakhar.bahuguna@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_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 <andre.simoesdiasvieira@arm.com> Thomas Preud'homme <thomas.preudhomme@arm.com> Prakhar Bahuguna <prakhar.bahuguna@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. * gcc.target/arm/tls-disable-literal-pool.c: New. Okay for stage1? -- Prakhar Bahuguna From 985100bbf8f168ab9a88ca29869453844eb6b58e Mon Sep 17 00:00:00 2001 From: Prakhar Bahuguna <prakhar.bahuguna@arm.com> Date: Tue, 18 Apr 2017 14:16:46 +0100 Subject: [PATCH] [ARM] PR71607: Fix ICE when loading constant --- gcc/config/arm/arm.c | 20 +++++++++--- gcc/config/arm/arm.md | 9 ++---- gcc/config/arm/vfp.md | 37 ++++++++++++++++++++++ ...low-flash-data.c => thumb2-slow-flash-data-1.c} | 0 .../gcc.target/arm/thumb2-slow-flash-data-2.c | 28 ++++++++++++++++ .../gcc.target/arm/thumb2-slow-flash-data-3.c | 25 +++++++++++++++ .../gcc.target/arm/thumb2-slow-flash-data-4.c | 26 +++++++++++++++ .../gcc.target/arm/thumb2-slow-flash-data-5.c | 14 ++++++++ .../gcc.target/arm/tls-disable-literal-pool.c | 15 +++++++++ 9 files changed, 163 insertions(+), 11 deletions(-) rename gcc/testsuite/gcc.target/arm/{thumb2-slow-flash-data.c => thumb2-slow-flash-data-1.c} (100%) create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c create mode 100644 gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c create mode 100644 gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c