[PATCH] D44528: [PowerPC] Implement canCombineStoreAndExtract and provide the missing codegen patterns

[Jinsong Ji via Phabricator via llvm-commits]

jsji added a comment.

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.



================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:3557
               (f64 (PPCcv_fp_to_sint_in_vsr f64:$src)), ixaddr:$dst, 8),
-            (STXSD (XSCVDPSXDS f64:$src), ixaddr:$dst)>;
+            (DFSTOREf64 (XSCVDPSXDS f64:$src), ixaddr:$dst)>;
   def : Pat<(PPCstore_scal_int_from_vsr
----------------
This looks like not related to "extract+store" exploitation? Maybe we should do it in another patch?


================
Comment at: test/CodeGen/PowerPC/combine-extract-store.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=pwr8 -mtriple=powerpc64le-unkknown-unknown \
----------------
I would be better if we can split this new testcase into new patch, then just show diff due to this change here. 


================
Comment at: test/CodeGen/PowerPC/combine-extract-store.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=pwr8 -mtriple=powerpc64le-unkknown-unknown \
+; RUN:   -verify-machineinstrs -O2 < %s | FileCheck %s
----------------
How about pwr9? Any change due to this patch?


================
Comment at: test/CodeGen/PowerPC/extract-and-store.ll:69
+; CHECK-NEXT:    addi r3, r7, 24
+; CHECK-NEXT:    stxsdx f0, 0, r3
 ; CHECK-NEXT:    blr
----------------
This looks not better than using stfd ? Can we avoid combining this case?


================
Comment at: test/CodeGen/PowerPC/extract-and-store.ll:122
+; CHECK-NEXT:    addi r3, r7, 12
+; CHECK-NEXT:    stfiwx f0, 0, r3
 ; CHECK-NEXT:    blr
----------------
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?


================
Comment at: test/CodeGen/PowerPC/extract-and-store.ll:157
+; CHECK-BE-NEXT:    addi r3, r7, 12
+; CHECK-BE-NEXT:    stxsiwx vs34, 0, r3
 ; CHECK-BE-NEXT:    blr
----------------
Similar to above, are we sure `stxsiwx` will store the save value as `stfs` for all single precision values?


================
Comment at: test/CodeGen/PowerPC/store_fptoi.ll:21
 ; CHECK: xscvdpsxds [[CONV:[0-9]+]], [[LD]]
-; CHECK-NEXT: stxsd [[CONV]], 0(4)
+; CHECK-NEXT: stfd [[CONV]], 0(4)
 ; CHECK-NEXT: blr
----------------
Looks like not related to "extract+store" exploitation? Maybe we should do it in another patch?


Repository:
  rL LLVM

https://reviews.llvm.org/D44528