[PATCH] D38486: [PPC] Implement the heuristic to choose between a X-Form VSX ld/st vs a X-Form FP ld/st.

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 1 09:20:52 PDT 2017


nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

The remaining comments I have are minor nits. Please feel free to fix them on the commit. Otherwise, LGTM.



================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:134
 
+    let isPseudo = 1 in {
+      // Pseudo instruction XFLOADf64 will be expanded to LXSDX or LFDX later
----------------
Style nits (of course, this applies not only here but everywhere below as well):
1. No braces when the `let` expression applies to a single def
2. Just enclose the new def into a block if the same `let` expression applies to both (i.e. `let CodeSize = 3` applies to both `LXSDX` and `XFLOADf64` here. Just enclose the two into a block like you enclosed them into the `mayLoad/mayStore` block.
3. I find it surprising that we need `let isPseudo = 1` on definitions that use `Pseudo<>`. Are you sure that's needed? Isn't it part of the class definition for `Pseudo<>`?


================
Comment at: test/CodeGen/PowerPC/VSX-XForm-Scalars.ll:12
+
+; CHECK-P8:           lxsiwax 34, 0, 3
+; CHECK-P8-NEXT:      xxspltw 34, 34, 1
----------------
So we don't get this on Power9?


https://reviews.llvm.org/D38486





More information about the llvm-commits mailing list