This is an archive of the discontinued LLVM Phabricator instance.

TTI: Honour cost model for estimating cost of vector-intrinsic and calls.
ClosedPublic

Authored by mzolotukhin on Mar 5 2015, 7:57 PM.

Details

Summary

TTI: Honour cost model for estimating cost of vector-intrinsic and calls.
Update tests in correspondance with changes in cost model.

This is the fifth of 6 patches for enabling vectorization of calls.

Diff Detail

Event Timeline

mzolotukhin updated this revision to Diff 21327.Mar 5 2015, 7:57 PM
mzolotukhin retitled this revision from to TTI: Honour cost model for estimating cost of vector-intrinsic and calls..
mzolotukhin updated this object.
mzolotukhin edited the test plan for this revision. (Show Details)
mzolotukhin added a subscriber: Unknown Object (MLST).
hfinkel added inline comments.Mar 5 2015, 8:52 PM
include/llvm/CodeGen/BasicTTIImpl.h
548

Why are you changing the default cost for intrinsics from one to ten? I believe it used to be one, and that was because we made an explicit decision to make intrinsics default to something cheap.

mzolotukhin added inline comments.Mar 6 2015, 10:59 AM
include/llvm/CodeGen/BasicTTIImpl.h
548

I made two changes here:

  1. Change 10 * Cost * Num to ScalarCalls * ScalarCost + ScalarizationOverhead. I believe factor 10 initially was used to model the scalarization overhead, but now we can estimate it more accurately.
  1. Change ScalarCalls + ScalarizationCost to ScalarCalls*10 + ScalarizationCost. To me it looked a bit inconsistent: when an unknown intrinsic has a vector type, we returned ScalarCalls + ScalarizationCost (effectively, ScalarCost is implicitly assumed to be 1). However, for known intrinsics scalar cost is 10 (unless the operation is legal and we know exact cost). I can revert this change, but the original code just looked a bit odd to me.

However, I can find explanation like "we expect all unknown intrinsics to be lowered to somewhat cheap (cost=1), in contrast to the listed ones, which will definitely be lowered to an expensive libcall (cost=10)". If that's the case, I agree that the original code is correct.

hfinkel added inline comments.Mar 6 2015, 11:14 AM
include/llvm/CodeGen/BasicTTIImpl.h
548

However, I can find explanation like "we expect all unknown intrinsics to be lowered to somewhat cheap (cost=1), in contrast to the listed ones, which will definitely be lowered to an expensive libcall (cost=10)". If that's the case, I agree that the original code is correct.

Yes, I believe that was the logic, and the original code was consistent with that reasoning.

mzolotukhin updated this revision to Diff 21404.Mar 6 2015, 3:14 PM
  • Address Hal's remark: do not change cost of scalar intrinsics from 1 to 10.
mzolotukhin updated this revision to Diff 21405.Mar 6 2015, 3:28 PM
  • Readjust tests back.
hfinkel accepted this revision.Mar 8 2015, 2:53 PM
hfinkel edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Mar 8 2015, 2:53 PM
mzolotukhin closed this revision.Mar 17 2015, 12:54 PM

Thanks, committed in r232528!