This is an archive of the discontinued LLVM Phabricator instance.

[TTI] Remove default condition type and predicate arguments from getCmpSelInstrCost
ClosedPublic

Authored by RKSimon on Oct 3 2021, 7:44 AM.

Details

Summary

We need to be better at exposing the comparison predicate to getCmpSelInstrCost calls as some targets (e.g. X86 SSE) have very different costs for different comparisons (PR48337), and we can't always rely on the optional Instruction argument.

This initial commit requires explicit condition type and predicate arguments. The next step will be to review a lot of the existing getCmpSelInstrCost calls which have used BAD_ICMP_PREDICATE even when the predicate is known.

Diff Detail

Unit TestsFailed

Event Timeline

RKSimon created this revision.Oct 3 2021, 7:44 AM
RKSimon requested review of this revision.Oct 3 2021, 7:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2021, 7:44 AM
fhahn added a comment.Oct 5 2021, 1:20 AM

Removing the default argument is great, thanks! As @ABataev said, it would be good to have a test case. Do you think it's possible to construct one or are more changes needed to show a test difference?

Yes I'd like to add test coverage for this - we're in the weird position that we don't have good checking in the cost models for different predicates, but its difficult to test this for without feeding accurate predicates to the cost models....

The X86 model is getting better at handling costs for different predicates - what I might do is make its costs for BAD_ICMP_PREDICATE/BAD_FCMP_PREDICATE have a much higher/worstcase default cost and work back from there.

This revision is now accepted and ready to land.Oct 6 2021, 6:51 AM
This revision was landed with ongoing or failed builds.Oct 6 2021, 7:41 AM
This revision was automatically updated to reflect the committed changes.