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

This patch removes compiler runtime assertions that ensure the implicit
conversion are only guaranteed to work for fixed-width vectors.

With the assert it would be impossible to get _anything_ to build until
the
entire codebase has been upgraded, even when the indiscriminate uses of
the size as uint64_t would work fine for both scalable and fixed-width
types.

This issue will need to be addressed differently, with build-time errors
rather than assertion failures, but that effort falls beyond the scope
of this patch.

Returning the scalable size and avoiding the assert in getFixedSize()
is a temporary stop-gap in order to use LLVM for compiling and using
the SVE ACLE intrinsics.

Reviewers: efriedma, huntergr, rovka, ctetreau, rengolin

Reviewed By: efriedma

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D75297

diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 23f36d5..3bb037b 100644 (file)
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -410,6 +410,17 @@ endif()
 
 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.")
 

diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index df231eb..70ad34a 100644 (file)
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -95,6 +95,10 @@ if(LLVM_ENABLE_EXPENSIVE_CHECKS)
   endif()
 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" )

diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 253190c..d800317 100644 (file)
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/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 @@ public:
 
   // 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.

diff --git a/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp b/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
index 1f14eb3..06f2ff3 100644 (file)
--- a/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
@@ -14,6 +14,7 @@
 #include "AMDGPULibFunc.h"
 #include <llvm/ADT/SmallString.h>
 #include <llvm/ADT/SmallVector.h>
+#include "llvm/ADT/StringExtras.h"
 #include <llvm/ADT/StringSwitch.h>
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/DerivedTypes.h"
@@ -479,8 +480,6 @@ static bool eatTerm(StringRef& mangledName, const char (&str)[N]) {
   return false;
 }
 
-static inline bool isDigit(char c) { return c >= '0' && c <= '9'; }
-
 static int eatNumber(StringRef& s) {
   size_t const savedSize = s.size();
   int n = 0;
@@ -605,7 +604,7 @@ bool ItaniumParamParser::parseItaniumParam(StringRef& param,
 
   // 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)