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

[Stefan Pintilie via Phabricator via cfe-commits]

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