Re: [PATCH, GCC/ARM/gcc-7-branch] Backport PR71607

[Richard Sandiford <richard.sandiford@linaro.org>]

Prakhar Bahuguna <prakhar.bahuguna@arm.com> writes:
> 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.

I couldn't see the const_int_operand bit in the attached patch, but:
const_int_operand *should* usually be used with the logical integer
mode, such as SImode or DImode.

const_ints are supposed to be stored in sign-extended form, so a 32-bit
integer with the MSB set should be 0xffffffff80000000|x instead of
0x80000000|x.  It's a bug if you have one where that isn't true.

In the patch it looks like this could come from:

> 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);
> +  ")

...these two splits, where the GEN_INTs should probably be:

  gen_int_mode (..., SImode);

instead.

Thanks,
Richard