This is an archive of the discontinued LLVM Phabricator instance.

[X86] Try to improve min/max reduction costs.
ClosedPublic

Authored by craig.topper on Mar 23 2020, 11:31 AM.

Details

Summary

This is similar to what I recently did for getArithmeticReductionCost.

I'm trying to account for the narrowing from 512->256->128 as we go.

I've also added a new helper method getMinMaxCost that tries to
handle the cases where we have native min/max instructions and
fall back to cmp+select when we don't.

This change has both increases and decreases on the costs. Please
point out any changes you think are needed.
I'm not very convinced by the numbers in the tables for some cases,
but I'd like to address those as a follow up so they don't get
lost in the diff.

Diff Detail

Event Timeline

craig.topper created this revision.Mar 23 2020, 11:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2020, 11:31 AM
Herald added a subscriber: hiraditya. · View Herald Transcript

The PHMINPOS vXi8/vXi16 lowering means we have different min vs max reduction costs - should we be trying to account for this?

llvm/test/Analysis/CostModel/X86/reduce-smax.ll
47–48

I realise its not a regression but still really bad.

95

No improvement?

Rebase and fix a bug.

craig.topper planned changes to this revision.Mar 29 2020, 12:21 AM

I"m working on scrubbing the tables and will post a new version when that is done.

Updated tables to more reasonable values.

RKSimon accepted this revision.Apr 9 2020, 2:26 PM

LGTM - cheers

llvm/test/Transforms/SLPVectorizer/X86/horizontal-minmax.ll
4 ↗(On Diff #253680)

Please can you add a AVX common prefix for AVX1/AVX2?

This revision is now accepted and ready to land.Apr 9 2020, 2:26 PM
This revision was automatically updated to reflect the committed changes.