Skip to content

Commit 2b94972

Browse files
committed
[GlobalISel][AArch64] Adding -disable-gisel-legality-check CL option
Currently it's impossible to test InstructionSelect pass with MIR which is considered illegal by the Legalizer in Assert builds. In early stages of porting an existing backend from SelectionDAG ISel to GlobalISel, however, we would have very basic CallLowering, Legalizer, and RegBankSelect implementations, but rather functional Instruction Select with quite a few patterns selectable due to the semi-automatic porting process borrowing them from SelectionDAG ISel. As we are trying to define legality as a property of being selectable by the instruction selector, it would be nice to be able to easily check what the selector can do in its current state w/o the legality check provided by the Legalizer getting in the way. It also seems beneficial to have a regression testing set up that would not allow the selector to silently regress in its support of the MIR not supported yet by the previous passes in the GlobalISel pipeline. This commit adds -disable-gisel-legality-check command line option to llc that disables those legality checks in RegBankSelect and InstructionSelect passes. It also adds quite a few MIR test cases for AArch64's Instruction Selector. Every one of them would fail on the legality check at the moment, but will select just fine if the check is disabled. Every test MachineFunction is intended to exercise a specific selection rule and that rule only, encoded in the MachineFunction's name by the rule's number, ID, and index of its GIM_Try opcode in TableGen'erated MatchTable (-optimize-match-table=false). Reviewers: ab, dsanders, qcolombet, rovka Reviewed By: bogner Subscribers: kristof.beyls, volkan, aditya_nandakumar, aemerson, rengolin, t.p.northover, javed.absar, llvm-commits Differential Revision: https://reviews.llvm.org/D42886 llvm-svn: 326396
1 parent 1e0116c commit 2b94972

File tree

5 files changed

+4589
-27
lines changed

5 files changed

+4589
-27
lines changed

llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "llvm/ADT/Optional.h"
2121
#include "llvm/ADT/STLExtras.h"
2222
#include "llvm/ADT/SmallVector.h"
23+
#include "llvm/CodeGen/MachineFunction.h"
2324
#include "llvm/CodeGen/TargetOpcodes.h"
2425
#include "llvm/Support/raw_ostream.h"
2526
#include "llvm/Support/LowLevelTypeImpl.h"
@@ -31,6 +32,8 @@
3132

3233
namespace llvm {
3334

35+
extern cl::opt<bool> DisableGISelLegalityCheck;
36+
3437
class MachineInstr;
3538
class MachineIRBuilder;
3639
class MachineRegisterInfo;
@@ -906,6 +909,12 @@ class LegalizerInfo {
906909
LegalizeRuleSet RulesForOpcode[LastOp - FirstOp + 1];
907910
};
908911

912+
#ifndef NDEBUG
913+
/// Checks that MIR is fully legal, returns an illegal instruction if it's not,
914+
/// nullptr otherwise
915+
const MachineInstr *machineFunctionIsIllegal(const MachineFunction &MF);
916+
#endif
917+
909918
} // end namespace llvm.
910919

911920
#endif // LLVM_CODEGEN_GLOBALISEL_LEGALIZERINFO_H

llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -81,20 +81,14 @@ bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {
8181
#ifndef NDEBUG
8282
// Check that our input is fully legal: we require the function to have the
8383
// Legalized property, so it should be.
84-
// FIXME: This should be in the MachineVerifier, but it can't use the
85-
// LegalizerInfo as it's currently in the separate GlobalISel library.
86-
// The RegBankSelected property is already checked in the verifier. Note
87-
// that it has the same layering problem, but we only use inline methods so
88-
// end up not needing to link against the GlobalISel library.
89-
if (const LegalizerInfo *MLI = MF.getSubtarget().getLegalizerInfo())
90-
for (MachineBasicBlock &MBB : MF)
91-
for (MachineInstr &MI : MBB)
92-
if (isPreISelGenericOpcode(MI.getOpcode()) && !MLI->isLegal(MI, MRI)) {
93-
reportGISelFailure(MF, TPC, MORE, "gisel-select",
94-
"instruction is not legal", MI);
95-
return false;
96-
}
97-
84+
// FIXME: This should be in the MachineVerifier, as the RegBankSelected
85+
// property check already is.
86+
if (!DisableGISelLegalityCheck)
87+
if (const MachineInstr *MI = machineFunctionIsIllegal(MF)) {
88+
reportGISelFailure(MF, TPC, MORE, "gisel-select",
89+
"instruction is not legal", *MI);
90+
return false;
91+
}
9892
#endif
9993
// FIXME: We could introduce new blocks and will need to fix the outer loop.
10094
// Until then, keep track of the number of blocks to assert that we don't.

llvm/lib/CodeGen/GlobalISel/LegalizerInfo.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ using namespace LegalizeActions;
3636

3737
#define DEBUG_TYPE "legalizer-info"
3838

39+
cl::opt<bool> llvm::DisableGISelLegalityCheck(
40+
"disable-gisel-legality-check",
41+
cl::desc("Don't verify that MIR is fully legal between GlobalISel passes"),
42+
cl::Hidden);
43+
3944
raw_ostream &LegalityQuery::print(raw_ostream &OS) const {
4045
OS << Opcode << ", {";
4146
for (const auto &Type : Types) {
@@ -495,3 +500,21 @@ LegalizerInfo::findVectorLegalAction(const InstrAspect &Aspect) const {
495500
LLT::vector(NumElementsAndAction.first,
496501
IntermediateType.getScalarSizeInBits())};
497502
}
503+
504+
#ifndef NDEBUG
505+
// FIXME: This should be in the MachineVerifier, but it can't use the
506+
// LegalizerInfo as it's currently in the separate GlobalISel library.
507+
// Note that RegBankSelected property already checked in the verifier
508+
// has the same layering problem, but we only use inline methods so
509+
// end up not needing to link against the GlobalISel library.
510+
const MachineInstr *llvm::machineFunctionIsIllegal(const MachineFunction &MF) {
511+
if (const LegalizerInfo *MLI = MF.getSubtarget().getLegalizerInfo()) {
512+
const MachineRegisterInfo &MRI = MF.getRegInfo();
513+
for (const MachineBasicBlock &MBB : MF)
514+
for (const MachineInstr &MI : MBB)
515+
if (isPreISelGenericOpcode(MI.getOpcode()) && !MLI->isLegal(MI, MRI))
516+
return &MI;
517+
}
518+
return nullptr;
519+
}
520+
#endif

llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -610,20 +610,13 @@ bool RegBankSelect::runOnMachineFunction(MachineFunction &MF) {
610610
#ifndef NDEBUG
611611
// Check that our input is fully legal: we require the function to have the
612612
// Legalized property, so it should be.
613-
// FIXME: This should be in the MachineVerifier, but it can't use the
614-
// LegalizerInfo as it's currently in the separate GlobalISel library.
615-
const MachineRegisterInfo &MRI = MF.getRegInfo();
616-
if (const LegalizerInfo *MLI = MF.getSubtarget().getLegalizerInfo()) {
617-
for (MachineBasicBlock &MBB : MF) {
618-
for (MachineInstr &MI : MBB) {
619-
if (isPreISelGenericOpcode(MI.getOpcode()) && !MLI->isLegal(MI, MRI)) {
620-
reportGISelFailure(MF, *TPC, *MORE, "gisel-regbankselect",
621-
"instruction is not legal", MI);
622-
return false;
623-
}
624-
}
613+
// FIXME: This should be in the MachineVerifier.
614+
if (!DisableGISelLegalityCheck)
615+
if (const MachineInstr *MI = machineFunctionIsIllegal(MF)) {
616+
reportGISelFailure(MF, *TPC, *MORE, "gisel-regbankselect",
617+
"instruction is not legal", *MI);
618+
return false;
625619
}
626-
}
627620
#endif
628621

629622
// Walk the function and assign register banks to all operands.

0 commit comments

Comments
 (0)