[PATCH] D83338: [PowerPC][Power10] Implemented Vector Shift Builtins

Amy Kwan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 17 14:27:25 PDT 2020


amyk added a comment.

Please also change the function names.



================
Comment at: clang/lib/Headers/altivec.h:17151
+
+/* vs[l | r | raq] */
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
----------------
Actually, sorry, I think this comment should be the following instead:
`/* vec_s[l | r | ra] */` since these functions are actually supposed to be `vec_sl`, `vec_sr`, `vec_sra`.


================
Comment at: clang/lib/Headers/altivec.h:17158
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sl(vector signed __int128 __a, vector unsigned __int128 __b) {
+  return __builtin_altivec_vslq((vector unsigned __int128) __a, __b);
----------------
stefanp wrote:
> nit:
> Is this supposed to be `vec_slq`?
Actually `vec_sl` seems to be correct. However, that would mean the other functions need to be renamed as the functions are supposed to be: `vec_sl`, `vec_sr`, `vec_sra`. Albion, could you please rename these functions and also ensure your test uses the correct naming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338





More information about the llvm-commits mailing list