[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

[Victor Huang via Phabricator via llvm-commits]

NeHuang added a comment.

Overall LGTM. I only have some nits comment.



================
Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:136
 
+vector unsigned char test_vexpandm_uc(void) {
+  // CHECK: @llvm.ppc.altivec.vexpandbm(<16 x i8> %{{.+}})
----------------
nit: can we change the naming convention to be consistent as above? (e.g.  "test_vec_expandm_uc") 

I do not have a strong preference for either way since we did have different naming conventions in `builtins-ppc-p10vector.c` and `p10-vector-mask-ops.ll` for the vector extract cases. 


================
Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:142
+
+vector unsigned short test_vexpandm_us(void) {
+  // CHECK: @llvm.ppc.altivec.vexpandhm(<8 x i16> %{{.+}})
----------------
ditto


================
Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:148
+
+vector unsigned int test_vexpandm_ui(void) {
+  // CHECK: @llvm.ppc.altivec.vexpandwm(<4 x i32> %{{.+}})
----------------
ditto


================
Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:154
+
+vector unsigned long long test_vexpandm_ull(void) {
+  // CHECK: @llvm.ppc.altivec.vexpanddm(<2 x i64> %{{.+}})
----------------
ditto


================
Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:160
+
+vector unsigned __int128 test_vexpandm_u128(void) {
+  // CHECK: @llvm.ppc.altivec.vexpandqm(<1 x i128> %{{.+}})
----------------
ditto


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:881
+                                     [(set v16i8:$vD, (int_ppc_altivec_vexpandbm
+					   v16i8:$vB))]>;
   def VEXPANDHM : VXForm_RD5_XO5_RS5<1602, 1, (outs vrrc:$vD), (ins vrrc:$vB),
----------------
amyk wrote:
> I have no idea why this indentation is off, since it did not appear like that previously. In any case, I can address this during the commit if it is OK.
This seems does not cause any clang-format check error during phabricator pre-merge check.  I tried downloading this patch and found the indentation is as expected. It seems like a display issue from phabricator. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727