stefanp added a comment. The title of the patch mentions both zero extend and sign extend. However, it seems that we only have instructions for the zero extend case. Is that right? I see both types of tests in: `test/CodeGen/builtins-ppc-p10vector.c` But I only see codegen tests for the zreo extend version. `test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll`
I bring it up because I want to make sure that this is intentional. ================ Comment at: clang/lib/Headers/altivec.h:16516 + +static __inline__ vector unsigned __int128 __ATTRS_o_ai +vec_xl_sext(signed long long __offset, signed char *__pointer) { ---------------- I'm a little confused about this. Your function signature says we return `vector unsigned __int128` but the return statement casts to `unaligned_vec_si128`. Is that how this is supposed to look? ================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14163 + if (!ValidLDType || (LD->getValueType(0) != MVT::i128) || + (LD->getExtensionType() != ISD::ZEXTLOAD)) + return SDValue(); ---------------- It looks like you are doing this for zero extend only. What about `ISD::EXTLOAD`? In that case the upper bits are undefined anyway so we can just assume a zero extend right? ================ Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll:41 + +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; These test cases tests that zero extending loads utilize the Load VSX Vector Rightmost ---------------- nit: Was this line auto-added? Usually this comment is added at the top of the file by the script. I don't think it is required here. 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