| Message ID | 28052a23-3c02-24f6-e699-1f29b0634520@arm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 27, 2017 at 01:32:11PM +0100, Richard Earnshaw (lists) wrote: > This patch fixes the regression caused by the changes to add square root > estimation when compiling for xgene-1 or exynos-m1 targets. > > The issue is that the expand path for the reciprocal estimate square > root pattern assumes that pattern cannot fail once it has been decided > that this expansion path is available, but because the logic deep inside > aarch64_emit_approx_sqrt() differs from use_rsqrt_p() the two disagree > as to what is safe. > > This patch refactors the logic to ensure that we cannot unknowingly make > different choices here. > > Bootstrap/testing completed ok. I'll apply this to trunk. > > Jakub: Are we having an RC2? If so, is this ok for the gcc-7 branch? > > PR target/80530 > * config/aarch64/aarch64.c (aarch64_emit_approx_sqrt): Ensure > that the logic for permitting reciprocal estimates matches that > in use_rsqrt_p. No testcase? We will have RC2 (later today or tomorrow), so I guess this is ok if you believe it is important enough to have in 7.1 and very low risk. Jakub
On 27/04/17 14:26, Jakub Jelinek wrote: > On Thu, Apr 27, 2017 at 01:32:11PM +0100, Richard Earnshaw (lists) wrote: >> This patch fixes the regression caused by the changes to add square root >> estimation when compiling for xgene-1 or exynos-m1 targets. >> >> The issue is that the expand path for the reciprocal estimate square >> root pattern assumes that pattern cannot fail once it has been decided >> that this expansion path is available, but because the logic deep inside >> aarch64_emit_approx_sqrt() differs from use_rsqrt_p() the two disagree >> as to what is safe. >> >> This patch refactors the logic to ensure that we cannot unknowingly make >> different choices here. >> >> Bootstrap/testing completed ok. I'll apply this to trunk. >> >> Jakub: Are we having an RC2? If so, is this ok for the gcc-7 branch? >> >> PR target/80530 >> * config/aarch64/aarch64.c (aarch64_emit_approx_sqrt): Ensure >> that the logic for permitting reciprocal estimates matches that >> in use_rsqrt_p. > > No testcase? > > We will have RC2 (later today or tomorrow), so I guess this is ok if you > believe it is important enough to have in 7.1 and very low risk. > > Jakub > Sorry, I should have said that this fixes an ICE with gcc.dg/tree-ssa/recip-1.c on affected platforms. Committing now. R.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 1e58e9d..6ef49da 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -7971,33 +7971,40 @@ aarch64_emit_approx_sqrt (rtx dst, rtx src, bool recp) machine_mode mode = GET_MODE (dst); if (GET_MODE_INNER (mode) == HFmode) - return false; + { + gcc_assert (!recp); + return false; + } - machine_mode mmsk = mode_for_vector - (int_mode_for_mode (GET_MODE_INNER (mode)), - GET_MODE_NUNITS (mode)); - bool use_approx_sqrt_p = (!recp - && (flag_mlow_precision_sqrt - || (aarch64_tune_params.approx_modes->sqrt - & AARCH64_APPROX_MODE (mode)))); - bool use_approx_rsqrt_p = (recp - && (flag_mrecip_low_precision_sqrt - || (aarch64_tune_params.approx_modes->recip_sqrt - & AARCH64_APPROX_MODE (mode)))); + machine_mode mmsk + = mode_for_vector (int_mode_for_mode (GET_MODE_INNER (mode)), + GET_MODE_NUNITS (mode)); + if (!recp) + { + if (!(flag_mlow_precision_sqrt + || (aarch64_tune_params.approx_modes->sqrt + & AARCH64_APPROX_MODE (mode)))) + return false; + + if (flag_finite_math_only + || flag_trapping_math + || !flag_unsafe_math_optimizations + || optimize_function_for_size_p (cfun)) + return false; + } + else + /* Caller assumes we cannot fail. */ + gcc_assert (use_rsqrt_p (mode)); - if (!flag_finite_math_only - || flag_trapping_math - || !flag_unsafe_math_optimizations - || !(use_approx_sqrt_p || use_approx_rsqrt_p) - || optimize_function_for_size_p (cfun)) - return false; rtx xmsk = gen_reg_rtx (mmsk); if (!recp) - /* When calculating the approximate square root, compare the argument with - 0.0 and create a mask. */ - emit_insn (gen_rtx_SET (xmsk, gen_rtx_NEG (mmsk, gen_rtx_EQ (mmsk, src, - CONST0_RTX (mode))))); + /* When calculating the approximate square root, compare the + argument with 0.0 and create a mask. */ + emit_insn (gen_rtx_SET (xmsk, + gen_rtx_NEG (mmsk, + gen_rtx_EQ (mmsk, src, + CONST0_RTX (mode))))); /* Estimate the approximate reciprocal square root. */ rtx xdst = gen_reg_rtx (mode);