[IR] Remove options to make scalable TypeSize access a warning #156336
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes the `LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORS` cmake option and the `-treat-scalable-fixed-error-as-warning` opt flag. We stopped treating these as warnings by default a long time ago (62f09d7), so I don't think it makes sense to retain these options at this point. Accessing a scalable TypeSize as fixed should always result in an error.
llvmbot
added
cmake
Member
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-llvm-support Author: Nikita Popov (nikic) ChangesThis removes the We stopped treating these as warnings by default a long time ago (62f09d7), so I don't think it makes sense to retain these options at this point. Accessing a scalable TypeSize as fixed should always result in an error. Full diff: https://github.com/llvm/llvm-project/pull/156336.diff 10 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index a90be4f6235e6..3eb695665130d 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -706,18 +706,6 @@ 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, 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 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.")
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 97aa1317bc218..a67543cdb668f 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -201,10 +201,6 @@ if (LLVM_USES_LIBSTDCXX)
endif()
endif()
-if (LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORS)
- add_compile_definitions(STRICT_FIXED_SIZE_VECTORS)
-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/CodeGen/ValueTypes.h b/llvm/include/llvm/CodeGen/ValueTypes.h
index 2a91cdc3ab134..43aaa2bcfc222 100644
--- a/llvm/include/llvm/CodeGen/ValueTypes.h
+++ b/llvm/include/llvm/CodeGen/ValueTypes.h
@@ -332,7 +332,7 @@ namespace llvm {
assert(isVector() && "Invalid vector type!");
if (isScalableVector())
- llvm::reportInvalidSizeRequest(
+ llvm::reportFatalInternalError(
"Possible incorrect use of EVT::getVectorNumElements() for "
"scalable vector. Scalable flag may be dropped, use "
"EVT::getVectorElementCount() instead");
diff --git a/llvm/include/llvm/CodeGenTypes/LowLevelType.h b/llvm/include/llvm/CodeGenTypes/LowLevelType.h
index d8e0848aff84d..4c1fe13790011 100644
--- a/llvm/include/llvm/CodeGenTypes/LowLevelType.h
+++ b/llvm/include/llvm/CodeGenTypes/LowLevelType.h
@@ -159,7 +159,7 @@ class LLT {
/// vector types.
constexpr uint16_t getNumElements() const {
if (isScalable())
- llvm::reportInvalidSizeRequest(
+ llvm::reportFatalInternalError(
"Possible incorrect use of LLT::getNumElements() for "
"scalable vector. Scalable flag may be dropped, use "
"LLT::getElementCount() instead");
diff --git a/llvm/include/llvm/CodeGenTypes/MachineValueType.h b/llvm/include/llvm/CodeGenTypes/MachineValueType.h
index b8e91a022ec5e..389bae209f406 100644
--- a/llvm/include/llvm/CodeGenTypes/MachineValueType.h
+++ b/llvm/include/llvm/CodeGenTypes/MachineValueType.h
@@ -294,7 +294,7 @@ namespace llvm {
unsigned getVectorNumElements() const {
if (isScalableVector())
- llvm::reportInvalidSizeRequest(
+ llvm::reportFatalInternalError(
"Possible incorrect use of MVT::getVectorNumElements() for "
"scalable vector. Scalable flag may be dropped, use "
"MVT::getVectorElementCount() instead");
diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 96425291aaedb..b6926e6fdeeb6 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -26,10 +26,6 @@
namespace llvm {
-/// Reports a diagnostic message to indicate an invalid size request has been
-/// done on a scalable vector. This function may not return.
-LLVM_ABI void reportInvalidSizeRequest(const char *Msg);
-
/// StackOffset holds a fixed and a scalable offset in bytes.
class StackOffset {
int64_t Fixed = 0;
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 8491633df97e8..be232f5bff587 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -2671,7 +2671,6 @@ static void initCommonOptions() {
initSignalsOptions();
initStatisticOptions();
initTimerOptions();
- initTypeSizeOptions();
initWithColorOptions();
initDebugOptions();
initRandomSeedOptions();
diff --git a/llvm/lib/Support/DebugOptions.h b/llvm/lib/Support/DebugOptions.h
index db727d5a584cb..6c3382e8f858d 100644
--- a/llvm/lib/Support/DebugOptions.h
+++ b/llvm/lib/Support/DebugOptions.h
@@ -24,7 +24,6 @@ void initGraphWriterOptions();
void initSignalsOptions();
void initStatisticOptions();
void initTimerOptions();
-void initTypeSizeOptions();
void initWithColorOptions();
void initDebugOptions();
void initRandomSeedOptions();
diff --git a/llvm/lib/Support/TypeSize.cpp b/llvm/lib/Support/TypeSize.cpp
index 43346b81cd676..3dbb00880faca 100644
--- a/llvm/lib/Support/TypeSize.cpp
+++ b/llvm/lib/Support/TypeSize.cpp
@@ -7,49 +7,13 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/TypeSize.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
-#include "llvm/Support/WithColor.h"
-
-#include "DebugOptions.h"
+#include "llvm/Support/Error.h"
using namespace llvm;
-#ifndef STRICT_FIXED_SIZE_VECTORS
-namespace {
-struct CreateScalableErrorAsWarning {
- /// The ScalableErrorAsWarning is a temporary measure to suppress errors from
- /// using the wrong interface on a scalable vector.
- static void *call() {
- return new cl::opt<bool>(
- "treat-scalable-fixed-error-as-warning", cl::Hidden,
- cl::desc(
- "Treat issues where a fixed-width property is requested from a "
- "scalable type as a warning, instead of an error"));
- }
-};
-} // namespace
-static ManagedStatic<cl::opt<bool>, CreateScalableErrorAsWarning>
- ScalableErrorAsWarning;
-void llvm::initTypeSizeOptions() { *ScalableErrorAsWarning; }
-#else
-void llvm::initTypeSizeOptions() {}
-#endif
-
-void llvm::reportInvalidSizeRequest(const char *Msg) {
-#ifndef STRICT_FIXED_SIZE_VECTORS
- if (*ScalableErrorAsWarning) {
- WithColor::warning() << "Invalid size request on a scalable vector; " << Msg
- << "\n";
- return;
- }
-#endif
- report_fatal_error("Invalid size request on a scalable vector.");
-}
-
TypeSize::operator TypeSize::ScalarTy() const {
if (isScalable()) {
- reportInvalidSizeRequest(
+ reportFatalInternalError(
"Cannot implicitly convert a scalable size to a fixed-width size in "
"`TypeSize::operator ScalarTy()`");
return getKnownMinValue();
diff --git a/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir b/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir
index 4d8067e16b964..61e3c73a3ee53 100644
--- a/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir
+++ b/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir
@@ -1,4 +1,4 @@
-# RUN: llc --verify-machineinstrs -mtriple=aarch64 -o - %s -mcpu=a64fx -aarch64-enable-pipeliner -pipeliner-max-mii=100 -pipeliner-enable-copytophi=0 -debug-only=pipeliner -run-pass=pipeliner -treat-scalable-fixed-error-as-warning 2>&1 | FileCheck %s
+# RUN: llc --verify-machineinstrs -mtriple=aarch64 -o - %s -mcpu=a64fx -aarch64-enable-pipeliner -pipeliner-max-mii=100 -pipeliner-enable-copytophi=0 -debug-only=pipeliner -run-pass=pipeliner 2>&1 | FileCheck %s
# REQUIRES: asserts
|
Member
|
@llvm/pr-subscribers-backend-aarch64 Author: Nikita Popov (nikic) ChangesThis removes the We stopped treating these as warnings by default a long time ago (62f09d7), so I don't think it makes sense to retain these options at this point. Accessing a scalable TypeSize as fixed should always result in an error. Full diff: https://github.com/llvm/llvm-project/pull/156336.diff 10 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index a90be4f6235e6..3eb695665130d 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -706,18 +706,6 @@ 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, 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 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.")
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 97aa1317bc218..a67543cdb668f 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -201,10 +201,6 @@ if (LLVM_USES_LIBSTDCXX)
endif()
endif()
-if (LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORS)
- add_compile_definitions(STRICT_FIXED_SIZE_VECTORS)
-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/CodeGen/ValueTypes.h b/llvm/include/llvm/CodeGen/ValueTypes.h
index 2a91cdc3ab134..43aaa2bcfc222 100644
--- a/llvm/include/llvm/CodeGen/ValueTypes.h
+++ b/llvm/include/llvm/CodeGen/ValueTypes.h
@@ -332,7 +332,7 @@ namespace llvm {
assert(isVector() && "Invalid vector type!");
if (isScalableVector())
- llvm::reportInvalidSizeRequest(
+ llvm::reportFatalInternalError(
"Possible incorrect use of EVT::getVectorNumElements() for "
"scalable vector. Scalable flag may be dropped, use "
"EVT::getVectorElementCount() instead");
diff --git a/llvm/include/llvm/CodeGenTypes/LowLevelType.h b/llvm/include/llvm/CodeGenTypes/LowLevelType.h
index d8e0848aff84d..4c1fe13790011 100644
--- a/llvm/include/llvm/CodeGenTypes/LowLevelType.h
+++ b/llvm/include/llvm/CodeGenTypes/LowLevelType.h
@@ -159,7 +159,7 @@ class LLT {
/// vector types.
constexpr uint16_t getNumElements() const {
if (isScalable())
- llvm::reportInvalidSizeRequest(
+ llvm::reportFatalInternalError(
"Possible incorrect use of LLT::getNumElements() for "
"scalable vector. Scalable flag may be dropped, use "
"LLT::getElementCount() instead");
diff --git a/llvm/include/llvm/CodeGenTypes/MachineValueType.h b/llvm/include/llvm/CodeGenTypes/MachineValueType.h
index b8e91a022ec5e..389bae209f406 100644
--- a/llvm/include/llvm/CodeGenTypes/MachineValueType.h
+++ b/llvm/include/llvm/CodeGenTypes/MachineValueType.h
@@ -294,7 +294,7 @@ namespace llvm {
unsigned getVectorNumElements() const {
if (isScalableVector())
- llvm::reportInvalidSizeRequest(
+ llvm::reportFatalInternalError(
"Possible incorrect use of MVT::getVectorNumElements() for "
"scalable vector. Scalable flag may be dropped, use "
"MVT::getVectorElementCount() instead");
diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 96425291aaedb..b6926e6fdeeb6 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -26,10 +26,6 @@
namespace llvm {
-/// Reports a diagnostic message to indicate an invalid size request has been
-/// done on a scalable vector. This function may not return.
-LLVM_ABI void reportInvalidSizeRequest(const char *Msg);
-
/// StackOffset holds a fixed and a scalable offset in bytes.
class StackOffset {
int64_t Fixed = 0;
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 8491633df97e8..be232f5bff587 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -2671,7 +2671,6 @@ static void initCommonOptions() {
initSignalsOptions();
initStatisticOptions();
initTimerOptions();
- initTypeSizeOptions();
initWithColorOptions();
initDebugOptions();
initRandomSeedOptions();
diff --git a/llvm/lib/Support/DebugOptions.h b/llvm/lib/Support/DebugOptions.h
index db727d5a584cb..6c3382e8f858d 100644
--- a/llvm/lib/Support/DebugOptions.h
+++ b/llvm/lib/Support/DebugOptions.h
@@ -24,7 +24,6 @@ void initGraphWriterOptions();
void initSignalsOptions();
void initStatisticOptions();
void initTimerOptions();
-void initTypeSizeOptions();
void initWithColorOptions();
void initDebugOptions();
void initRandomSeedOptions();
diff --git a/llvm/lib/Support/TypeSize.cpp b/llvm/lib/Support/TypeSize.cpp
index 43346b81cd676..3dbb00880faca 100644
--- a/llvm/lib/Support/TypeSize.cpp
+++ b/llvm/lib/Support/TypeSize.cpp
@@ -7,49 +7,13 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/TypeSize.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
-#include "llvm/Support/WithColor.h"
-
-#include "DebugOptions.h"
+#include "llvm/Support/Error.h"
using namespace llvm;
-#ifndef STRICT_FIXED_SIZE_VECTORS
-namespace {
-struct CreateScalableErrorAsWarning {
- /// The ScalableErrorAsWarning is a temporary measure to suppress errors from
- /// using the wrong interface on a scalable vector.
- static void *call() {
- return new cl::opt<bool>(
- "treat-scalable-fixed-error-as-warning", cl::Hidden,
- cl::desc(
- "Treat issues where a fixed-width property is requested from a "
- "scalable type as a warning, instead of an error"));
- }
-};
-} // namespace
-static ManagedStatic<cl::opt<bool>, CreateScalableErrorAsWarning>
- ScalableErrorAsWarning;
-void llvm::initTypeSizeOptions() { *ScalableErrorAsWarning; }
-#else
-void llvm::initTypeSizeOptions() {}
-#endif
-
-void llvm::reportInvalidSizeRequest(const char *Msg) {
-#ifndef STRICT_FIXED_SIZE_VECTORS
- if (*ScalableErrorAsWarning) {
- WithColor::warning() << "Invalid size request on a scalable vector; " << Msg
- << "\n";
- return;
- }
-#endif
- report_fatal_error("Invalid size request on a scalable vector.");
-}
-
TypeSize::operator TypeSize::ScalarTy() const {
if (isScalable()) {
- reportInvalidSizeRequest(
+ reportFatalInternalError(
"Cannot implicitly convert a scalable size to a fixed-width size in "
"`TypeSize::operator ScalarTy()`");
return getKnownMinValue();
diff --git a/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir b/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir
index 4d8067e16b964..61e3c73a3ee53 100644
--- a/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir
+++ b/llvm/test/CodeGen/AArch64/sms-order-physreg-deps.mir
@@ -1,4 +1,4 @@
-# RUN: llc --verify-machineinstrs -mtriple=aarch64 -o - %s -mcpu=a64fx -aarch64-enable-pipeliner -pipeliner-max-mii=100 -pipeliner-enable-copytophi=0 -debug-only=pipeliner -run-pass=pipeliner -treat-scalable-fixed-error-as-warning 2>&1 | FileCheck %s
+# RUN: llc --verify-machineinstrs -mtriple=aarch64 -o - %s -mcpu=a64fx -aarch64-enable-pipeliner -pipeliner-max-mii=100 -pipeliner-enable-copytophi=0 -debug-only=pipeliner -run-pass=pipeliner 2>&1 | FileCheck %s
# REQUIRES: asserts
|
nikic
added a commit
to nikic/llvm-zorg
that referenced
this pull request
Sep 1, 2025
This is already the default, and I'm dropping this option in llvm/llvm-project#156336.
Collaborator
|
Linaro has buildbots using this option, for example https://github.com/llvm/llvm-zorg/blob/2b44506b85cf6d604d36e8b8c882a35cbf508d3f/buildbot/osuosl/master/config/builders.py#L481. We did so at the request of Arm, so if the reviewers here agree with this change I will do the changes for llvm-zorg. I would appreciate it if we could get the buildbot changes live first (as this takes a few days usually) and then merge this change. A very small point in favour of this change, it would remove a ton of warnings from the buildbots. Since we get a lot of "option unused" when doing linking via. clang. |
Collaborator
|
Oh and I see you just posted a zorg PR :) Thanks! |
sdesmalen-arm
approved these changes
Sep 1, 2025
Collaborator
sdesmalen-arm
left a comment
sdesmalen-arm
left a comment
There was a problem hiding this comment.
Given how scalable vector support has matured in LLVM over the past few years and that Clang has stopped demoting these errors for a while now, it makes sense for this code to be removed.
nikic
added a commit
to llvm/llvm-zorg
that referenced
this pull request
Sep 1, 2025
…#584) This is already the default, and I'm dropping this option in llvm/llvm-project#156336.
paulwalker-arm
approved these changes
Sep 1, 2025
Contributor
Author
|
I believe the buildbot changes should be deployed now (though the buildbots are broken for an unrelated reason). We can revert this if I'm wrong about that... |
Contributor
Author
And reverted. I assumed the fact that buildbot was down all of yesterday would surely require a restart, but apparently that was not right. How can you find out which configuration buildbot currently uses? |
Collaborator
|
That was my first thought too but I suspect it's got some sort of staging branch in case of problems with main. The bot is currently configuring stage 2 with: https://lab.llvm.org/buildbot/#/builders/41/builds/8602/steps/10/logs/stdio Which includes the flag. |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/20838 Here is the relevant piece of the build log for the reference |
Collaborator
|
We also need llvm/llvm-zorg#586 to correct a different problem, so I've emailed Galina to ask for a restart that includes that and your zorg change. This may take longer than usual depending on what the problem behind the scenes was. |
Collaborator
|
You can reland this now, the bots have updated. |
nikic
added a commit
that referenced
this pull request
Sep 3, 2025
#156336) Reapplying now that buildbot has picked up the new configuration that does not use -treat-scalable-fixed-error-as-warning. ----- This removes the `LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORS` cmake option and the `-treat-scalable-fixed-error-as-warning` opt flag. We stopped treating these as warnings by default a long time ago (62f09d7), so I don't think it makes sense to retain these options at this point. Accessing a scalable TypeSize as fixed should always result in an error.
nikic
added a commit
to nikic/llvm-project
that referenced
this pull request
Sep 8, 2025
After llvm#156336 this cast operator has become trivial, so inline it into the header. This is a minor compile-time improvement.
nikic
added a commit
that referenced
this pull request
Sep 8, 2025
After #156336 this cast operator has become trivial, so inline it into the header. This is a minor compile-time improvement.
vvereschaka
pushed a commit
to vvereschaka/llvm-zorg
that referenced
this pull request
Sep 25, 2025
…llvm#584) This is already the default, and I'm dropping this option in llvm/llvm-project#156336.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the
LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORScmake option and the-treat-scalable-fixed-error-as-warningopt flag.We stopped treating these as warnings by default a long time ago (62f09d7), so I don't think it makes sense to retain these options at this point. Accessing a scalable TypeSize as fixed should always result in an error.