This is an archive of the discontinued LLVM Phabricator instance.

update add\sub costs of vectors of 64 in X86\SLM arch
ClosedPublic

Authored by magabari on Jun 7 2017, 4:58 AM.

Details

Summary

this patch updates the cost of addq\subq (add\subtract of vectors of 64bits)
based on the performance numbers of SLM arch.

Diff Detail

Repository
rL LLVM

Event Timeline

magabari created this revision.Jun 7 2017, 4:58 AM
RKSimon edited edge metadata.Jun 7 2017, 5:51 AM
RKSimon added a subscriber: llvm-commits.

Please don't forget to include llvm-commits as a subscriber to all llvm patches

You might want to add a slm target to the llvm\test\Transforms\SLPVectorizer\X86\arith-add.ll, arith-mul.ll and arith-sub.ll tests as well.

You might want to add a slm target to the llvm\test\Transforms\SLPVectorizer\X86\arith-add.ll, arith-mul.ll and arith-sub.ll tests as well.

You mean to split the slm-arith-costs.ll test to those files or to add extra test cases via checking those tests on SLM also?

You might want to add a slm target to the llvm\test\Transforms\SLPVectorizer\X86\arith-add.ll, arith-mul.ll and arith-sub.ll tests as well.

You mean to split the slm-arith-costs.ll test to those files or to add extra test cases via checking those tests on SLM also?

I mean adding an extra slm specific 'RUN' pass to each file:

; RUN: opt < %s -mtriple=x86_64-unknown -mcpu=slm -basicaa -slp-vectorizer -S | FileCheck %s --check-prefix=CHECK --check-prefix=SLM

I've added the SLP vectorization tests at rL305151

I've added the SLP vectorization tests at rL305151

Simon I saw that you have added that to the tests.
Is there is a need to do something else?

I've added the SLP vectorization tests at rL305151

Simon I saw that you have added that to the tests.
Is there is a need to do something else?

Did the tests change at all with your cost model changes?

I've added the SLP vectorization tests at rL305151

Simon I saw that you have added that to the tests.
Is there is a need to do something else?

Did the tests change at all with your cost model changes?

I run make check with your change and it pass.

efriedma added inline comments.
test/Transforms/LoopVectorize/X86/slm-no-vectorize.ll
1 ↗(On Diff #101694)

The vectorizer is clearly doing something wrong if it's generating a 64-bit multiply for this loop... might want to clarify the comment.

fixed test comment

test/Transforms/LoopVectorize/X86/slm-no-vectorize.ll
1 ↗(On Diff #101694)

fixed

magabari updated this revision to Diff 102643.Jun 15 2017, 1:46 AM
RKSimon accepted this revision.Jun 21 2017, 5:21 AM

LGTM with one minor about the comment in the lv test

test/Transforms/LoopVectorize/X86/slm-no-vectorize.ll
5 ↗(On Diff #102643)

Very minor but it might be tidier if the comment is put after the RUN target lines.

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