[PowerPC] Implement canCombineStoreAndExtract and provide the missing codegen patterns

[nemanjai]

Summary

We have VSX store instructions that will store a single element from a vector without modifying it in any way. Previous generation cores can do this for word-sized elements and Power9 can also do it for half-word and byte-sized elements.
The TableGen patterns for the word-sized versions were missing - this patch adds them.

Furthermore, it provides the information about the cost of such a combine - zero cost when the index is the element number that the instruction stores, cost of 3 for other elements. The cost for the other elements is because a vector permute is needed to shift the element and it's only worthwhile if keeping the value as a vector reduces the cost enough to offset the cost of the permute.

[nemanjai]

Add the missing predicates on the patterns.

[lei]

test/CodeGen/PowerPC/combine-extract-store.ll

9

nit: line past col 80

12

Should these 2 lines be CHECK-DAG since they don't necessarily have to happen in this order?

[nemanjai]

Ping.

test/CodeGen/PowerPC/combine-extract-store.ll

9

I think we generally allow code in test cases to have lines however long clang produces them.

12

They certainly don't need to happen in this order, but I find that changing the CHECK directive produced by the tool makes the test case harder to maintain sometimes.

[nemanjai]

Implemented all the patterns for codegen of extract->store.

[steven.zhang]

lib/Target/PowerPC/PPCISelLowering.cpp

13959

I didn't take deep look at the implementation for this patch. The condition here seems not quite align with the comments. If the bitwidth is 32bit, we will combine the store and extract no matter if it is Power9 or not. I am not sure if this is by intention.

[jsji]

It is a good idea if we can CombineStoreAndExtract to save one instruction.
But looks like to me that it is not always safe and beneficial to do so, especially for floating points.

lib/Target/PowerPC/PPCInstrVSX.td

3073–3074

This looks like not related to "extract+store" exploitation? Maybe we should do it in another patch?

test/CodeGen/PowerPC/combine-extract-store.ll

2

I would be better if we can split this new testcase into new patch, then just show diff due to this change here.

3

How about pwr9? Any change due to this patch?

test/CodeGen/PowerPC/extract-and-store.ll

69 ↗

(On Diff #170392)

This looks not better than using stfd ? Can we avoid combining this case?

122 ↗

(On Diff #170392)

Are we sure that the semantic are equivalent here for stfs-> stfiwx?

With stfs: The contents of register FRS are converted to single format (see page 160) and stored into the word in storage addressed by EA.,

With stfiwx: (FRS)32:63 are stored, without conversion, into the word in storage addressed by EA.

So, is it safe to assume that there is not difference due to conversion?

157 ↗

(On Diff #170392)

Similar to above, are we sure stxsiwx will store the save value as stfs for all single precision values?

test/CodeGen/PowerPC/store_fptoi.ll

21 ↗

(On Diff #170392)

Looks like not related to "extract+store" exploitation? Maybe we should do it in another patch?

[nemanjai]

Upon closer inspection, this actually almost never fires on PPC so spending any more time on it does not seem useful. Abandoning this patch.

lib/Target/PowerPC/PPCISelLowering.cpp

13959

That is correct. And I feel that the comment does not suggest otherwise. Perhaps if I change the comment to something like:

// Prior to Power9, we only have an instruction that combines a store and extract
// for i32 (STXSIWX). ISA 3.0 (Power9) introduced instructions that do this for
// subword types (i8, i16). There is also no advantage to doing this for i64 on
// any subtarget.