[PATCH] D79755: Implement constexpr BinaryOperator for vector types

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon closed this revision.
RKSimon added a comment.

Committed at rGb30c16670e428d09a0854a8f418e46a3e705e4d1 
 (with a 
typo in the Differential Revision tag so phab didn't catch it)


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

https://reviews.llvm.org/D79755



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

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D79755



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

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 271713.
erichkeane marked 9 inline comments as done.
erichkeane added a comment.

@AaronBallman's suggestions.


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

https://reviews.llvm.org/D79755

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-vectors.cpp

Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -0,0 +1,616 @@
+// RUN: %clang_cc1 -std=c++14 -Wno-unused-value %s -disable-llvm-passes -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s
+
+// FIXME: Unfortunately there is no good way to validate that our values are
+// correct since Vector types don't have operator [] implemented for constexpr.
+// Instead, we need to use filecheck to ensure the emitted IR is correct. Once
+// someone implements array subscript operator for these types as constexpr,
+// this test should modified to jsut use static asserts.
+
+using FourCharsVecSize __attribute__((vector_size(4))) = char;
+using FourIntsVecSize __attribute__((vector_size(16))) = int;
+using FourLongLongsVecSize __attribute__((vector_size(32))) = long long;
+using FourFloatsVecSize __attribute__((vector_size(16))) = float;
+using FourDoublesVecSize __attribute__((vector_size(32))) = double;
+
+using FourCharsExtVec __attribute__((ext_vector_type(4))) = char;
+using FourIntsExtVec __attribute__((ext_vector_type(4))) = int;
+using FourLongLongsExtVec __attribute__((ext_vector_type(4))) = long long;
+using FourFloatsExtVec __attribute__((ext_vector_type(4))) = float;
+using FourDoublesExtVec __attribute__((ext_vector_type(4))) = double;
+
+// Next a series of tests to make sure these operations are usable in
+// constexpr functions. Template instantiations don't emit Winvalid-constexpr,
+// so we have to do these as macros.
+#define MathShiftOps(Type)\
+  constexpr auto MathShiftOps##Type(Type a, Type b) { \
+a = a + b;\
+a = a - b;\
+a = a * b;\
+a = a / b;\
+b = a + 1;\
+b = a - 1;\
+b = a * 1;\
+b = a / 1;\
+a += a;   \
+a -= a;   \
+a *= a;   \
+a /= a;   \
+b += a;   \
+b -= a;   \
+b *= a;   \
+b /= a;   \
+a < b;\
+a > b;\
+a <= b;   \
+a >= b;   \
+a == b;   \
+a != b;   \
+a &\
+a || b;   \
+auto c = (a, b);  \
+return c; \
+  }
+
+// Ops specific to Integers.
+#define MathShiftOpsInts(Type)\
+  constexpr auto MathShiftopsInts##Type(Type a, Type b) { \
+a = a << b;   \
+a = a >> b;   \
+a = a << 3;   \
+a = a >> 3;   \
+a = 3 << b;   \
+a = 3 >> b;   \
+a <<= b;  \
+a >>= b;  \
+a <<= 3;  \
+a >>= 3;  \
+a = a % b;\
+a  \
+a | b;\
+a ^ b;\
+return a; \
+  }
+
+MathShiftOps(FourCharsVecSize);
+MathShiftOps(FourIntsVecSize);
+MathShiftOps(FourLongLongsVecSize);
+MathShiftOps(FourFloatsVecSize);
+MathShiftOps(FourDoublesVecSize);
+MathShiftOps(FourCharsExtVec);
+MathShiftOps(FourIntsExtVec);
+MathShiftOps(FourLongLongsExtVec);
+MathShiftOps(FourFloatsExtVec);
+MathShiftOps(FourDoublesExtVec);
+
+MathShiftOpsInts(FourCharsVecSize);
+MathShiftOpsInts(FourIntsVecSize);

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2699
+RHSValue.getInt(), Result);
+  assert(LHSValue.getKind() == APValue::Float && "SHhuld be no other options");
+  return handleLogicalOpForVector(LHSValue.getFloat(), Opcode,

typo: Should



Comment at: clang/lib/AST/ExprConstant.cpp:2741
+  RHSValue.getInt(), Result);
+  assert(LHSValue.getKind() == APValue::Float && "SHhuld be no other options");
+  return handleCompareOpForVectorHelper(LHSValue.getFloat(), Opcode,

Typo: Should



Comment at: clang/lib/AST/ExprConstant.cpp:2754
+
+  const VectorType *VT = E->getType()->castAs();
+  unsigned NumElements = VT->getNumElements();

`const auto *`



Comment at: clang/lib/AST/ExprConstant.cpp:2778
+if (EltTy->isIntegerType()) {
+
+  APSInt EltResult{Info.Ctx.getIntWidth(EltTy),

Might as well remove the spurious whitespace.



Comment at: clang/lib/AST/ExprConstant.cpp:2782
+
+  if (BinaryOperator::isLogicalOp(Opcode)) {
+if (!handleLogicalOpForVector(LHSElt, Opcode, RHSElt, EltResult)) {

How about:
```
bool Success = true;
if (BinaryOperator::isLogicalOp(Opcode))
  Success = handleLogicalOpForVector(...);
else if (BinaryOperator::isComparisonOp(Opcode))
  Success = handleCompareOpForVector(...);
else
  Success = handleIntIntBinOp(...);

if (!Success) {
  Info.FFDiag(E);
  return false;
}
ResultElements.push_back(...);
```



Comment at: clang/lib/AST/ExprConstant.cpp:2799
+  }
+  ResultElements.push_back(APValue(EltResult));
+

`emplace_back()` instead?



Comment at: clang/lib/AST/ExprConstant.cpp:2800
+  ResultElements.push_back(APValue(EltResult));
+
+} else if (EltTy->isFloatingType()) {

Spurious newline?



Comment at: clang/lib/AST/ExprConstant.cpp:2813
+
+  ResultElements.push_back(APValue(LHSFloat));
+}

`emplace_back()` instead?



Comment at: clang/lib/AST/ExprConstant.cpp:9688
+bool VisitBinaryOperator(const BinaryOperator *E);
+// FIXME: Missing: unary -, unary ~,
 // conditional operator (for GNU conditional select),

Can you re-flow the rest of the comment now that it's been changed?


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

https://reviews.llvm.org/D79755



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

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon edited reviewers, added: efriedma; removed: eli.friedman.
RKSimon added a comment.

Any frontend people got any comments on this? I'm keen to see constexpr vector 
support added but know next to nothing about the frontend.


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

https://reviews.llvm.org/D79755



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

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added reviewers: rsmith, void.
RKSimon added a comment.

Adding some people who know more about this


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

https://reviews.llvm.org/D79755



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

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: eli.friedman, RKSimon, aaron.ballman.

These operations do member-wise versions of the all of the listed
operations.  This patch implements all of the binaryoperators for these
types. Note that the test is required to use codegen as I could not come
up with a good way to validate the values without the array-subscript
operator implemented (which is likely a much more involved change).

This is not full constexpr support for these types, but should at least 
support a large number of uses.


https://reviews.llvm.org/D79755

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-vectors.cpp

Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -0,0 +1,616 @@
+// RUN: %clang_cc1 -std=c++14 -Wno-unused-value %s -disable-llvm-passes -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s
+
+// FIXME: Unfortunately there is no good way to validate that our values are
+// correct since Vector types don't have operator [] implemented for constexpr.
+// Instead, we need to use filecheck to ensure the emitted IR is correct. Once
+// someone implements array subscript operator for these types as constexpr,
+// this test should modified to jsut use static asserts.
+
+using FourCharsVecSize __attribute__((vector_size(4))) = char;
+using FourIntsVecSize __attribute__((vector_size(16))) = int;
+using FourLongLongsVecSize __attribute__((vector_size(32))) = long long;
+using FourFloatsVecSize __attribute__((vector_size(16))) = float;
+using FourDoublesVecSize __attribute__((vector_size(32))) = double;
+
+using FourCharsExtVec __attribute__((ext_vector_type(4))) = char;
+using FourIntsExtVec __attribute__((ext_vector_type(4))) = int;
+using FourLongLongsExtVec __attribute__((ext_vector_type(4))) = long long;
+using FourFloatsExtVec __attribute__((ext_vector_type(4))) = float;
+using FourDoublesExtVec __attribute__((ext_vector_type(4))) = double;
+
+// Next a series of tests to make sure these operations are usable in
+// constexpr functions. Template instantiations don't emit Winvalid-constexpr,
+// so we have to do these as macros.
+#define MathShiftOps(Type)\
+  constexpr auto MathShiftOps##Type(Type a, Type b) { \
+a = a + b;\
+a = a - b;\
+a = a * b;\
+a = a / b;\
+b = a + 1;\
+b = a - 1;\
+b = a * 1;\
+b = a / 1;\
+a += a;   \
+a -= a;   \
+a *= a;   \
+a /= a;   \
+b += a;   \
+b -= a;   \
+b *= a;   \
+b /= a;   \
+a < b;\
+a > b;\
+a <= b;   \
+a >= b;   \
+a == b;   \
+a != b;   \
+a &\
+a || b;   \
+auto c = (a, b);  \
+return c; \
+  }
+
+// Ops specific to Integers.
+#define MathShiftOpsInts(Type)\
+  constexpr auto MathShiftopsInts##Type(Type a, Type b) { \
+a = a << b;   \
+a = a >> b;   \
+a = a << 3;   \
+a = a >> 3;   \
+a = 3 << b;   \
+a = 3 >> b;   \
+a <<= b;  \
+a >>= b;  \
+a <<= 3;  \
+a >>= 3;  \
+a = a % b;\
+a  \
+a | b;\
+a ^ b;\
+return a; \
+  }
+
+MathShiftOps(FourCharsVecSize);
+MathShiftOps(FourIntsVecSize);