[PATCH] D63636: [PowerPC][Altivec] Fix offsets for vec_xl and vec_xst

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai marked 3 inline comments as done.
nemanjai added inline comments.


================
Comment at: lib/Headers/altivec.h:16364
                                                       signed short *__ptr) {
-  return *(unaligned_vec_sshort *)(__ptr + __offset);
+  signed char *Adjusted = (signed char *)__ptr + __offset;
+  return *(unaligned_vec_sshort *)((signed short *)Adjusted);
----------------
jsji wrote:
> Why we name it `Adjusted`?  Why not just `__addr`? 
Sure. I don't really have any preference with respect to the name at all.


================
Comment at: lib/Headers/altivec.h:16365
+  signed char *Adjusted = (signed char *)__ptr + __offset;
+  return *(unaligned_vec_sshort *)((signed short *)Adjusted);
 }
----------------
jsji wrote:
> Why we want to cast it to `(signed short *)` again? Looks like unnecessary casting to me?
Argh, yup the double cast is silly. I initially did something different for this and just missed cleaning up these. I'll update.


================
Comment at: test/CodeGen/builtins-ppc-xl-xst.c:4
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx -target-feature +power8-vector -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-P8
+#include <altivec.h>
----------------
jsji wrote:
> Any difference for results without `power8-vector `, except for `test9` and `test10`?
> 
> Why not split `test9` and `test10` to another file for simplicity?
I like running all of them both with and without power8-vector. I can simplify this by using `check-prefixes=CHECK,CHECK-P8` so that we only have one sequence of checks for each function.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63636