[clang] 0f7bbbc - Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.
Sander de Smalen via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 2 03:22:55 PDT 2021
Author: Sander de Smalen
Date: 2021-04-02T10:55:22+01:00
New Revision: 0f7bbbc481e20a152c74bc315f8995b62d54c8c0
URL: https://github.com/llvm/llvm-project/commit/0f7bbbc481e20a152c74bc315f8995b62d54c8c0
DIFF: https://github.com/llvm/llvm-project/commit/0f7bbbc481e20a152c74bc315f8995b62d54c8c0.diff
LOG: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.
In order to bring up scalable vector support in LLVM incrementally,
we introduced behaviour to emit a warning, instead of an error, when
asking the wrong question of a scalable vector, like asking for the
fixed number of elements.
This patch puts that behaviour under a flag. The default behaviour is
that the compiler will always error, which means that all LLVM unit
tests and regression tests will now fail when a code-path is taken that
still uses the wrong interface.
The behaviour to demote an error to a warning can be individually enabled
for tools that want to support experimental use of scalable vectors.
This patch enables that behaviour when driving compilation from Clang.
This means that for users who want to try out scalable-vector support,
fixed-width codegen support, or build user-code with scalable vector
intrinsics, Clang will not crash and burn when the compiler encounters
such a case.
This allows us to do away with the following pattern in many of the SVE tests:
RUN: .... 2>%t
RUN: cat %t | FileCheck --check-prefix=WARN
WARN-NOT: warning: ...
The behaviour to emit warnings is only temporary and we expect this flag
to be removed in the future when scalable vector support is more stable.
This patch also has fixes the following tests:
unittests:
ScalableVectorMVTsTest.SizeQueries
SelectionDAGAddressAnalysisTest.unknownSizeFrameObjects
AArch64SelectionDAGTest.computeKnownBitsSVE_ZERO_EXTEND_VECTOR_INREG
regression tests:
Transforms/InstCombine/vscale_gep.ll
Reviewed By: paulwalker-arm, ctetreau
Differential Revision: https://reviews.llvm.org/D98856
Added:
llvm/lib/Support/TypeSize.cpp
Modified:
clang/lib/Driver/ToolChains/Clang.cpp
llvm/include/llvm/CodeGen/ValueTypes.h
llvm/include/llvm/Support/TypeSize.h
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
llvm/lib/Support/CMakeLists.txt
llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 30a0b090b6922..3d5cdb3acd244 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5051,6 +5051,21 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
RenderTargetOptions(Triple, Args, KernelOrKext, CmdArgs);
+ // FIXME: For now we want to demote any errors to warnings, when they have
+ // been raised for asking the wrong question of scalable vectors, such as
+ // asking for the fixed number of elements. This may happen because code that
+ // is not yet ported to work for scalable vectors uses the wrong interfaces,
+ // whereas the behaviour is actually correct. Emitting a warning helps bring
+ // up scalable vector support in an incremental way. When scalable vector
+ // support is stable enough, all uses of wrong interfaces should be considered
+ // as errors, but until then, we can live with a warning being emitted by the
+ // compiler. This way, Clang can be used to compile code with scalable vectors
+ // and identify possible issues.
+ if (isa<BackendJobAction>(JA)) {
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("-treat-scalable-fixed-error-as-warning");
+ }
+
// These two are potentially updated by AddClangCLArgs.
codegenoptions::DebugInfoKind DebugInfoKind = codegenoptions::NoDebugInfo;
bool EmitCodeView = false;
diff --git a/llvm/include/llvm/CodeGen/ValueTypes.h b/llvm/include/llvm/CodeGen/ValueTypes.h
index ccdaab5ed96d2..e7346f7a75abc 100644
--- a/llvm/include/llvm/CodeGen/ValueTypes.h
+++ b/llvm/include/llvm/CodeGen/ValueTypes.h
@@ -299,19 +299,16 @@ namespace llvm {
/// Given a vector type, return the number of elements it contains.
unsigned getVectorNumElements() const {
-#ifdef STRICT_FIXED_SIZE_VECTORS
- assert(isFixedLengthVector() && "Invalid vector type!");
-#else
assert(isVector() && "Invalid vector type!");
+
if (isScalableVector())
- WithColor::warning()
- << "Possible incorrect use of EVT::getVectorNumElements() for "
- "scalable vector. Scalable flag may be dropped, use "
- "EVT::getVectorElementCount() instead\n";
-#endif
- if (isSimple())
- return V.getVectorNumElements();
- return getExtendedVectorNumElements();
+ llvm::reportInvalidSizeRequest(
+ "Possible incorrect use of EVT::getVectorNumElements() for "
+ "scalable vector. Scalable flag may be dropped, use "
+ "EVT::getVectorElementCount() instead");
+
+ return isSimple() ? V.getVectorNumElements()
+ : getExtendedVectorNumElements();
}
// Given a (possibly scalable) vector type, return the ElementCount
diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 72589693a4df4..30bbbd7db8c97 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -27,6 +27,10 @@
namespace llvm {
+/// Reports a diagnostic message to indicate an invalid size request has been
+/// done on a scalable vector. This function may not return.
+void reportInvalidSizeRequest(const char *Msg);
+
template <typename LeafTy> struct LinearPolyBaseTypeTraits {};
//===----------------------------------------------------------------------===//
@@ -446,17 +450,7 @@ class TypeSize : public LinearPolySize<TypeSize> {
// else
// bail out early for scalable vectors and use getFixedValue()
// }
- operator ScalarTy() const {
-#ifdef STRICT_FIXED_SIZE_VECTORS
- return getFixedValue();
-#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 getKnownMinValue();
-#endif
- }
+ operator ScalarTy() const;
// Additional operators needed to avoid ambiguous parses
// because of the implicit conversion hack.
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 80c2a63725adc..23bb5e6545920 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -4332,7 +4332,10 @@ static Value *SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops,
if (Q.isUndefValue(Ops[0]))
return UndefValue::get(GEPTy);
- bool IsScalableVec = isa<ScalableVectorType>(SrcTy);
+ bool IsScalableVec =
+ isa<ScalableVectorType>(SrcTy) || any_of(Ops, [](const Value *V) {
+ return isa<ScalableVectorType>(V->getType());
+ });
if (Ops.size() == 2) {
// getelementptr P, 0 -> P.
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index a60610dfccf34..943e06e7286c5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -4847,8 +4847,8 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
assert(VT.isVector() && "This DAG node is restricted to vector types.");
assert(Operand.getValueType().bitsLE(VT) &&
"The input must be the same size or smaller than the result.");
- assert(VT.getVectorNumElements() <
- Operand.getValueType().getVectorNumElements() &&
+ assert(VT.getVectorMinNumElements() <
+ Operand.getValueType().getVectorMinNumElements() &&
"The destination vector type must have fewer lanes than the input.");
break;
case ISD::ABS:
diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt
index 33caf5833728e..ddacb4feaa0f2 100644
--- a/llvm/lib/Support/CMakeLists.txt
+++ b/llvm/lib/Support/CMakeLists.txt
@@ -185,6 +185,7 @@ add_llvm_component_library(LLVMSupport
TrigramIndex.cpp
Triple.cpp
Twine.cpp
+ TypeSize.cpp
Unicode.cpp
UnicodeCaseFold.cpp
VersionTuple.cpp
diff --git a/llvm/lib/Support/TypeSize.cpp b/llvm/lib/Support/TypeSize.cpp
new file mode 100644
index 0000000000000..83d40d9cb0edd
--- /dev/null
+++ b/llvm/lib/Support/TypeSize.cpp
@@ -0,0 +1,41 @@
+//===- TypeSize.cpp - Wrapper around type sizes------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/TypeSize.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+
+/// The ScalableErrorAsWarning is a temporary measure to suppress errors from
+/// using the wrong interface on a scalable vector.
+cl::opt<bool> ScalableErrorAsWarning(
+ "treat-scalable-fixed-error-as-warning", cl::Hidden, cl::init(false),
+ cl::desc("Treat issues where a fixed-width property is requested from a "
+ "scalable type as a warning, instead of an error."),
+ cl::ZeroOrMore);
+
+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(
+ "Cannot implicitly convert a scalable size to a fixed-width size in "
+ "`TypeSize::operator ScalarTy()`");
+ return getKnownMinValue();
+ }
+ return getFixedValue();
+}
diff --git a/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp b/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
index 566b2e75c70c5..e38d1045586cb 100644
--- a/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ b/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -180,7 +180,8 @@ TEST(ScalableVectorMVTsTest, SizeQueries) {
// Check convenience size scaling methods.
EXPECT_EQ(v2i32.getSizeInBits() * 2, v4i32.getSizeInBits());
EXPECT_EQ(2 * nxv2i32.getSizeInBits(), nxv4i32.getSizeInBits());
- EXPECT_EQ(nxv2f64.getSizeInBits() / 2, nxv2i32.getSizeInBits());
+ EXPECT_EQ(nxv2f64.getSizeInBits().divideCoefficientBy(2),
+ nxv2i32.getSizeInBits());
}
} // end anonymous namespace
More information about the cfe-commits
mailing list