[PATCH] D82502: [PowerPC][Power10] Implement Load VSX Vector and Sign Extend and Zero Extend

[Amy Kwan via Phabricator via cfe-commits]

amyk requested changes to this revision.
amyk added a comment.
This revision now requires changes to proceed.

I have a few comments from the last time we looked at this together. 
Just also FYI that the backend tests will be apart of the `p10-vsx-builtins.ll` file introduced in https://reviews.llvm.org/D82431.



================
Comment at: clang/lib/Headers/altivec.h:16514
+
+/* vect_xl_sext */
+static __inline__ vector signed __int128 __ATTRS_o_ai
----------------
Nit: space after this comment.


================
Comment at: clang/lib/Headers/altivec.h:16517
+vec_xl_sext(signed long long __offset, signed char *__pointer) {
+  return (vector signed __int128)*(__pointer + __offset);
+}
----------------
I realized that I think we are supposed to use `unaligned_vec_si128` and `unaligned_vec_ui128` instead of `vector signed __int128` and `vector unsigned __int128` when we return (for here, and other places).


================
Comment at: clang/lib/Headers/altivec.h:16535
+
+/* vec_xl_zext */
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
----------------
Nit: space after this comment.


================
Comment at: clang/lib/Headers/altivec.h:16879
 }
+
 #endif /* __POWER10_VECTOR__ */
----------------
Remove unrelated whitespace change.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13767
+
+  // Look for the pattern of a load from a narrow width to i128, feeding
+  // into a BUILD_VECTOR of v1i128. Replace this sequence with a PPCISD node
----------------
Move this comment above the combine function since this comment was intended to describe the function.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13774
+  // Other return types are not valid for the LXVRZX replacement.
+  if (N->getValueType(0) != MVT::v1i128) {
+    return SDValue();
----------------
Can omit the `{ }` since we just have a single return underneath.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13781
+  // is a load instruction.
+  if (Operand.getOpcode() != ISD::LOAD) {
+    return SDValue();
----------------
Can omit the `{ }` since we just have a single return underneath.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:583
+
+  // Special isntructions for PPC10 load vsx vector with zero extend
+  // Utilize the appropriate Load VSX Vector Rightmost instruction depending
----------------
Can remove this comment.


================
Comment at: llvm/test/CodeGen/PowerPC/p10-vsx-builtins.ll:6
+
+; ZEXT TEST CASES
+
----------------
Please update this comment to be more descriptive. Maybe something like, 
```
These test cases tests that zero extending loads utilize the Load VSX Vector Rightmost (lxvr[b|h|w|d]) instructions in Power10.
```


================
Comment at: llvm/test/CodeGen/PowerPC/p10-vsx-builtins.ll:8
+
+; i8
+; CHECK: lxvrbx
----------------
Please remove the comments/checks above the functions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82502/new/

https://reviews.llvm.org/D82502