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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: include/llvm/CodeGen/BasicTTIImpl.h:494
+    // assumption when they can provide more accurate information.
+    unsigned OpCost = 1;
 
----------------
hfinkel wrote:
> spatel wrote:
> > hfinkel wrote:
> > > spatel wrote:
> > > > ABataev wrote:
> > > > > Maybe it is better to add a virtual method that will return the default cost of the integer and floating point operations for the target? The default implementation should keep 1 and 2, but for X86 it should return 1 in both cases.
> > > > I don't know of any target where anything but '1' is the right default answer, so I'd rather not perpetuate this. I think it's better to correct the problems revealed by this change.
> > > I agree, but recall that these are measuring reciprocal throughputs, and it's not unreasonable to assume that floating-point ops will have lower throughputs than integer operations in some general sense. Nevertheless, '2' seems somewhat arbitrary in this regard.
> > Ah, the code comment isn't accurate then. And since we're using integers, this is really a relative reciprocal throughput (can't go under 1) rather than an actual cycle-based value?
> Not sure what you mean by "cycle based." All cost model results are relative, but this cost model used by the vectorizer is supposed to return (relative) reciprocal throughputs. We now have a separate cost model for latency (and a separate "user cost" model, which essentially models micro-op count).
In the backend, the scheduler model shows instruction timing like this (in test/CodeGen/X86/sse2-schedule.ll):
; SKYLAKE-NEXT:    vaddpd %xmm1, %xmm0, %xmm0 # sched: [4:0.50]

The "0.50" means we can execute 2 of these per cycle because there are 2 units that process these instructions.

We're integer 1-based here, so if we want to show that a target has 3 integer adders and 1 FP adder, we would make FP ops cost 3 rather than 1 (instead of the 0.33 for integer adds that we would expect in the scheduler model).


https://reviews.llvm.org/D43079