[PATCH] D106757: [PowerPC] Implement partial vector ld/st builtins for XL compatibility

Sun Jul 25 07:23:10 PDT 2021

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