[llvm] r333576 - [GlobalISel][Legalizer] LegalizerInfo verifier: check rules cover type indices
Roman Tereshin via llvm-commits
llvm-commits at lists.llvm.org
Wed May 30 11:45:32 PDT 2018
Author: rtereshin
Date: Wed May 30 11:45:32 2018
New Revision: 333576
URL: http://llvm.org/viewvc/llvm-project?rev=333576&view=rev
Log:
[GlobalISel][Legalizer] LegalizerInfo verifier: check rules cover type indices
This commit adds a simple verifier that tracks type indices being
touched by legalization rules' builders.
Every target will now have an opportunity to call
LegalizerInfo::verify(...) at the end of its derived LegalizerInfo's
constructor and check there are no obvious mistakes like checking only
first type for an opcode that has more than one type index and therefore
implicitly declaring any type for the second (and higher) type index
legal.
The check is only ran in assert builds and should have very minor
performance impact in assert builds and none in release builds.
This commit does not add LegalizerInfo::verify(...) calls to
target-specific legalizers, look for separate commits for that.
This commit also doesn't make the verification errors fatal, only
produces an error message, look for a later commit that does.
Reviewers: aemerson, qcolombet
Reviewed By: aemerson
Differential Revision: https://reviews.llvm.org/D46338
Modified:
llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp
Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h?rev=333576&r1=333575&r2=333576&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h Wed May 30 11:45:32 2018
@@ -19,6 +19,7 @@
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/TargetOpcodes.h"
@@ -37,6 +38,7 @@ extern cl::opt<bool> DisableGISelLegalit
class MachineInstr;
class MachineIRBuilder;
class MachineRegisterInfo;
+class MCInstrInfo;
namespace LegalizeActions {
enum LegalizeAction : std::uint8_t {
@@ -253,6 +255,33 @@ class LegalizeRuleSet {
bool IsAliasedByAnother;
SmallVector<LegalizeRule, 2> Rules;
+#ifndef NDEBUG
+ /// If bit I is set, this rule set contains a rule that may handle (predicate
+ /// or perform an action upon (or both)) the type index I. The uncertainty
+ /// comes from free-form rules executing user-provided lambda functions. We
+ /// conservatively assume such rules do the right thing and cover all type
+ /// indices. The bitset is intentionally 1 bit wider than it absolutely needs
+ /// to be to distinguish such cases from the cases where all type indices are
+ /// individually handled.
+ SmallBitVector TypeIdxsCovered{MCOI::OPERAND_LAST_GENERIC -
+ MCOI::OPERAND_FIRST_GENERIC + 2};
+#endif
+
+ unsigned typeIdx(unsigned TypeIdx) {
+ assert(TypeIdx <=
+ (MCOI::OPERAND_LAST_GENERIC - MCOI::OPERAND_FIRST_GENERIC) &&
+ "Type Index is out of bounds");
+#ifndef NDEBUG
+ TypeIdxsCovered.set(TypeIdx);
+#endif
+ return TypeIdx;
+ }
+ void markAllTypeIdxsAsCovered() {
+#ifndef NDEBUG
+ TypeIdxsCovered.set();
+#endif
+ }
+
void add(const LegalizeRule &Rule) {
assert(AliasOf == 0 &&
"RuleSet is aliased, change the representative opcode instead");
@@ -280,7 +309,7 @@ class LegalizeRuleSet {
LegalizeRuleSet &actionFor(LegalizeAction Action,
std::initializer_list<LLT> Types) {
using namespace LegalityPredicates;
- return actionIf(Action, typeInSet(0, Types));
+ return actionIf(Action, typeInSet(typeIdx(0), Types));
}
/// Use the given action when type index 0 is any type in the given list.
/// Action should be an action that requires mutation.
@@ -288,7 +317,7 @@ class LegalizeRuleSet {
std::initializer_list<LLT> Types,
LegalizeMutation Mutation) {
using namespace LegalityPredicates;
- return actionIf(Action, typeInSet(0, Types), Mutation);
+ return actionIf(Action, typeInSet(typeIdx(0), Types), Mutation);
}
/// Use the given action when type indexes 0 and 1 is any type pair in the
/// given list.
@@ -296,7 +325,7 @@ class LegalizeRuleSet {
LegalizeRuleSet &actionFor(LegalizeAction Action,
std::initializer_list<std::pair<LLT, LLT>> Types) {
using namespace LegalityPredicates;
- return actionIf(Action, typePairInSet(0, 1, Types));
+ return actionIf(Action, typePairInSet(typeIdx(0), typeIdx(1), Types));
}
/// Use the given action when type indexes 0 and 1 is any type pair in the
/// given list.
@@ -305,7 +334,8 @@ class LegalizeRuleSet {
std::initializer_list<std::pair<LLT, LLT>> Types,
LegalizeMutation Mutation) {
using namespace LegalityPredicates;
- return actionIf(Action, typePairInSet(0, 1, Types), Mutation);
+ return actionIf(Action, typePairInSet(typeIdx(0), typeIdx(1), Types),
+ Mutation);
}
/// Use the given action when type indexes 0 and 1 are both in the given list.
/// That is, the type pair is in the cartesian product of the list.
@@ -313,7 +343,8 @@ class LegalizeRuleSet {
LegalizeRuleSet &actionForCartesianProduct(LegalizeAction Action,
std::initializer_list<LLT> Types) {
using namespace LegalityPredicates;
- return actionIf(Action, all(typeInSet(0, Types), typeInSet(1, Types)));
+ return actionIf(Action, all(typeInSet(typeIdx(0), Types),
+ typeInSet(typeIdx(1), Types)));
}
/// Use the given action when type indexes 0 and 1 are both in their
/// respective lists.
@@ -324,7 +355,20 @@ class LegalizeRuleSet {
std::initializer_list<LLT> Types0,
std::initializer_list<LLT> Types1) {
using namespace LegalityPredicates;
- return actionIf(Action, all(typeInSet(0, Types0), typeInSet(1, Types1)));
+ return actionIf(Action, all(typeInSet(typeIdx(0), Types0),
+ typeInSet(typeIdx(1), Types1)));
+ }
+ /// Use the given action when type indexes 0, 1, and 2 are all in their
+ /// respective lists.
+ /// That is, the type triple is in the cartesian product of the lists
+ /// Action should not be an action that requires mutation.
+ LegalizeRuleSet &actionForCartesianProduct(
+ LegalizeAction Action, std::initializer_list<LLT> Types0,
+ std::initializer_list<LLT> Types1, std::initializer_list<LLT> Types2) {
+ using namespace LegalityPredicates;
+ return actionIf(Action, all(typeInSet(typeIdx(0), Types0),
+ all(typeInSet(typeIdx(1), Types1),
+ typeInSet(typeIdx(2), Types2))));
}
public:
@@ -342,6 +386,9 @@ public:
/// The instruction is legal if predicate is true.
LegalizeRuleSet &legalIf(LegalityPredicate Predicate) {
+ // We have no choice but conservatively assume that the free-form
+ // user-provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::Legal, Predicate);
}
/// The instruction is legal when type index 0 is any type in the given list.
@@ -360,7 +407,7 @@ public:
TypesAndMemSize) {
return actionIf(LegalizeAction::Legal,
LegalityPredicates::typePairAndMemSizeInSet(
- 0, 1, /*MMOIdx*/ 0, TypesAndMemSize));
+ typeIdx(0), typeIdx(1), /*MMOIdx*/ 0, TypesAndMemSize));
}
/// The instruction is legal when type indexes 0 and 1 are both in the given
/// list. That is, the type pair is in the cartesian product of the list.
@@ -377,17 +424,26 @@ public:
/// The instruction is lowered.
LegalizeRuleSet &lower() {
using namespace LegalizeMutations;
+ // We have no choice but conservatively assume that predicate-less lowering
+ // properly handles all type indices by design:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::Lower, always);
}
/// The instruction is lowered if predicate is true. Keep type index 0 as the
/// same type.
LegalizeRuleSet &lowerIf(LegalityPredicate Predicate) {
using namespace LegalizeMutations;
+ // We have no choice but conservatively assume that lowering with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::Lower, Predicate);
}
/// The instruction is lowered if predicate is true.
LegalizeRuleSet &lowerIf(LegalityPredicate Predicate,
LegalizeMutation Mutation) {
+ // We have no choice but conservatively assume that lowering with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::Lower, Predicate, Mutation);
}
/// The instruction is lowered when type index 0 is any type in the given
@@ -414,9 +470,28 @@ public:
LegalizeMutation Mutation) {
return actionFor(LegalizeAction::Lower, Types, Mutation);
}
+ /// The instruction is lowered when type indexes 0 and 1 are both in their
+ /// respective lists.
+ LegalizeRuleSet &lowerForCartesianProduct(std::initializer_list<LLT> Types0,
+ std::initializer_list<LLT> Types1) {
+ using namespace LegalityPredicates;
+ return actionForCartesianProduct(LegalizeAction::Lower, Types0, Types1);
+ }
+ /// The instruction is lowered when when type indexes 0, 1, and 2 are all in
+ /// their respective lists.
+ LegalizeRuleSet &lowerForCartesianProduct(std::initializer_list<LLT> Types0,
+ std::initializer_list<LLT> Types1,
+ std::initializer_list<LLT> Types2) {
+ using namespace LegalityPredicates;
+ return actionForCartesianProduct(LegalizeAction::Lower, Types0, Types1,
+ Types2);
+ }
/// Like legalIf, but for the Libcall action.
LegalizeRuleSet &libcallIf(LegalityPredicate Predicate) {
+ // We have no choice but conservatively assume that a libcall with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::Libcall, Predicate);
}
LegalizeRuleSet &libcallFor(std::initializer_list<LLT> Types) {
@@ -440,12 +515,18 @@ public:
/// true.
LegalizeRuleSet &widenScalarIf(LegalityPredicate Predicate,
LegalizeMutation Mutation) {
+ // We have no choice but conservatively assume that an action with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::WidenScalar, Predicate, Mutation);
}
/// Narrow the scalar to the one selected by the mutation if the predicate is
/// true.
LegalizeRuleSet &narrowScalarIf(LegalityPredicate Predicate,
LegalizeMutation Mutation) {
+ // We have no choice but conservatively assume that an action with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::NarrowScalar, Predicate, Mutation);
}
@@ -453,12 +534,18 @@ public:
/// predicate is true.
LegalizeRuleSet &moreElementsIf(LegalityPredicate Predicate,
LegalizeMutation Mutation) {
+ // We have no choice but conservatively assume that an action with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::MoreElements, Predicate, Mutation);
}
/// Remove elements to reach the type selected by the mutation if the
/// predicate is true.
LegalizeRuleSet &fewerElementsIf(LegalityPredicate Predicate,
LegalizeMutation Mutation) {
+ // We have no choice but conservatively assume that an action with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::FewerElements, Predicate, Mutation);
}
@@ -475,6 +562,9 @@ public:
}
LegalizeRuleSet &customIf(LegalityPredicate Predicate) {
+ // We have no choice but conservatively assume that a custom action with a
+ // free-form user provided Predicate properly handles all type indices:
+ markAllTypeIdxsAsCovered();
return actionIf(LegalizeAction::Custom, Predicate);
}
LegalizeRuleSet &customFor(std::initializer_list<LLT> Types) {
@@ -494,13 +584,14 @@ public:
LegalizeRuleSet &widenScalarToNextPow2(unsigned TypeIdx,
unsigned MinSize = 0) {
using namespace LegalityPredicates;
- return actionIf(LegalizeAction::WidenScalar, sizeNotPow2(TypeIdx),
+ return actionIf(LegalizeAction::WidenScalar, sizeNotPow2(typeIdx(TypeIdx)),
LegalizeMutations::widenScalarToNextPow2(TypeIdx, MinSize));
}
LegalizeRuleSet &narrowScalar(unsigned TypeIdx, LegalizeMutation Mutation) {
using namespace LegalityPredicates;
- return actionIf(LegalizeAction::NarrowScalar, isScalar(TypeIdx), Mutation);
+ return actionIf(LegalizeAction::NarrowScalar, isScalar(typeIdx(TypeIdx)),
+ Mutation);
}
/// Ensure the scalar is at least as wide as Ty.
@@ -509,7 +600,7 @@ public:
using namespace LegalizeMutations;
return actionIf(LegalizeAction::WidenScalar,
narrowerThan(TypeIdx, Ty.getSizeInBits()),
- changeTo(TypeIdx, Ty));
+ changeTo(typeIdx(TypeIdx), Ty));
}
/// Ensure the scalar is at most as wide as Ty.
@@ -518,7 +609,7 @@ public:
using namespace LegalizeMutations;
return actionIf(LegalizeAction::NarrowScalar,
widerThan(TypeIdx, Ty.getSizeInBits()),
- changeTo(TypeIdx, Ty));
+ changeTo(typeIdx(TypeIdx), Ty));
}
/// Conditionally limit the maximum size of the scalar.
@@ -533,7 +624,7 @@ public:
return widerThan(TypeIdx, Ty.getSizeInBits()) &&
Predicate(Query);
},
- changeTo(TypeIdx, Ty));
+ changeTo(typeIdx(TypeIdx), Ty));
}
/// Limit the range of scalar sizes to MinTy and MaxTy.
@@ -548,13 +639,16 @@ public:
/// two.
LegalizeRuleSet &moreElementsToNextPow2(unsigned TypeIdx) {
using namespace LegalityPredicates;
- return actionIf(LegalizeAction::MoreElements, numElementsNotPow2(TypeIdx),
+ return actionIf(LegalizeAction::MoreElements,
+ numElementsNotPow2(typeIdx(TypeIdx)),
LegalizeMutations::moreElementsToNextPow2(TypeIdx));
}
/// Limit the number of elements in EltTy vectors to at least MinElements.
LegalizeRuleSet &clampMinNumElements(unsigned TypeIdx, const LLT &EltTy,
unsigned MinElements) {
+ // Mark the type index as covered:
+ typeIdx(TypeIdx);
return actionIf(
LegalizeAction::MoreElements,
[=](const LegalityQuery &Query) {
@@ -571,6 +665,8 @@ public:
/// Limit the number of elements in EltTy vectors to at most MaxElements.
LegalizeRuleSet &clampMaxNumElements(unsigned TypeIdx, const LLT &EltTy,
unsigned MaxElements) {
+ // Mark the type index as covered:
+ typeIdx(TypeIdx);
return actionIf(
LegalizeAction::FewerElements,
[=](const LegalityQuery &Query) {
@@ -607,6 +703,11 @@ public:
return *this;
}
+ /// Check if there is no type index which is obviously not handled by the
+ /// LegalizeRuleSet in any way at all.
+ /// \pre Type indices of the opcode form a dense [0, \p NumTypeIdxs) set.
+ bool verifyTypeIdxsCoverage(unsigned NumTypeIdxs) const;
+
/// Apply the ruleset to the given LegalityQuery.
LegalizeActionStep apply(const LegalityQuery &Query) const;
};
@@ -624,6 +725,10 @@ public:
/// before any query is made or incorrect results may be returned.
void computeTables();
+ /// Perform simple self-diagnostic and assert if there is anything obviously
+ /// wrong with the actions set up.
+ void verify(const MCInstrInfo &MII) const;
+
static bool needsLegalizingToDifferentSize(const LegalizeAction Action) {
using namespace LegalizeActions;
switch (Action) {
Modified: llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp?rev=333576&r1=333575&r2=333576&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/LegalizerInfo.cpp Wed May 30 11:45:32 2018
@@ -24,6 +24,7 @@
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/TargetOpcodes.h"
#include "llvm/MC/MCInstrDesc.h"
+#include "llvm/MC/MCInstrInfo.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/LowLevelTypeImpl.h"
@@ -83,6 +84,28 @@ LegalizeActionStep LegalizeRuleSet::appl
return {LegalizeAction::Unsupported, 0, LLT{}};
}
+bool LegalizeRuleSet::verifyTypeIdxsCoverage(unsigned NumTypeIdxs) const {
+#ifndef NDEBUG
+ if (Rules.empty()) {
+ LLVM_DEBUG(
+ dbgs() << ".. type index coverage check SKIPPED: no rules defined\n");
+ return true;
+ }
+ const int64_t FirstUncovered = TypeIdxsCovered.find_first_unset();
+ if (FirstUncovered < 0) {
+ LLVM_DEBUG(dbgs() << ".. type index coverage check SKIPPED:"
+ " user-defined predicate detected\n");
+ return true;
+ }
+ const bool AllCovered = (FirstUncovered >= NumTypeIdxs);
+ LLVM_DEBUG(dbgs() << ".. the first uncovered type index: " << FirstUncovered
+ << ", " << (AllCovered ? "OK" : "FAIL") << "\n");
+ return AllCovered;
+#else
+ return true;
+#endif
+}
+
LegalizerInfo::LegalizerInfo() : TablesInitialized(false) {
// Set defaults.
// FIXME: these two (G_ANYEXT and G_TRUNC?) can be legalized to the
@@ -516,6 +539,35 @@ LegalizerInfo::findVectorLegalAction(con
IntermediateType.getScalarSizeInBits())};
}
+/// \pre Type indices of every opcode form a dense set starting from 0.
+void LegalizerInfo::verify(const MCInstrInfo &MII) const {
+#ifndef NDEBUG
+ std::vector<unsigned> FailedOpcodes;
+ for (unsigned Opcode = FirstOp; Opcode <= LastOp; ++Opcode) {
+ const MCInstrDesc &MCID = MII.get(Opcode);
+ const unsigned NumTypeIdxs = std::accumulate(
+ MCID.opInfo_begin(), MCID.opInfo_end(), 0U,
+ [](unsigned Acc, const MCOperandInfo &OpInfo) {
+ return OpInfo.isGenericType()
+ ? std::max(OpInfo.getGenericTypeIndex() + 1U, Acc)
+ : Acc;
+ });
+ LLVM_DEBUG(dbgs() << MII.getName(Opcode) << " (opcode " << Opcode
+ << "): " << NumTypeIdxs << " type ind"
+ << (NumTypeIdxs == 1 ? "ex" : "ices") << "\n");
+ const LegalizeRuleSet &RuleSet = getActionDefinitions(Opcode);
+ if (!RuleSet.verifyTypeIdxsCoverage(NumTypeIdxs))
+ FailedOpcodes.push_back(Opcode);
+ }
+ if (!FailedOpcodes.empty()) {
+ errs() << "The following opcodes have ill-defined legalization rules:";
+ for (unsigned Opcode : FailedOpcodes)
+ errs() << " " << MII.getName(Opcode);
+ errs() << "\n";
+ }
+#endif
+}
+
#ifndef NDEBUG
// FIXME: This should be in the MachineVerifier, but it can't use the
// LegalizerInfo as it's currently in the separate GlobalISel library.
More information about the llvm-commits
mailing list