pengfei added a comment.

> If it exists it must be tested.
> Every piece of code generation needs to be tested.

Let me show you the number:

  $ grep -rho '__builtin_ia32\w\+' clang/test/CodeGen | sort|uniq |wc -l
  337
  $ grep -rho '_mm512_\w\+' clang/test/CodeGen | sort|uniq |wc -l
  2304

Note most of `__builtin_ia32` tests are negative ones and `_mm512_` is less 
than 1/3 of the total x86 intrinsics. Two orders of magnitude!

I took a look at the tests. The positive tests come from 
clang/test/CodeGen/builtins-x86.c. Other targets don't have a big test either
`wc -l clang/test/CodeGen/builtins-*`

> These are not testing use of these builtins correctly

As I have explained, users are not suggested to use these builtins given we 
have provided the more stable, well documented corresponding intrinsics. The 
only case user has to use it is the intrinsic is missing. In that case, we do 
need test case for it.

In a word, as titled, it is NFC from the perspective of intrinsics. So I think 
we don't need test cases for them.

> This test amounts to a builtin header test for immintrin.h. This should move 
> to clang/test/Headers and replaced with a builtin test directly checking the 
> builtin handling

Not get your point. But currently no builtin tests under `clang/test/Headers/`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140467

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

Reply via email to