[PATCH] D59924: [PowerPC][Clang] Port MMX intrinsics and basic test cases to Power

[Jinsong Ji via Phabricator via cfe-commits]

jsji requested changes to this revision.
jsji added a comment.
This revision now requires changes to proceed.

Since we are adding new headers that is not originated by this patch, 
please also update the description (and commit message) to include who actually contributed to `mmintrin.h` and other headers.
eg:headers are mostly based on headers developed by Steven Munroe, with some contribution of Paul Clarke, Bill Schmidt,  Jinsong, Zeson.



================
Comment at: clang/lib/Driver/ToolChains/PPCLinux.cpp:22
+  if (getArch() != llvm::Triple::ppc &&
+      !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+      !DriverArgs.hasArg(options::OPT_nobuiltininc)) {
----------------
Why we want to exclude these includes when `nostdinc` is on? These include files are NOT standard include files.


================
Comment at: clang/lib/Driver/ToolChains/PPCLinux.h:30
+} // end namespace toolchains
+} // namespace driver
+} // namespace clang
----------------
missing `end` in comment?


================
Comment at: clang/test/CodeGen/ppc-mmintrin.c:24
+// CHECK: [[REG1:[0-9a-zA-Z_%.]+]] = call <8 x i16> @vec_cmplt
+// CHECK: store <8 x i16> [[REG1]], <8 x i16>* [[REG2:[0-9a-zA-Z_%.]+]], align 16
+// CHECK: [[REG3:[0-9a-zA-Z_%.]+]] = call <16 x i8> @vec_packs
----------------
We should use `CHECK-NEXT` instead of `CHECK` here. This also apply to most of the following `CHECK`.


================
Comment at: clang/test/CodeGen/ppc-mmintrin.c:25
+// CHECK: store <8 x i16> [[REG1]], <8 x i16>* [[REG2:[0-9a-zA-Z_%.]+]], align 16
+// CHECK: [[REG3:[0-9a-zA-Z_%.]+]] = call <16 x i8> @vec_packs
+// CHECK: store <16 x i8> [[REG3]], <16 x i8>* [[REG4:[0-9a-zA-Z_%.]+]], align 16
----------------
Why we omit checking two `load` for `vec_packs` here?


================
Comment at: clang/test/CodeGen/ppc-mmintrin.c:36
+// CHECK: i64 @_mm_packs_pi16(i64 {{[0-9a-zA-Z_%.]+}}, i64 {{[0-9a-zA-Z_%.]+}})
+// CHECK: call <16 x i8> @vec_packs(short vector[8], short vector[8])
+
----------------
We should also check the `load` and make sure that the endian-specific code are working.


================
Comment at: clang/test/Headers/ppc-intrinsics.c:3
+
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -DTEST_MACRO -target powerpc64-gnu-linux %s -Xclang -verify -o - | FileCheck %s
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -target powerpc64-gnu-linux %s -Xclang -verify -x c++ -o - | FileCheck %s
----------------
Why `-DTEST_MACRO`?


================
Comment at: clang/test/Headers/ppc-intrinsics.c:7
+
+// RUN: NOT %clang -S -emit-llvm -target powerpc64-gnu-linux %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR
+
----------------
NOT? 
` NOT: command not found ^`


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924