Page MenuHomePhabricator

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

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

Details

Reviewers
RKSimon
spatel
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

Unit TestsFailed

TimeTest
20 msLLVM.Transforms/InstCombine::ExtractCast.ll
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/opt < /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Transforms/InstCombine/ExtractCast.ll -instcombine -S -o - | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/FileCheck /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Transforms/InstCombine/ExtractCast.ll

Event Timeline

craig.topper created this revision.Mon, Mar 23, 11:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Mar 23, 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
54–55

I realise its not a regression but still really bad.

102–103

No improvement?

Rebase and fix a bug.

craig.topper planned changes to this revision.Sun, Mar 29, 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.