Page MenuHomePhabricator

[PowerPC] Handle Vector Sum Reduction
Needs ReviewPublic

Authored by stefanp on Nov 26 2021, 6:58 AM.

Details

Reviewers
nemanjai
lei
Group Reviewers
Restricted Project
Summary

Better codegen is possible for Vector Sum Reduction on Power 9 and up.
This patch adds handling for several vector sum reductions depending
on the type of the vector.

Diff Detail

Unit TestsFailed

TimeTest
90 msx64 debian > LLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llvm-go go=/usr/bin/go test llvm.org/llvm/bindings/go/llvm

Event Timeline

stefanp created this revision.Nov 26 2021, 6:58 AM
stefanp requested review of this revision.Nov 26 2021, 6:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 26 2021, 6:58 AM
stefanp added a reviewer: Restricted Project.Nov 26 2021, 6:59 AM
nemanjai added inline comments.Nov 26 2021, 10:23 AM
llvm/lib/Target/PowerPC/PPCMIPeephole.cpp
1067 ↗(On Diff #390045)

This is a more general and orthogonal peephole. Can you please separate this out to another patch and make it more general? This patch can then depend on that one.

1072 ↗(On Diff #390045)

Please make this more general by implementing a separate function called something like isVectorSymmetrical() that will just check whether the two halves of the vector are the same. Then this can be flipped to an early exit condition.

P.S. Keep in mind that it is possible that DefVecReg == nullptr which would cause a crash when you call DefVecReg->getOpcode().

stefanp updated this revision to Diff 390103.Nov 26 2021, 11:55 AM

Split out the peephole optimization that would replace MFVSRLD with MFVSRD
when possible.

That part of the patch will be a new patch soon.

amyk added a subscriber: amyk.Dec 3 2021, 1:26 PM
amyk added inline comments.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
10757

It looks like this portion:

if (VecInput.getOpcode() != ISD::SIGN_EXTEND &&
    VecInput.getOpcode() != ISD::ZERO_EXTEND)
  return SDValue();

// Check that we are extending from v16i8 to v16i32.
if (VecInput.getOperand(0).getSimpleValueType() != MVT::v16i8)
  return SDValue();

is common in both cases. Does it make sense to pull this part out of the case statements so we don't duplicate it?

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
93

I think it would be good to save the opcode first, and then use it in this condition.

amyk added a comment.Dec 3 2021, 1:49 PM

Also, I noticed that I think you may have meant to put [PowerPC] Handle Vector Sum Reduction instead of [PowerPC] Handle Vector Sum Reducation for the title. :-)

stefanp retitled this revision from [PowerPC] Handle Vector Sum Reducation to [PowerPC] Handle Vector Sum Reduction.Dec 16 2021, 7:54 AM
stefanp added inline comments.Dec 17 2021, 6:52 AM
llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
93

Sounds good. I will do that!

stefanp added inline comments.Dec 17 2021, 7:02 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
10757

There is actually more common code between the two case statements. I think I'm going to merge to two cases but I don't want to pull the if statements out of the switch. The way that I look at this is that anything that is outside the switch should apply to all of the possible types in the switch. Since I cannot guarantee that for those two early exists I will just leave them in the switch.

stefanp updated this revision to Diff 395130.Dec 17 2021, 7:33 AM

Merged some common code to make things easier to read.