[PATCH] D43079: [TTI CostModel] change default cost of FP ops to 1 (PR36280)

[Adam Nemet via Phabricator via llvm-commits]

anemet added a comment.

In https://reviews.llvm.org/D43079#1013886, @spatel wrote:

> In https://reviews.llvm.org/D43079#1013835, @anemet wrote:
>
> > Please revert until these things get worked out so that we can properly track performance.   We are seeing many regressions including 17% on 444.namd and 12% on 482.sphinx3 in SPECfp 2006.
>
>
> Hi Adam -
>
> Rather than reverting for all targets, can we just hack ARM/AArch with patches like this (if we can add at least one test to show what changed, that would be better of course) :
>
>   Index: lib/Target/ARM/ARMTargetTransformInfo.cpp
>   ===================================================================
>   --- lib/Target/ARM/ARMTargetTransformInfo.cpp	(revision 325579)
>   +++ lib/Target/ARM/ARMTargetTransformInfo.cpp	(working copy)
>   @@ -514,6 +514,13 @@
>      int Cost = BaseT::getArithmeticInstrCost(Opcode, Ty, Op1Info, Op2Info,
>                                               Opd1PropInfo, Opd2PropInfo);
>   
>   +  // Assume that floating point arithmetic operations cost twice as much as
>   +  // integer operations.
>   +  // FIXME: This is a win on several perf benchmarks running on CPU model ???,
>   +  // but there are no regression tests that show why or how this is good.
>   +  if (Ty->isFPOrFPVectorTy())
>   +    Cost *= 2;
>   +
>      // This is somewhat of a hack. The problem that we are facing is that SROA
>      // creates a sequence of shift, and, or instructions to construct values.
>      // These sequences are recognized by the ISel and have zero-cost. Not so for
>  
>


Seeing such major swings, my preference would be to revert and put the new version up for review (I think that your hack works).  Then commit the new combined version after a few days so that the perf bots got a chance to recover.  What do you think?

Adam


Repository:
  rL LLVM

https://reviews.llvm.org/D43079