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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to