[PATCH] D81816: [PowerPC] Add support for vector bool __int128 for Power10

[Lei Huang via Phabricator via cfe-commits]

lei added inline comments.


================
Comment at: clang/test/Parser/p10-vector-bool-128.c:2
+// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -target-feature +altivec -target-feature +vsx -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-feature +altivec -target-feature +vsx -fsyntax-only -verify %s
+// expected-no-diagnostics
----------------
saghir wrote:
> lei wrote:
> > saghir wrote:
> > > lei wrote:
> > > > add run line for feature `cpu=pwr10 +power10-vector`
> > > Added `pwr10`.
> > > `vector bool __int128` type should work with `pwr10` and `vsx` enabled, `power10-vector` is not needed explicitly.
> > `cpu=pwr10 -vsx +power10-vector`
> I have now added both `-target-cpu pwr10` and `-target-feature +power10-vector` as you mentioned in the first comment. 
> However, I am not quite sure what you are looking to test here by adding `-vsx` because that would disable `vsx`, which in turn would disable `power10-vector` and we would not be able to test the legitimate use here.
`-vsx` diable it, but the following `+power10-vector` should enable it.
So:
```
// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-cpu pwr10 \
// RUN:            -target-feature -vsx -target-feature +power10-vector
```


================
Comment at: clang/test/Parser/p10-vector-bool-128.c:2
+// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -target-cpu pwr10 \
+// RUN:            -target-feature +altivec -target-feature +vsx \
+// RUN:            -target-feature +power10-vector -fsyntax-only -verify %s
----------------
I don't think you need `-target-feature +altivec ` here.


================
Comment at: clang/test/Parser/p10-vector-bool-128.c:5
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-cpu pwr10 \
+// RUN:            -target-feature +altivec -target-feature +vsx \
+// RUN:            -target-feature +power10-vector -fsyntax-only -verify %s
----------------
this line shouldn't be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81816