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

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

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

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
6

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.