[llvm] 8105935 - [TypeSize] Allow returning scalable size in implicit conversion to uint64_t
Sander de Smalen via llvm-commits
llvm-commits at lists.llvm.org
Sun Mar 15 07:11:35 PDT 2020
Author: Sander de Smalen
Date: 2020-03-15T13:48:49Z
New Revision: 8105935d3aa98089f24b356ae782771d024d9174
URL: https://github.com/llvm/llvm-project/commit/8105935d3aa98089f24b356ae782771d024d9174
DIFF: https://github.com/llvm/llvm-project/commit/8105935d3aa98089f24b356ae782771d024d9174.diff
LOG: [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
Added:
Modified:
llvm/CMakeLists.txt
llvm/cmake/modules/HandleLLVMOptions.cmake
llvm/include/llvm/Support/TypeSize.h
llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
Removed:
################################################################################
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 23f36d5039a7..3bb037b803de 100644
--- 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 df231eb3308c..70ad34a41bde 100644
--- 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 253190c08a04..d80031720422 100644
--- 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 @@ class TypeSize {
// 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 1f14eb3a6fad..06f2ff39844d 100644
--- 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)
More information about the llvm-commits
mailing list