This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, ARM] Improve robustness of -mslow-flash-data
- From: Christophe Lyon <christophe dot lyon at linaro dot org>
- To: "Thomas Preud'homme" <thomas dot preudhomme at foss dot arm dot com>
- Cc: Kyrylo Tkachov <kyrylo dot tkachov at arm dot com>, Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>, Richard Earnshaw <richard dot earnshaw at arm dot com>, gcc Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 20 Nov 2018 11:22:40 +0100
- Subject: Re: [PATCH, ARM] Improve robustness of -mslow-flash-data
- References: <69515af5-8d73-4b1e-d58f-23c08720c180@foss.arm.com>
On Mon, 19 Nov 2018 at 18:56, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
>
> Hi,
>
> Current code to handle -mslow-flash-data in machine description files
> suffers from a number of issues which this patch fixes:
>
> 1) The insn_and_split in vfp.md to load a generic floating-point
> constant via GPR first and move it to VFP register are guarded by
> !reload_completed which is forbidden explicitely in the GCC internals
> documentation section 17.2 point 3;
>
> 2) A number of testcase in the testsuite ICEs under -mslow-flash-data
> when targeting the hardfloat ABI [1];
>
> 3) Instructions performing load from literal pool are not disabled.
>
> These problems are addressed by 2 separate actions:
>
> 1) Making the splitters take a clobber and changing the expanders
> accordingly to generate a mov with clobber in cases where a literal
> pool would be used. The splitter can thus be enabled after reload since
> it does not call gen_reg_rtx anymore;
>
> 2) Adding new predicates and constraints to disable literal pool loads
> in existing instructions when -mslow-flash-data is in effect.
>
> The patch also rework the splitter for DFmode slightly to generate an
> intermediate DI load instead of 2 intermediate SI loads, thus relying on
> the existing DI splitters instead of redoing their job. At last, the
> patch adds some missing arm_fp_ok effective target to some of the
> slow-flash-data testcases.
>
> [1]
> c-c++-common/Wunused-var-3.c
> gcc.c-torture/compile/pr72771.c
> gcc.c-torture/compile/vector-5.c
> gcc.c-torture/compile/vector-6.c
> gcc.c-torture/execute/20030914-1.c
> gcc.c-torture/execute/20050316-1.c
> gcc.c-torture/execute/pr59643.c
> gcc.dg/builtin-tgmath-1.c
> gcc.dg/debug/pr55730.c
> gcc.dg/graphite/interchange-7.c
> gcc.dg/pr56890-2.c
> gcc.dg/pr68474.c
> gcc.dg/pr80286.c
> gcc.dg/torture/pr35227.c
> gcc.dg/torture/pr65077.c
> gcc.dg/torture/pr86363.c
> g++.dg/torture/pr81112.C
> g++.dg/torture/pr82985.C
> g++.dg/warn/Wunused-var-7.C
> and a lot more in libstdc++ in special_functions/*_comp_ellint_* and
> special_functions/*_ellint_* directories.
>
> ChangeLog entries are as follows:
>
> *** gcc/ChangeLog ***
>
> 2018-11-14 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * config/arm/arm.md (arm_movdi): Split if -mslow-flash-data and
> source is a constant that would be loaded by literal pool.
> (movsf expander): Generate a no_literal_pool_sf_immediate insn if
> -mslow-flash-data is present, targeting hardfloat ABI and source is a
> float constant that cannot be loaded via vmov.
> (movdf expander): Likewise but generate a no_literal_pool_df_immediate
> insn.
> (arm_movsf_soft_insn): Split if -mslow-flash-data and source is a
> float constant that would be loaded by literal pool.
> (softfloat constant movsf splitter): Splitter for the above case.
> (movdf_soft_insn): Split if -mslow-flash-data and source is a float
> constant that would be loaded by literal pool.
> (softfloat constant movdf splitter): Splitter for the above case.
> * config/arm/constraints.md (Pz): Document existing constraint.
> (Ha): Define constraint.
> (Tu): Likewise.
> * config/arm/predicates.md (hard_sf_operand): New predicate.
> (hard_df_operand): Likewise.
> * config/arm/thumb2.md (thumb2_movsi_insn): Split if
> -mslow-flash-data and constant would be loaded by literal pool.
> * constant/arm/vfp.md (thumb2_movsi_vfp): Likewise and disable constant
> load in VFP register.
> (movdi_vfp): Likewise.
> (thumb2_movsf_vfp): Use hard_sf_operand as predicate for source to
> prevent match for a constant load if -mslow-flash-data and constant
> cannot be loaded via vmov. Adapt constraint accordingly by
> using Ha instead of E for generic floating-point constant load.
> (thumb2_movdf_vfp): Likewise using hard_df_operand predicate instead.
> (no_literal_pool_df_immediate): Add a clobber to use as the
> intermediate general purpose register and also enable it after reload
> but disable it constant is a valid FP constant. Add constraints and
> generate a DI intermediate load rather than 2 SI loads.
> (no_literal_pool_sf_immediate): Add a clobber to use as the
> intermediate general purpose register and also enable it after
> reload.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2018-11-14 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * gcc.target/arm/thumb2-slow-flash-data-2.c: Require arm_fp_ok
> effective target.
> * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.
> * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.
> * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.
>
> Testing: Built arm-none-eabi cross compilers for Armv7E-M defaulting to
> softfloat and hardfloat ABI which showed no regression and some
> FAIL->PASS for hardfloat ABI. Bootstraped on Arm and Thumb-2 without any
> regression. Compiled SPEC2k6 without -mslow-flash-data and checked that
> code generation didn't change.
>
FWIW, it's OK for my validations: I do not see the improvements you mention,
because I do not have this very configuration. On arm-eabi --with-cpu=cortex-m3,
I only see the above tests PASS -> UNSUPPORTED, which is OK
(the reason being "-mfloat-abi=hard: selected procesor lacks an FPU"_
Thanks,
Christophe
> Is this ok for stage3?
>
> Best regards,
>
> Thomas