This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC][Power10] Exploit store rightmost vector element instructions.
ClosedPublic

Authored by Conanap on Oct 10 2020, 11:40 AM.

Details

Summary

Using the store rightmost vector element instructions to do vector
element extraction and store. The rightmost vector element on little
endian is the zeroth vector element, with these patterns that element
can be extracted and stored in one instruction for all vector types.

Diff Detail

Event Timeline

kamaub created this revision.Oct 10 2020, 11:40 AM
kamaub requested review of this revision.Oct 10 2020, 11:40 AM
kamaub added a project: Restricted Project.Oct 10 2020, 11:44 AM
lei added inline comments.Oct 22 2020, 7:31 AM
llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
1

There's some redundancies here:

  1. The default CHECK are not used in any of the run lines, but are updated in some cases.
  2. The newly generated CHECK-LE and CHECK-BE are at times exact duplicate of each other and of the default CHECK.

Suggest to update thus so it's more clear what the changes are:

RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
; RUN:   FileCheck %s --check-prefix=CHECK,LE
; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
; RUN:   FileCheck %s --check-prefix=CHECK,BE

For cases where there is a differnce in LE and BE behaviour, remove the default CHECK and add new. Otherwise just leave the original default CHECKs.

llvm/test/CodeGen/PowerPC/store-rightmost-vector-elt.ll
5

run line for BE?

amyk requested changes to this revision.Dec 15 2020, 1:40 PM

Please address the comments regarding the test cases.

llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
1

I also agree with Lei's comment. We should add check prefixes and remove/add the CHECKs when its necessary.

llvm/test/CodeGen/PowerPC/store-rightmost-vector-elt.ll
5

Agree we should have a run line for BE.

This revision now requires changes to proceed.Dec 15 2020, 1:40 PM
Conanap commandeered this revision.Dec 16 2020, 10:58 AM
Conanap added a reviewer: kamaub.

Will take over patch from Kamau

Conanap updated this revision to Diff 312264.Dec 16 2020, 11:02 AM
Conanap marked 4 inline comments as done.

Updated tests as per Amy and Lei's comments

lei accepted this revision as: lei.Dec 17 2020, 1:22 PM

LGTM

amyk accepted this revision as: amyk.Dec 17 2020, 1:28 PM

LGTM. Thanks for the update Albion.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 22 2020, 9:07 AM
This revision was automatically updated to reflect the committed changes.
jsji added a subscriber: jsji.EditedDec 23 2020, 7:05 AM

LGTM. Thanks for the update Albion.

@amyk If you request changes on behalf of PowerPC, you should accept it on behalf of PowerPC as well.
Or else it will show This revision was not accepted when it landed; it landed in state Needs Review. Thanks.

nemanjai added inline comments.Dec 28 2020, 7:27 AM
llvm/lib/Target/PowerPC/PPCInstrPrefix.td
2564

This COPY_TO_REGCLASS and below are redundant and could cause issues in the future. Fixed in e73f885c988d.