Mark Schimmel via llvm-dev
2019-Apr-01 18:20 UTC
[llvm-dev] Please expose predicates to MachineVerifier
Could we expose predicates defined in the target InstrInfo.td file to the
MachineVerifier? We use BuildMI() to create many instructions after ISEL, but
the predicates are not being checked at this point. Thus, I could forget to
check the target and build an instruction that is illegal for a specific
configuration. In such a case it would be nice if the MachineVerifier could
detect this for me.
Example predicate IsCore8 usage:
def ADDx: BINOP<...>, Requires<[IsCore8]>;
let Predicates=[IsCore8] in {
def : SUBX: BINOP<...>;
}
The predicates are encoded into the targets GenDAGIsel file but not anywhere
else that I can find:
OPC_CheckPatternPredicate, 4, // (Subtarget.isCore8())
Thanks
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190401/b46c6b83/attachment.html>
Eli Friedman via llvm-dev
2019-Apr-01 18:33 UTC
[llvm-dev] [EXT] Please expose predicates to MachineVerifier
There's already an existing hook for adding target-specific verification
logic in the target's TargetInstrInfo::verifyInstruction. That doesn't
really scale to checking that you're using an appropriate subtarget for
every instruction, though. (You could, but it would involve a lot of redundant
hand-written code for many targets.)
We can't really use any of the existing code generated by TableGen to
perform the checks in question; I don't think we currently expose the
relevant predicates in any useful way. But it would probably be appropriate to
extend TableGen to generate code for those checks, I think. It would probably
also require some modifications to the TableGen files themselves; currently, at
least for some targets, we use Requires predicates on instructions which are
legal, but we don't want the TableGen'ed isel to select for some reason.
-Eli
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Mark
Schimmel via llvm-dev
Sent: Monday, April 1, 2019 11:21 AM
To: llvm-dev at lists.llvm.org
Subject: [EXT] [llvm-dev] Please expose predicates to MachineVerifier
Could we expose predicates defined in the target InstrInfo.td file to the
MachineVerifier? We use BuildMI() to create many instructions after ISEL, but
the predicates are not being checked at this point. Thus, I could forget to
check the target and build an instruction that is illegal for a specific
configuration. In such a case it would be nice if the MachineVerifier could
detect this for me.
Example predicate IsCore8 usage:
def ADDx: BINOP<...>, Requires<[IsCore8]>;
let Predicates=[IsCore8] in {
def : SUBX: BINOP<...>;
}
The predicates are encoded into the targets GenDAGIsel file but not anywhere
else that I can find:
OPC_CheckPatternPredicate, 4, // (Subtarget.isCore8())
Thanks
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190401/b159c012/attachment.html>
Björn Pettersson A via llvm-dev
2019-Apr-24 21:48 UTC
[llvm-dev] [EXT] Please expose predicates to MachineVerifier
Hi Mark!
Downstream I've added some support for our target (in this example called
Qwerty) like this.
In utils/TableGen/InstrInfoEmitter.cpp I've added:
--------------------------------------------------------------------------------
void EmitPredicateVerifier(RecordKeeper &Records, raw_ostream &OS) {
CodeGenTarget Target(Records);
StringRef NameSpace = Target.getInstNamespace();
// The generated VerifyPredicates method is only guaranteed to work for
// Qwerty (it takes a QwerySubtarget* as input). So avoid generating it for
// other targets.
if (Target.getName() != "Qwerty")
return;
OS << "#ifdef GET_PREDICATE_VERIFIER\n";
OS << "#undef GET_PREDICATE_VERIFIER\n";
OS << "namespace llvm {\n";
OS << "namespace " << NameSpace << "
{\n\n";
OS << "// "<< "VerifyPredicates" <<
"\nLLVM_READONLY\n";
OS << "static bool "<< "VerifyPredicates"
<< "(uint16_t Opcode, const QwertySubtarget *Subtarget)
{\n";
OS << " switch (Opcode) {\n";
unsigned Num = 0;
for (const CodeGenInstruction *Inst : Target.getInstructionsByEnumValue()) {
std::vector<Record *> PredicateRecs
Inst->TheDef->getValueAsListOfDefs("Predicates");
// This is more or less copied from PatternToMatch::getPredicateCheck(),
// so let's sort the checks in the same way as it is done for the
DAGIsel
// matcher.
std::sort(PredicateRecs.begin(), PredicateRecs.end(), LessRecord());
SmallString<128> PredicateCheck;
for (Record *Pred : PredicateRecs) {
if (!PredicateCheck.empty())
PredicateCheck += " && ";
PredicateCheck += "(";
PredicateCheck += Pred->getValueAsString("CondString");
PredicateCheck += ")";
}
if (!PredicateCheck.empty()) {
OS << " case " << Num << ": /* "
<< Inst->TheDef->getName() << " */";
OS << " return " << PredicateCheck <<
";\n";
}
Num++;
}
OS << " }\n";
OS << " return true;\n";
OS << "}\n\n";
OS << "} // End " << NameSpace << "
namespace\n";
OS << "} // End llvm namespace\n";
OS << "#endif // GET_PREDICATE_VERIFIER\n\n";
}
--------------------------------------------------------------------------------
And at the end of the same file it is plugged in like this:
--------------------------------------------------------------------------------
namespace llvm {
void EmitInstrInfo(RecordKeeper &RK, raw_ostream &OS) {
InstrInfoEmitter(RK).run(OS);
EmitMapTable(RK, OS);
+ EmitPredicateVerifier(RK, OS);
}
--------------------------------------------------------------------------------
And then in QwertyInstrInfo we do:
--------------------------------------------------------------------------------
bool QwertyInstrInfo::verifyInstruction(const MachineInstr &MI,
StringRef &ErrInfo) const {
if (!QWERTY::VerifyPredicates(MI.getOpcode(), &Subtarget)) {
ErrInfo = "Instruction is not available for the subtarget.";
return false;
}
...
--------------------------------------------------------------------------------
If this is of interest for other targets (also in-tree) I guess it could be
generalized a little bit (as you see above I've hardcoded some parts to use
"QwertySubtarget"), and maybe it also can be plugged in to be called
from MachineVerifier directly.
The function generated by tablegen will look something like this:
--------------------------------------------------------------------------------
#ifdef GET_PREDICATE_VERIFIER
#undef GET_PREDICATE_VERIFIER
namespace llvm {
namespace QWERTY {
// VerifyPredicates
LLVM_READONLY
static bool VerifyPredicates(uint16_t Opcode, const QwertySubtarget *Subtarget)
{
switch (Opcode) {
case 183: /* add_only_v1_pseudo */ return (Subtarget->isQwertyV1());
case 195: /* add_only_v2_pseudo */ return (Subtarget->isQwertyV2());
case 296: /* xor_only_v2 */ return (Subtarget->isQwertyV2());
}
return true;
}
} // End QWERTY namespace
} // End llvm namespace
#endif // GET_PREDICATE_VERIFIER
--------------------------------------------------------------------------------
Feel free to create a patch from the above on your own. Or let me know if there
is interest in having something similar to the above added for in-tree targets,
and then I can try to find time to generalize my patch.
/Björn
________________________________________
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Eli
Friedman via llvm-dev <llvm-dev at lists.llvm.org>
Sent: Monday, April 1, 2019 8:33:26 PM
To: Mark Schimmel; llvm-dev
Subject: Re: [llvm-dev] [EXT] Please expose predicates to MachineVerifier
There’s already an existing hook for adding target-specific verification logic
in the target’s TargetInstrInfo::verifyInstruction. That doesn’t really scale
to checking that you’re using an appropriate subtarget for every instruction,
though. (You could, but it would involve a lot of redundant hand-written code
for many targets.)
We can’t really use any of the existing code generated by TableGen to perform
the checks in question; I don’t think we currently expose the relevant
predicates in any useful way. But it would probably be appropriate to extend
TableGen to generate code for those checks, I think. It would probably also
require some modifications to the TableGen files themselves; currently, at least
for some targets, we use Requires predicates on instructions which are legal,
but we don’t want the TableGen’ed isel to select for some reason.
-Eli
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Mark
Schimmel via llvm-dev
Sent: Monday, April 1, 2019 11:21 AM
To: llvm-dev at lists.llvm.org
Subject: [EXT] [llvm-dev] Please expose predicates to MachineVerifier
Could we expose predicates defined in the target InstrInfo.td file to the
MachineVerifier? We use BuildMI() to create many instructions after ISEL, but
the predicates are not being checked at this point. Thus, I could forget to
check the target and build an instruction that is illegal for a specific
configuration. In such a case it would be nice if the MachineVerifier could
detect this for me.
Example predicate IsCore8 usage:
def ADDx: BINOP<…>, Requires<[IsCore8]>;
let Predicates=[IsCore8] in {
def : SUBX: BINOP<…>;
}
The predicates are encoded into the targets GenDAGIsel file but not anywhere
else that I can find:
OPC_CheckPatternPredicate, 4, // (Subtarget.isCore8())
Thanks
Björn Pettersson A via llvm-dev
2019-Apr-24 22:04 UTC
[llvm-dev] [EXT] Please expose predicates to MachineVerifier
And as Eli already pointed out, some extra attention might be needed for this
kind of EmitPredicateVerifier hack to work for any target.
In our downstream target we have separated all instruction definitions from the
patterns that selects the instructions. So I can use Requires<> tp specify
the predicates for when a specific instruction is valid when defining an
instruction. And then I can use totally different predicates for the Pat<>
rules that selects the instructions.
For a target that is mixing things up by having selection patterns included into
the instruction definitions it might be more complicated if the backend is
allowed to select the same instruction based on other predicates outside of the
ISelDAG.
________________________________________
From: Mark Schimmel <Mark.Schimmel at synopsys.com>
Sent: Wednesday, April 24, 2019 11:50:56 PM
To: Björn Pettersson A; Mark Schimmel; Eli Friedman
Subject: RE: [llvm-dev] [EXT] Please expose predicates to MachineVerifier
Thanks, I greatly appreciate it!
-----Original Message-----
From: Björn Pettersson A <bjorn.a.pettersson at ericsson.com>
Sent: Wednesday, April 24, 2019 2:48 PM
To: Mark Schimmel <mark.schimmel at synopsys.com>; Eli Friedman
<efriedma at quicinc.com>; llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] [EXT] Please expose predicates to MachineVerifier
Hi Mark!
Downstream I've added some support for our target (in this example called
Qwerty) like this.
In utils/TableGen/InstrInfoEmitter.cpp I've added:
--------------------------------------------------------------------------------
void EmitPredicateVerifier(RecordKeeper &Records, raw_ostream &OS) {
CodeGenTarget Target(Records);
StringRef NameSpace = Target.getInstNamespace();
// The generated VerifyPredicates method is only guaranteed to work for
// Qwerty (it takes a QwerySubtarget* as input). So avoid generating it for
// other targets.
if (Target.getName() != "Qwerty")
return;
OS << "#ifdef GET_PREDICATE_VERIFIER\n";
OS << "#undef GET_PREDICATE_VERIFIER\n";
OS << "namespace llvm {\n";
OS << "namespace " << NameSpace << "
{\n\n";
OS << "// "<< "VerifyPredicates" <<
"\nLLVM_READONLY\n";
OS << "static bool "<< "VerifyPredicates"
<< "(uint16_t Opcode, const QwertySubtarget *Subtarget)
{\n";
OS << " switch (Opcode) {\n";
unsigned Num = 0;
for (const CodeGenInstruction *Inst : Target.getInstructionsByEnumValue()) {
std::vector<Record *> PredicateRecs
Inst->TheDef->getValueAsListOfDefs("Predicates");
// This is more or less copied from PatternToMatch::getPredicateCheck(),
// so let's sort the checks in the same way as it is done for the
DAGIsel
// matcher.
std::sort(PredicateRecs.begin(), PredicateRecs.end(), LessRecord());
SmallString<128> PredicateCheck;
for (Record *Pred : PredicateRecs) {
if (!PredicateCheck.empty())
PredicateCheck += " && ";
PredicateCheck += "(";
PredicateCheck += Pred->getValueAsString("CondString");
PredicateCheck += ")";
}
if (!PredicateCheck.empty()) {
OS << " case " << Num << ": /* "
<< Inst->TheDef->getName() << " */";
OS << " return " << PredicateCheck <<
";\n";
}
Num++;
}
OS << " }\n";
OS << " return true;\n";
OS << "}\n\n";
OS << "} // End " << NameSpace << "
namespace\n";
OS << "} // End llvm namespace\n";
OS << "#endif // GET_PREDICATE_VERIFIER\n\n"; }
--------------------------------------------------------------------------------
And at the end of the same file it is plugged in like this:
--------------------------------------------------------------------------------
namespace llvm {
void EmitInstrInfo(RecordKeeper &RK, raw_ostream &OS) {
InstrInfoEmitter(RK).run(OS);
EmitMapTable(RK, OS);
+ EmitPredicateVerifier(RK, OS);
}
--------------------------------------------------------------------------------
And then in QwertyInstrInfo we do:
--------------------------------------------------------------------------------
bool QwertyInstrInfo::verifyInstruction(const MachineInstr &MI,
StringRef &ErrInfo) const {
if (!QWERTY::VerifyPredicates(MI.getOpcode(), &Subtarget)) {
ErrInfo = "Instruction is not available for the subtarget.";
return false;
}
...
--------------------------------------------------------------------------------
If this is of interest for other targets (also in-tree) I guess it could be
generalized a little bit (as you see above I've hardcoded some parts to use
"QwertySubtarget"), and maybe it also can be plugged in to be called
from MachineVerifier directly.
The function generated by tablegen will look something like this:
--------------------------------------------------------------------------------
#ifdef GET_PREDICATE_VERIFIER
#undef GET_PREDICATE_VERIFIER
namespace llvm {
namespace QWERTY {
// VerifyPredicates
LLVM_READONLY
static bool VerifyPredicates(uint16_t Opcode, const QwertySubtarget *Subtarget)
{
switch (Opcode) {
case 183: /* add_only_v1_pseudo */ return (Subtarget->isQwertyV1());
case 195: /* add_only_v2_pseudo */ return (Subtarget->isQwertyV2());
case 296: /* xor_only_v2 */ return (Subtarget->isQwertyV2());
}
return true;
}
} // End QWERTY namespace
} // End llvm namespace
#endif // GET_PREDICATE_VERIFIER
--------------------------------------------------------------------------------
Feel free to create a patch from the above on your own. Or let me know if there
is interest in having something similar to the above added for in-tree targets,
and then I can try to find time to generalize my patch.
/Björn
________________________________________
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Eli
Friedman via llvm-dev <llvm-dev at lists.llvm.org>
Sent: Monday, April 1, 2019 8:33:26 PM
To: Mark Schimmel; llvm-dev
Subject: Re: [llvm-dev] [EXT] Please expose predicates to MachineVerifier
There's already an existing hook for adding target-specific verification
logic in the target's TargetInstrInfo::verifyInstruction. That doesn't
really scale to checking that you're using an appropriate subtarget for
every instruction, though. (You could, but it would involve a lot of redundant
hand-written code for many targets.)
We can't really use any of the existing code generated by TableGen to
perform the checks in question; I don't think we currently expose the
relevant predicates in any useful way. But it would probably be appropriate to
extend TableGen to generate code for those checks, I think. It would probably
also require some modifications to the TableGen files themselves; currently, at
least for some targets, we use Requires predicates on instructions which are
legal, but we don't want the TableGen'ed isel to select for some reason.
-Eli
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Mark
Schimmel via llvm-dev
Sent: Monday, April 1, 2019 11:21 AM
To: llvm-dev at lists.llvm.org
Subject: [EXT] [llvm-dev] Please expose predicates to MachineVerifier
Could we expose predicates defined in the target InstrInfo.td file to the
MachineVerifier? We use BuildMI() to create many instructions after ISEL, but
the predicates are not being checked at this point. Thus, I could forget to
check the target and build an instruction that is illegal for a specific
configuration. In such a case it would be nice if the MachineVerifier could
detect this for me.
Example predicate IsCore8 usage:
def ADDx: BINOP<.>, Requires<[IsCore8]>;
let Predicates=[IsCore8] in {
def : SUBX: BINOP<.>;
}
The predicates are encoded into the targets GenDAGIsel file but not anywhere
else that I can find:
OPC_CheckPatternPredicate, 4, // (Subtarget.isCore8())
Thanks