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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits