Page MenuHomePhabricator

[PowerPC] Cust lower fpext v2f32 to v2f64 from extract_subvector v4f32
Needs RevisionPublic

Authored by lei on May 15 2019, 1:36 PM.

Details

Summary

This is a follow up patch from https://reviews.llvm.org/D57857 to handle extract_subvector v4f32.

For cases where we fpext of v2f32 to v2f64 from extract_subvector we currently generate on P9 the following:

lxv 0, 0(3)
xxsldwi 1, 0, 0, 1
xscvspdpn 2, 0
xxsldwi 3, 0, 0, 3
xxswapd 0, 0
xscvspdpn 1, 1
xscvspdpn 3, 3
xscvspdpn 0, 0
xxmrghd 0, 0, 3
xxmrghd 1, 2, 1
stxv 0, 0(4)
stxv 1, 0(5)

This patch custom lower it to the following sequence:

        lxv 0, 0(3)       # load the v4f32 <w0, w1, w2, w3>
	xxmrghw 2, 0, 0   # Produce the following vector <w0, w0, w1, w1>
        xxmrglw 3, 0, 0   # Produce the following vector <w2, w2, w3, w3>
        xvcvspdp 2, 2     # FP-extend to <d0, d1>
	xvcvspdp 3, 3     # FP-extend to <d2, d3>
        stxv 2, 0(5)      # Store <d0, d1> (%vecinit11)
        stxv 3, 0(4)      # Store <d2, d3> (%vecinit4)

Diff Detail

Event Timeline

lei created this revision.May 15 2019, 1:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2019, 1:36 PM
lei updated this revision to Diff 199674.May 15 2019, 1:46 PM
nemanjai requested changes to this revision.Jul 5 2019, 11:13 AM

A few minor nits and a functional problem that needs to be addressed (exiting if the input is not a v4f32).

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
9884

Nit: spaces around the ==.

9887

I think you mean doubleword here.

9889

Nit: the return SDValue() should be on the line under the if.
We also need a check here that the input to the EXTRACT_SUBVECTOR is actually of type v4f32, otherwise this doesn't work.
If you add a test case much like the one you added here, but change all instances of
<4 x float> to <16 x float>. This will not do the right thing.

9890

Nit: spaces around :. Also, this is really just Idx >> 1 isn't it?

9893

Same nit about the code being on the line under the if. And also, not word, but doubleword.

This revision now requires changes to proceed.Jul 5 2019, 11:13 AM
lei updated this revision to Diff 211160.Jul 22 2019, 11:49 AM
lei marked 5 inline comments as done.

Address review comments.

nemanjai requested changes to this revision.Wed, Aug 21, 12:02 AM
nemanjai added inline comments.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
9884

I don't think this assert does what you want it to.
This makes sure that one of the following is true:

  • There are two operands
  • The second operand is a constant
  • The first operand does not have type v4f32

And what you want is that all of those are true (except the last one needs to be flipped):

assert(Op0.getNumOperands() == 2 &&
       isa<ConstantSDNode(Op0.getOperand(1) &&
       Op0.getOperand(0).getValueType == MVT::v4f32 &&
       "Input must be MVT::v4f32 and operand 2 must be a constant!");

But in any case, I think we should not assert that last one - it can be false without anything being broken (i.e. pre-legalize). This should be an early exit here:

if (Op0.getOperand(0).getValueType() != MVT::v4f32)
  return SDValue();
9895

s/double word/doubleword

llvm/test/CodeGen/PowerPC/reduce_scalarization02.ll
11

Please add an additional test case where the input type is simply changed from <4 x float> to <16 x float>.

This revision now requires changes to proceed.Wed, Aug 21, 12:02 AM