sdesmalen created this revision.
Herald added subscribers: dexonsmith, hiraditya, kristof.beyls, mgorny.
sdesmalen requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D98856
Files:
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/CodeGen/ValueTypes.cpp
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/TypeSize.cpp
llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
Index: llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
===================================================================
--- llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
+++ llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp
@@ -169,10 +169,10 @@
TypeSize Offset1 = SubVecVT.getStoreSize();
SDValue Index0 = DAG->getMemBasePlusOffset(FIPtr, Offset0, Loc);
SDValue Index1 = DAG->getMemBasePlusOffset(FIPtr, Offset1, Loc);
- SDValue Store0 = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index0,
- PtrInfo.getWithOffset(Offset0));
+ SDValue Store0 =
+ DAG->getStore(DAG->getEntryNode(), Loc, Value, Index0, PtrInfo);
SDValue Store1 = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index1,
- PtrInfo.getWithOffset(Offset1));
+ MachinePointerInfo(PtrInfo.getAddrSpace()));
Optional<int64_t> NumBytes0 = MemoryLocation::getSizeOrUnknown(
cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize());
Optional<int64_t> NumBytes1 = MemoryLocation::getSizeOrUnknown(
Index: llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
===================================================================
--- llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -180,7 +180,8 @@
// 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
Index: llvm/lib/Support/TypeSize.cpp
===================================================================
--- /dev/null
+++ llvm/lib/Support/TypeSize.cpp
@@ -0,0 +1,39 @@
+//===- 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;
+
+cl::opt<bool> llvm::TypeSizeClOpt::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);
+
+TypeSize::operator TypeSize::ScalarTy() const {
+ auto Error = []() {
+ report_fatal_error("Cannot convert a scalable size to a fixed-width size!");
+ };
+#ifdef STRICT_FIXED_SIZE_VECTORS
+ if (isScalable())
+ Error();
+#else
+ if (isScalable()) {
+ if (llvm::TypeSizeClOpt::ScalableErrorAsWarning) {
+ WithColor::warning() << "Compiler has made implicit assumption that "
+ "TypeSize is not scalable. This may or may not "
+ "lead to broken code.\n";
+ return getKnownMinValue();
+ } else
+ Error();
+ }
+ return getFixedValue();
+#endif
+}
Index: llvm/lib/Support/CMakeLists.txt
===================================================================
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -185,6 +185,7 @@
TrigramIndex.cpp
Triple.cpp
Twine.cpp
+ TypeSize.cpp
Unicode.cpp
UnicodeCaseFold.cpp
VersionTuple.cpp
Index: llvm/lib/CodeGen/ValueTypes.cpp
===================================================================
--- llvm/lib/CodeGen/ValueTypes.cpp
+++ llvm/lib/CodeGen/ValueTypes.cpp
@@ -14,6 +14,31 @@
#include "llvm/Support/TypeSize.h"
using namespace llvm;
+unsigned EVT::getVectorNumElements() const {
+ auto Error = []() {
+ report_fatal_error(
+ "Cannot return the exact number of elements of a scalable vector!");
+ };
+#ifdef STRICT_FIXED_SIZE_VECTORS
+ if (isScalableVector())
+ Error();
+#else
+ assert(isVector() && "Invalid vector type!");
+ if (isScalableVector()) {
+ if (llvm::TypeSizeClOpt::ScalableErrorAsWarning)
+ WithColor::warning()
+ << "Possible incorrect use of EVT::getVectorNumElements() for "
+ "scalable vector. Scalable flag may be dropped, use "
+ "EVT::getVectorElementCount() instead\n";
+ else
+ Error();
+ }
+#endif
+ if (isSimple())
+ return V.getVectorNumElements();
+ return getExtendedVectorNumElements();
+}
+
EVT EVT::changeExtendedTypeToInteger() const {
assert(isExtended() && "Type is not extended!");
LLVMContext &Context = LLVMTy->getContext();
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -4777,8 +4777,8 @@
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:
Index: llvm/lib/Analysis/InstructionSimplify.cpp
===================================================================
--- llvm/lib/Analysis/InstructionSimplify.cpp
+++ llvm/lib/Analysis/InstructionSimplify.cpp
@@ -4329,7 +4329,10 @@
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.
Index: llvm/include/llvm/Support/TypeSize.h
===================================================================
--- llvm/include/llvm/Support/TypeSize.h
+++ llvm/include/llvm/Support/TypeSize.h
@@ -16,6 +16,7 @@
#define LLVM_SUPPORT_TYPESIZE_H
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/WithColor.h"
@@ -27,6 +28,12 @@
namespace llvm {
+namespace TypeSizeClOpt {
+/// The ScalableErrorAsWarning is a temporary measure to suppress errors from
+/// using the wrong interface.
+extern cl::opt<bool> ScalableErrorAsWarning;
+} // namespace TypeSizeClOpt
+
template <typename LeafTy> struct LinearPolyBaseTypeTraits {};
//===----------------------------------------------------------------------===//
@@ -446,17 +453,7 @@
// 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.
Index: llvm/include/llvm/CodeGen/ValueTypes.h
===================================================================
--- llvm/include/llvm/CodeGen/ValueTypes.h
+++ llvm/include/llvm/CodeGen/ValueTypes.h
@@ -298,21 +298,7 @@
}
/// 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();
- }
+ unsigned getVectorNumElements() const;
// Given a (possibly scalable) vector type, return the ElementCount
ElementCount getVectorElementCount() const {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5039,6 +5039,21 @@
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
