[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 248763.
sdesmalen added a comment.
Herald added subscribers: kerbowa, hiraditya, nhaehnle, jvesely, arsenm.

- Replaced deprecated warning (at LLVM compile time) with runtime warning message.


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
  llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp


Index: llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
@@ -605,7 +605,7 @@
 
   // parse type
   char const TC = param.front();
-  if (::isDigit(TC)) {
+  if (isDigit(TC)) {
     res.ArgType = StringSwitch<AMDGPULibFunc::EType>
       (eatLengthPrefixedName(param))
       .Case("ocl_image1darray" , AMDGPULibFunc::IMG1DA)
Index: llvm/include/llvm/Support/TypeSize.h
===================================================================
--- llvm/include/llvm/Support/TypeSize.h
+++ llvm/include/llvm/Support/TypeSize.h
@@ -15,6 +15,8 @@
 #ifndef LLVM_SUPPORT_TYPESIZE_H
 #define LLVM_SUPPORT_TYPESIZE_H
 
+#include "llvm/Support/WithColor.h"
+
 #include <cstdint>
 #include <cassert>
 
@@ -146,10 +148,32 @@
 
   // 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.
+  // 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()
+  //   }
   operator uint64_t() const {
+#ifdef STRICT_IMPLICIT_CONVERSION_TYPESIZE
     return getFixedSize();
+#else
+    if (isScalable())
+      WithColor::warning() << "Compiler has made implicit assumption that "
+                              "TypeSize is not scalable. This may or may not "
+                              "lead to broken code.\n";
+    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.248763.patch
Type: text/x-patch
Size: 3862 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200306/ebb4bcbc/attachment-0001.bin>