This is an archive of the discontinued LLVM Phabricator instance.

[CostModel][X86] Support cost kind specific look up tables
ClosedPublic

Authored by RKSimon on Aug 19 2022, 3:18 AM.

Details

Summary

Most of our cost model tables have been created assuming cost kind == recip-throughput. But we're starting to see passes wanting to get accurate costs for the other kinds as well. Some of these can be determined procedurally (e.g. codesize by default could just be the split count after type legalization), but others are going to need to be handled in cost tables - this is especially true for x86 which has so many ISA combinations.

I've created a 'CostKindCosts' struct which can hold cost values for the 4 cost kinds, defaulting to -1U for unknown cost, this can be used with the existing CostTblEntryT/CostTableLookup template code. I've also added a [TargetCostKind] accessor to make it much easier to look up individual costs.

This just changes the ISD::SELECT costs to check the effect (and also to check that the ISD::SETCC are correctly handled for default/None cost kinds) - the plan would be to slowly extend this and move the CostKindTblEntry type somewhere generic to allow other targets to use it once its matured.

I'm also going to resurrect D103695 so that it can help with latency/codesize/sizelatency coverage testing.

For sizelatency - IIRC the definition was vague to let it be target specific - I've tried to use typical uop counts so they're comparable to MicroOpBufferSize etc.

Diff Detail

Event Timeline

RKSimon created this revision.Aug 19 2022, 3:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2022, 3:18 AM
RKSimon requested review of this revision.Aug 19 2022, 3:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2022, 3:18 AM
RKSimon updated this revision to Diff 454046.Aug 19 2022, 9:29 AM
RKSimon edited the summary of this revision. (Show Details)

consistently use #uop count for sizelatency

pengfei added inline comments.Aug 19 2022, 5:48 PM
llvm/lib/Target/X86/X86TargetTransformInfo.cpp
2727

Do we have a place to introduce the meaning of each elements, where they are used and how to get the correct value?
Besides, the previous comments are just for throughput? How can we explain the other values?

RKSimon added inline comments.Aug 20 2022, 1:22 AM
llvm/lib/Target/X86/X86TargetTransformInfo.cpp
2727

I'll tweak the description at the top of the file

RKSimon updated this revision to Diff 454187.Aug 20 2022, 2:55 AM

Update the description of the cost mdel values for each cost kind.

pengfei added inline comments.Aug 20 2022, 4:02 AM
llvm/lib/Target/X86/X86TargetTransformInfo.cpp
2727

The description looks great. But I still don't understand the difference between SETCC and SELECT. Why are other cost kinds zero for SETCC?

RKSimon added inline comments.Aug 20 2022, 4:18 AM
llvm/lib/Target/X86/X86TargetTransformInfo.cpp
2727

They aren't - they should be set to the "invalid" ~0U value by the CostKindCosts constructor (assuming I've gotten the c++ initialization orders right in my brain...).

I can make that explicit if you prefer: CostKindCosts(4,4,1,3), but I was trying to avoid making it too busy, or I can add default initialization values to the CostKindCosts members?

pengfei added inline comments.Aug 20 2022, 5:18 AM
llvm/lib/Target/X86/X86TargetTransformInfo.cpp
2727

Oh, yes. Sorry I mistook for arrays... It's clear to me now :)

RKSimon retitled this revision from [CostModel][X86] RFC - Support cost kind specific look up tables to [CostModel][X86] Support cost kind specific look up tables.Aug 23 2022, 12:19 PM
RKSimon edited the summary of this revision. (Show Details)
pengfei accepted this revision.Aug 25 2022, 2:12 AM

LGTM.

This revision is now accepted and ready to land.Aug 25 2022, 2:12 AM
This revision was landed with ongoing or failed builds.Aug 25 2022, 4:34 AM
This revision was automatically updated to reflect the committed changes.

Reverted this in ab85996e475ceddfda82255c314229ac0c0f4994, it crashes clang during selfhost.

OK - I'll take a look

Based off the stack dump from rGab85996e475c - my guess is that llvm::sinkRegion() is requesting instruction costs for an instruction with an "exotic" TypeID not supported by MVT::getVT - Function, Tokens or Metadata most likely.