Re: [PATCH] PR target/65240, Fix Power{7,8} insn constraint issue with -O3 -ffast-math

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Mar 18, 2015 at 12:18 AM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> On Thu, Mar 12, 2015 at 11:37:14AM -0400, David Edelsohn wrote:
>> Please check on the performance implications of removing the special
>> constant support.  I know that it is late, but I think that ripping it
>> out is less risky than trying to fix this, if the performance impact
>> is not bad.
>
> Now, I haven't drilled down to exactly what is causing the performance
> differences, but I've done some Spec 2006 runs comparing subversion id 221194,
> with the two patches.
>
> The first patch is a rewrite of the code that I originally put into the
> compiler to move floating point constants under -ffast-math during the first
> split pass.  A minor tweak would need to be done to the original patch so that
> it works with -mcmodel=small or -m32 options.
>
> The second patch completely eliminates keeping the non-0 constant around in
> RTL, and pushes it to memory during the initial RTL generation, since it is
> felt that the RTL optimizations no longer need the constant in RTL to convert
> division by constant into multiplication by the reciprocal.
>
> The benchmarcks that show a difference are.  Note, I do not count benchmarks
> that differ by less than 2% to be significant.  Percentages more than 100% mean
> the benchmark ran faster:
>
> Benchmark               Patch-1         Patch-2
> =========               =======         =======
> 401.bzip2               102.59%         103.51%
> 462.libquantum          100.28%          97.52%
> 483.xalancbmk            97.72%          97.90%
> 435.gromacs             104.48%          99.39%
> 436.cactusADM           102.19%         102.90%
> 470.lbm                 100.39%          97.45%
> Spec INT score           99.86%          99.86%
> Spec FP score           100.50%          99.81%
>
> Patch #1 had 3 faster benchmarks and 1 slower benchmark.  Patch #2 had 2 faster
> benchmarks, and 3 slower benchmarks.

Did you double-check if there are any differences in generated code?
Esp. the SPEC INT benchmarks look odd - they don't contain any
FP code.

Richard.

> I tend to feel patch #2 is cleaner, though it is slightly slower.  However, I
> can go with patch #1 if desired.
>
> Patch #2 bootstrapped fine, and had no regressions in the test suite.  Did
> you want me to install patch #1, patch #2, or do you want more information?
>
> [gcc]
> 2015-03-17  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/65240
>         * config/rs6000/predicates.md (easy_fp_constant): Remove special
>         -ffast-math handling that kept non-0 constants live in the RTL
>         until reload.  Remove logic testing the number of instructions it
>         took to create a constant in a GPR that was never used, due to a
>         test for soft-float earlier.
>         (memory_fp_constant): Delete, no longer used.
>
>         * config/rs6000/rs6000.md (mov<MODE>_hardfloat): Remove
>         alternatives for loading non-0 constants into GPRs for hard
>         floating point that is no longer needed due to changes in
>         easy_fp_constant.  Add support for loading 0.0 into GPRs.
>         (mov<mode>_hardfloat32): Likewise.
>         (mov<mode>_hardfloat64): Likewise.
>         (mov<mode>_64bit_dm): Likewise.
>         (movtd_64bit_nodm): Likewise.
>         (pre-reload move FP constant define_split): Delete define_split,
>         since it is no longer used.
>         (extenddftf2_internal): Remove GHF constraints that are not valid
>         for extenddftf2.
>
> [gcc/testsuite]
> 2015-03-17  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/65240
>         * gcc/testsuite/g++.dg/pr65240.h: Add tests for PR 65240.
>         * gcc/testsuite/g++.dg/pr65240-1.C: Likewise.
>         * gcc/testsuite/g++.dg/pr65240-2.C: Likewise.
>         * gcc/testsuite/g++.dg/pr65240-3.C: Likewise.
>         * gcc/testsuite/g++.dg/pr65240-4.C: Likewise.
>
> --
> Michael Meissner, IBM
> IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
> email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797