saghir marked 3 inline comments as done. saghir added inline comments.
================ Comment at: clang/lib/Sema/DeclSpec.cpp:1155 + // TODO: Update comment with correct Programming Interface Manual + // version once it is available. __int128 has also been added + // to vector bool for Power10. ---------------- lei wrote: > saghir wrote: > > lei wrote: > > > Not sure what you mean here. > > Earlier comment had the Programming Interface Manual version number (PIM > > 2.1); asserting only char/int were valid with vector bool. > > This patch adds __int128, so the comment needs to be updated with the > > latest version of the document once it is available. > Maybe need to add a TODO in all the sections of code, that you update, where > it mentions PIM 2.1 I have changed comment such that it does not need the document version. ================ Comment at: clang/test/Parser/altivec-bool-128.c:2 +// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify %s + ---------------- lei wrote: > saghir wrote: > > lei wrote: > > > test for `-mcpu=pwr10 -target-feature -power10-vector` and `-mcpu=pwr10 > > > -target-feature -vsx` > > > same for cxx-altivec-bool-128.cpp > > This test basically checks that `VSX` needs to be enabled to have `vector > > bool __int128` type work. > Yes. I am asking you to add a test to check that `power10-vector` is also > needed to be enabled for this type to work. > eg. this type should not be enabled for `-mcpu=pwr10 -target-feature +vsx > -target-feature -power10-vector` Done, added a line for `pwr10`, `+vsx` and `-power10-vector`. ================ 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 ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81816/new/ https://reviews.llvm.org/D81816 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
