This is an archive of the discontinued LLVM Phabricator instance.

[CostModel] Unify getMemoryOpCost
ClosedPublic

Authored by samparker on Jun 2 2020, 1:19 AM.

Details

Summary

Use getMemoryOpCost from the generic implementation of getUserCost and have getInstructionThroughput return the result of that for loads and stores. This also means that the X86 implementation of getUserCost can be removed with the functionality folded into its getMemoryOpCost.

Diff Detail

Event Timeline

samparker created this revision.Jun 2 2020, 1:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2020, 1:19 AM
RKSimon accepted this revision.Jun 5 2020, 1:40 AM

LGTM with a few style comments - they were already present, but probably worth handling in the refactoring.

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
855

auto *SI = cast<StoreInst>(U);

862

auto *LI = cast<LoadInst>(U);

llvm/lib/Target/X86/X86TargetTransformInfo.cpp
2966

if (isa_and_nonnull<StoreInst>(I)) {

2970

auto *GEP

This revision is now accepted and ready to land.Jun 5 2020, 1:40 AM
This revision was automatically updated to reflect the committed changes.

Hello,

It seems that commit is breaking some of our PPC buildbots (e.g. http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/4118)
Could you please fix it asap or revert ?

Thanks.

sbc100 added a subscriber: sbc100.Jun 5 2020, 3:24 PM

We are also seeing a crash on the wasm waterfall the bisects to this change:

https://ci.chromium.org/p/wasm/builders/ci/linux/25463

Looks like MVT::getVT getting called with StructTyID (which is can't handle).

Thanks both for letting me know, I've now committed a hopeful fix.