[PATCH] D78576: [SVE] Make VectorType::getNumElements() complain for scalable vectors

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau created this revision.
Herald added subscribers: llvm-commits, psnobl, rkruppe, tschuett, mgorny.
Herald added a reviewer: efriedma.
Herald added a project: LLVM.
ctetreau added reviewers: sdesmalen, c-rhodes.
ctetreau added a parent revision: D77587: [SVE] Add new VectorType subclasses.

Piggy-back off of TypeSize's STRICT_FIXED_SIZE_VECTORS flag and:

- if it is defined, assert that the vector is not scalable
- if it is not defined, complain if the vector is scalable


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78576

Files:
  llvm/CMakeLists.txt
  llvm/include/llvm/IR/DerivedTypes.h


Index: llvm/include/llvm/IR/DerivedTypes.h
===================================================================
--- llvm/include/llvm/IR/DerivedTypes.h
+++ llvm/include/llvm/IR/DerivedTypes.h
@@ -417,7 +417,21 @@
   /// Get the number of elements in this vector. It does not make sense to call
   /// this function on a scalable vector, and this will be moved into
   /// FixedVectorType in a future commit
-  unsigned getNumElements() const { return EC.Min; }
+  unsigned getNumElements() const {
+    ElementCount EC = getElementCount();
+#ifdef STRICT_FIXED_SIZE_VECTORS
+    assert(!EC.Scalable &&
+           "Request for fixed number of elements from scalable vector");
+    return EC.Min;
+#else
+    if (EC.Scalable)
+      WithColor::warning()
+          << "The code that requested the fixed number of elements has made "
+             "the assumption that this vector is not scalable. This assumption "
+             "was not correct, and this may lead to broken code\n";
+    return EC.Min;
+#endif
+  }
 
   Type *getElementType() const { return ContainedType; }
 
Index: llvm/CMakeLists.txt
===================================================================
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -415,15 +415,16 @@
 option(LLVM_ENABLE_EXPENSIVE_CHECKS "Enable expensive checks" OFF)
 
 # While adding scalable vector support to LLVM, we temporarily want to
-# allow an implicit conversion of TypeSize to uint64_t. This CMake flag
-# enables a more strict conversion where it asserts that the type is not
-# a scalable vector type.
+# allow an implicit conversion of TypeSize to uint64_t, and to allow
+# code to get the fixed number of elements from a possibly scalable vector.
+# This CMake flag enables a more strict mode where it asserts that the type
+# is not a scalable vector type.
 #
 # Enabling this flag makes it easier to find cases where the compiler makes
 # assumptions on the size being 'fixed size', when building tests for
 # SVE/SVE2 or other scalable vector architectures.
 option(LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORS
-       "Enable assertions that type is not scalable in implicit conversion from TypeSize to uint64_t" OFF)
+       "Enable assertions that type is not scalable in implicit conversion from TypeSize to uint64_t and calls to getNumElements" OFF)
 
 set(LLVM_ABI_BREAKING_CHECKS "WITH_ASSERTS" CACHE STRING
   "Enable abi-breaking checks.  Can be WITH_ASSERTS, FORCE_ON or FORCE_OFF.")


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78576.259052.patch
Type: text/x-patch
Size: 2448 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200421/c7a2fde9/attachment.bin>