nemanjai added a comment.

In D106757#2902849 <https://reviews.llvm.org/D106757#2902849>, @Conanap wrote:

> do we need an IR -> ASM test case as well?

I didn't add this as the builtins do not produce any new IR - there are no new 
intrinsics or any other modifications to the back end. Since the code is 
completely contained in the front end, I feel like the tests should be as well.



================
Comment at: clang/lib/Headers/altivec.h:3151
+#else
+#define __vec_ldrmb __builtin_vsx_ldrmb
+#define __vec_strmb __builtin_vsx_strmb
----------------
Conanap wrote:
> I believe the preference is to have this defined in 
> `clang/lib/Basic/Targets/PPC.cpp` under `defineXLCompatMacros`
That is a very good point. However for the Altivec builtins, there is long 
standing precedent for having these defined in the header. We actually don't 
want the vector builtins to be defined without the inclusion of `altivec.h`.


================
Comment at: clang/test/CodeGen/builtins-ppc-ld-st-rmb.c:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: powerpc-registered-target
----------------
The test case is quite verbose but the checks were produced by the script so it 
should be easy to maintain. The reason I added so many checks is that the 
produce code is very dependent on:
- endianness
- CPU
- the number of bytes (in the store case)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106757

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

Reply via email to