[PATCH] D143210: [PowerPC] Include vector bool and pixel when emitting lax warning

2023-02-22 Thread Kamau Bridgeman via Phabricator via cfe-commits 
kamaub added a comment.

We may also need an associated test case for the changed behaviour for using 
`areCompatibleVectorTypes()` instead of `areSameVectorElemTypes()`.
The test coverage should display when warnings are emitted now that we account 
for vector bool, vector pixel and type qualifiers.




Comment at: clang/lib/Sema/SemaExpr.cpp:9861
   isLaxVectorConversion(RHSType, LHSType)) {
-if (VecType->getVectorKind() == VectorType::AltiVecVector)
+if (VecType->getVectorKind() == VectorType::AltiVecVector ||
+VecType->getVectorKind() == VectorType::AltiVecBool ||

Is there test coverage for these addition to the if condition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143210

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

[PATCH] D143210: [PowerPC] Include vector bool and pixel when emitting lax warning

2023-02-21 Thread Maryam Moghadas via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG02a71b05fc67: [PowerPC] Include vector bool and pixel when 
emitting lax warning (authored by maryammo).

Changed prior to commit:
  https://reviews.llvm.org/D143210?vs=494413=499296#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143210

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGen/SystemZ/zvector.c
  clang/test/CodeGen/SystemZ/zvector2.c

Index: clang/test/CodeGen/SystemZ/zvector2.c
===
--- clang/test/CodeGen/SystemZ/zvector2.c
+++ clang/test/CodeGen/SystemZ/zvector2.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z14 -fzvector \
-// RUN:  -O -emit-llvm -o - -W -Wall -Werror %s | FileCheck %s
+// RUN:  -O -emit-llvm -o - -W -Wall -Werror -Wno-error=deprecate-lax-vec-conv-all %s | FileCheck %s
 
 volatile vector float ff, ff2;
 volatile vector bool int bi;
Index: clang/test/CodeGen/SystemZ/zvector.c
===
--- clang/test/CodeGen/SystemZ/zvector.c
+++ clang/test/CodeGen/SystemZ/zvector.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -fzvector -emit-llvm -o - -W -Wall -Werror %s | opt -S -passes=mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -fzvector \
+// RUN: -emit-llvm -o - -W -Wall -Werror -Wno-error=deprecate-lax-vec-conv-all \
+// RUN: %s | opt -S -passes=mem2reg | FileCheck %s
 
 volatile vector signed char sc, sc2;
 volatile vector unsigned char uc, uc2;
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1770,7 +1770,7 @@
  !ToType->hasAttr(attr::ArmMveStrictPolymorphism))) {
   if (S.isLaxVectorConversion(FromType, ToType) &&
   S.anyAltivecTypes(FromType, ToType) &&
-  !S.areSameVectorElemTypes(FromType, ToType) &&
+  !S.Context.areCompatibleVectorTypes(FromType, ToType) &&
   !InOverloadResolution && !CStyle) {
 S.Diag(From->getBeginLoc(), diag::warn_deprecated_lax_vec_conv_all)
 << FromType << ToType;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7987,30 +7987,24 @@
  "expected at least one type to be a vector here");
 
   bool IsSrcTyAltivec =
-  SrcTy->isVectorType() && (SrcTy->castAs()->getVectorKind() ==
-VectorType::AltiVecVector);
+  SrcTy->isVectorType() && ((SrcTy->castAs()->getVectorKind() ==
+ VectorType::AltiVecVector) ||
+(SrcTy->castAs()->getVectorKind() ==
+ VectorType::AltiVecBool) ||
+(SrcTy->castAs()->getVectorKind() ==
+ VectorType::AltiVecPixel));
+
   bool IsDestTyAltivec = DestTy->isVectorType() &&
- (DestTy->castAs()->getVectorKind() ==
-  VectorType::AltiVecVector);
+ ((DestTy->castAs()->getVectorKind() ==
+   VectorType::AltiVecVector) ||
+  (DestTy->castAs()->getVectorKind() ==
+   VectorType::AltiVecBool) ||
+  (DestTy->castAs()->getVectorKind() ==
+   VectorType::AltiVecPixel));
 
   return (IsSrcTyAltivec || IsDestTyAltivec);
 }
 
-// This returns true if both vectors have the same element type.
-bool Sema::areSameVectorElemTypes(QualType SrcTy, QualType DestTy) {
-  assert((DestTy->isVectorType() || SrcTy->isVectorType()) &&
- "expected at least one type to be a vector here");
-
-  uint64_t SrcLen, DestLen;
-  QualType SrcEltTy, DestEltTy;
-  if (!breakDownVectorType(SrcTy, SrcLen, SrcEltTy))
-return false;
-  if (!breakDownVectorType(DestTy, DestLen, DestEltTy))
-return false;
-
-  return (SrcEltTy == DestEltTy);
-}
-
 /// Are the two types lax-compatible vector types?  That is, given
 /// that one of them is a vector, do they have equal storage sizes,
 /// where the storage size is the number of elements times the element
@@ -9848,7 +9842,7 @@
 // change, so if we are converting between vector types where
 // at least one is an Altivec vector, emit a warning.
 if (anyAltivecTypes(RHSType, LHSType) &&
-!areSameVectorElemTypes(RHSType, LHSType))
+!Context.areCompatibleVectorTypes(RHSType, LHSType))
   Diag(RHS.get()->getExprLoc(),

[PATCH] D143210: [PowerPC] Include vector bool and pixel when emitting lax warning

2023-02-15 Thread Maryam Moghadas via Phabricator via cfe-commits 
maryammo added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:9845
 if (anyAltivecTypes(RHSType, LHSType) &&
-!areSameVectorElemTypes(RHSType, LHSType))
+!Context.areCompatibleVectorTypes(RHSType, LHSType))
   Diag(RHS.get()->getExprLoc(), diag::warn_deprecated_lax_vec_conv_all)

amyk wrote:
> Might be a silly question, but what does it mean to be a "compatible" vector 
> type?
`areCompatibleVectorTypes` is defined in lib/AST/ASTContext.cpp,  it considers 
all vectors with the same unqualified types to be compatible, also vectors with 
the same number of elements where neither of them is vector bool/pixel or some 
specific SVE types are compatible.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143210

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

[PATCH] D143210: PowerPC] Include vector bool and pixel when emitting lax warning

2023-02-15 Thread Amy Kwan via Phabricator via cfe-commits 
amyk added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:9845
 if (anyAltivecTypes(RHSType, LHSType) &&
-!areSameVectorElemTypes(RHSType, LHSType))
+!Context.areCompatibleVectorTypes(RHSType, LHSType))
   Diag(RHS.get()->getExprLoc(), diag::warn_deprecated_lax_vec_conv_all)

Might be a silly question, but what does it mean to be a "compatible" vector 
type?



Comment at: clang/test/CodeGen/SystemZ/zvector.c:1
-// RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -fzvector 
-emit-llvm -o - -W -Wall -Werror %s | opt -S -passes=mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -fzvector 
-emit-llvm -o - -W -Wall -Werror -Wno-error=deprecate-lax-vec-conv-all \
+// RUN: %s | opt -S -passes=mem2reg | FileCheck %s

Nit: I think this line is still over 80 characters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143210

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

[PATCH] D143210: PowerPC] Include vector bool and pixel when emitting lax warning

2023-02-02 Thread Maryam Moghadas via Phabricator via cfe-commits 
maryammo created this revision.
Herald added subscribers: shchenz, nemanjai.
Herald added a project: All.
maryammo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch is to fix some missing lax-vector-conversion warnings including
cases that involve vector bool and vector pixel, also to fix the vector
compatibility check for the warnings.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143210

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGen/SystemZ/zvector.c
  clang/test/CodeGen/SystemZ/zvector2.c

Index: clang/test/CodeGen/SystemZ/zvector2.c
===
--- clang/test/CodeGen/SystemZ/zvector2.c
+++ clang/test/CodeGen/SystemZ/zvector2.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z14 -fzvector \
-// RUN:  -O -emit-llvm -o - -W -Wall -Werror %s | FileCheck %s
+// RUN:  -O -emit-llvm -o - -W -Wall -Werror -Wno-error=deprecate-lax-vec-conv-all %s | FileCheck %s
 
 volatile vector float ff, ff2;
 volatile vector bool int bi;
Index: clang/test/CodeGen/SystemZ/zvector.c
===
--- clang/test/CodeGen/SystemZ/zvector.c
+++ clang/test/CodeGen/SystemZ/zvector.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -fzvector -emit-llvm -o - -W -Wall -Werror %s | opt -S -passes=mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -fzvector -emit-llvm -o - -W -Wall -Werror -Wno-error=deprecate-lax-vec-conv-all \
+// RUN: %s | opt -S -passes=mem2reg | FileCheck %s
 
 volatile vector signed char sc, sc2;
 volatile vector unsigned char uc, uc2;
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1770,7 +1770,7 @@
  !ToType->hasAttr(attr::ArmMveStrictPolymorphism))) {
   if (S.isLaxVectorConversion(FromType, ToType) &&
   S.anyAltivecTypes(FromType, ToType) &&
-  !S.areSameVectorElemTypes(FromType, ToType) &&
+  !S.Context.areCompatibleVectorTypes(FromType, ToType) &&
   !InOverloadResolution && !CStyle) {
 S.Diag(From->getBeginLoc(), diag::warn_deprecated_lax_vec_conv_all)
 << FromType << ToType;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7987,30 +7987,24 @@
  "expected at least one type to be a vector here");
 
   bool IsSrcTyAltivec =
-  SrcTy->isVectorType() && (SrcTy->castAs()->getVectorKind() ==
-VectorType::AltiVecVector);
+  SrcTy->isVectorType() && ((SrcTy->castAs()->getVectorKind() ==
+ VectorType::AltiVecVector) ||
+(SrcTy->castAs()->getVectorKind() ==
+ VectorType::AltiVecBool) ||
+(SrcTy->castAs()->getVectorKind() ==
+ VectorType::AltiVecPixel));
+
   bool IsDestTyAltivec = DestTy->isVectorType() &&
- (DestTy->castAs()->getVectorKind() ==
-  VectorType::AltiVecVector);
+ ((DestTy->castAs()->getVectorKind() ==
+   VectorType::AltiVecVector) ||
+  (DestTy->castAs()->getVectorKind() ==
+   VectorType::AltiVecBool) ||
+  (DestTy->castAs()->getVectorKind() ==
+   VectorType::AltiVecPixel));
 
   return (IsSrcTyAltivec || IsDestTyAltivec);
 }
 
-// This returns true if both vectors have the same element type.
-bool Sema::areSameVectorElemTypes(QualType SrcTy, QualType DestTy) {
-  assert((DestTy->isVectorType() || SrcTy->isVectorType()) &&
- "expected at least one type to be a vector here");
-
-  uint64_t SrcLen, DestLen;
-  QualType SrcEltTy, DestEltTy;
-  if (!breakDownVectorType(SrcTy, SrcLen, SrcEltTy))
-return false;
-  if (!breakDownVectorType(DestTy, DestLen, DestEltTy))
-return false;
-
-  return (SrcEltTy == DestEltTy);
-}
-
 /// Are the two types lax-compatible vector types?  That is, given
 /// that one of them is a vector, do they have equal storage sizes,
 /// where the storage size is the number of elements times the element
@@ -9848,7 +9842,7 @@
 // change, so if we are converting between vector types where
 // at least one is an Altivec vector, emit a warning.
 if (anyAltivecTypes(RHSType, LHSType) &&
-!areSameVectorElemTypes(RHSType, LHSType))
+!Context.areCompatibleVectorTypes(RHSType, LHSType))
   Diag(RHS.get()->getExprLoc(),