This is an archive of the discontinued LLVM Phabricator instance.

Implementation of PPC lowering for vp_load/vp_store with no mask
Needs ReviewPublic

Authored by hussainjk on Sep 7 2021, 10:36 AM.

Details

Summary

On PowerPC, lower vp_load and vp_store nodes with %evl but no %mask operands to lxvl and stxvl instructions. Depends on D109377.

Diff Detail

Event Timeline

hussainjk created this revision.Sep 7 2021, 10:36 AM
hussainjk requested review of this revision.Sep 7 2021, 10:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 7 2021, 10:36 AM
bmahjour added inline comments.Sep 8 2021, 1:51 PM
llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
1339–1384

in what patch is this function first introduced? I don't see it in main or any of the other recent patches you posted.

hussainjk updated this revision to Diff 371446.Sep 8 2021, 2:25 PM

Fixing some arc errors when making the patch.

hussainjk updated this revision to Diff 371453.Sep 8 2021, 2:48 PM

Putting back accidentally-deleted test file.

hussainjk updated this revision to Diff 371792.Sep 9 2021, 11:31 PM

Expanded LIT tests with a lot more cases, including vector splitting.

nemanjai added inline comments.Sep 10 2021, 2:45 AM
llvm/lib/Target/PowerPC/PPCInstrVSX.td
2437

Why are these here? These nodes are target independent so it definitely seems like the wrong thing to do to define them in a PPC-specific file.

llvm/test/CodeGen/PowerPC/ldst-with-length-vector.ll
2

This needs to have a big endian line as well and also a -O0 for each to ensure that it works with FastISEL.

nemanjai added inline comments.Sep 10 2021, 10:57 AM
llvm/test/CodeGen/PowerPC/ldst-with-length-vector.ll
7

I must be missing something here. I thought the %evl parameter is the explicit vector length but it is clearly getting ignored and we are shifting a constant. So I don't really follow what is happening here.

nemanjai added inline comments.Sep 10 2021, 11:03 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15242

The semantics here are very different from what I assumed. I assumed one can load 5 byte elements of a v16i8 vector by using the v16i8 type and a length of 5. But we have to use a different type (i.e. v5i8)?

bmahjour added inline comments.Sep 10 2021, 12:56 PM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15249

should this be NewOps[3] instead?

15276

5 seems out or range. Shouldn't it be NewOps[4] = ShiftedLength; ?

hussainjk updated this revision to Diff 372051.Sep 10 2021, 8:13 PM

Fixed tests incorrectly using zeroinitializer instead of undef for %evl, causing it to be ignored.

RolandF added inline comments.
llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
1339–1384

This is a target specialization of the default function from https://reviews.llvm.org/D78203.