[PATCH] D75297: [TypeSize] Allow returning scalable size in implicit conversion to uint64_t

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen updated this revision to Diff 248521.
sdesmalen added a comment.
Herald added a subscriber: mgorny.

- Added cmake option to enable use of getFixedSize() in implicit conversion of TypeSize to uint64_t, which triggers an assertion failure when the size is scalable.
- Added deprecation warning for the implicit conversion of TypeSize->uint64_t.


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

https://reviews.llvm.org/D75297

Files:
  llvm/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/include/llvm/Support/TypeSize.h


Index: llvm/include/llvm/Support/TypeSize.h
===================================================================
--- llvm/include/llvm/Support/TypeSize.h
+++ llvm/include/llvm/Support/TypeSize.h
@@ -146,10 +146,29 @@
 
   // Casts to a uint64_t if this is a fixed-width size.
   //
-  // NOTE: This interface is obsolete and will be removed in a future version
-  // of LLVM in favour of calling getFixedSize() directly.
-  operator uint64_t() const {
+  // This interface is deprecated and will be removed in a future version
+  // of LLVM in favour of upgrading uses that rely on this implicit conversion
+  // to uint64_t. Calls to functions that return a TypeSize should use the
+  // proper interfaces to TypeSize.
+  // In practice this is mostly calls to MVT/EVT::getSizeInBits().
+  //
+  // To determine how to upgrade the code:
+  //
+  //   if (<algorithm works for both scalable and fixed-width vectors>)
+  //     use getKnownMinSize()
+  //   else if (<algorithm works only for fixed-width vectors>) {
+  //     if <algorithm can be adapted for both scalable and fixed-width vectors>
+  //       update the algorithm and use getKnownMinSize()
+  //     else
+  //       bail out early for scalable vectors and use getFixedSize()
+  //   }
+  LLVM_ATTRIBUTE_DEPRECATED(operator uint64_t() const,
+                            "Use explicit interfaces on TypeSize instead") {
+#ifdef STRICT_IMPLICIT_CONVERSION_TYPESIZE
     return getFixedSize();
+#else
+    return getKnownMinSize();
+#endif
   }
 
   // Additional convenience operators needed to avoid ambiguous parses.
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===================================================================
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -80,6 +80,10 @@
   add_definitions(-D_GLIBCXX_DEBUG)
 endif()
 
+if (LLVM_ENABLE_STRICT_IMPLICIT_CONVERSION_TYPESIZE)
+  add_definitions(-DSTRICT_IMPLICIT_CONVERSION_TYPESIZE)
+endif()
+
 string(TOUPPER "${LLVM_ABI_BREAKING_CHECKS}" uppercase_LLVM_ABI_BREAKING_CHECKS)
 
 if( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "WITH_ASSERTS" )
Index: llvm/CMakeLists.txt
===================================================================
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -415,6 +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.
+#
+# 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_IMPLICIT_CONVERSION_TYPESIZE "Enable assertions that type is not scalable in implicit conversion from TypeSize to uint64_t" 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: D75297.248521.patch
Type: text/x-patch
Size: 3118 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200305/1138e538/attachment.bin>