[PATCH] D25618: Check that emitted instructions meet their predicates on all targets except ARM, Mips, and X86.
Jay Foad via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 22 04:03:14 PST 2021
foad added a comment.
In D25618#3135577 <https://reviews.llvm.org/D25618#3135577>, @dsanders wrote:
> In D25618#3135224 <https://reviews.llvm.org/D25618#3135224>, @foad wrote:
>
>> Sorry for commenting on such an old patch but I am wondering why was `verifyInstructionPredicates` implemented inside of `MCCodeEmitter`? In practice this seem to mean that `llc -filetype=obj` gets this checking but `llc -filetype=asm` (the default, as used by almost all lit codegen tests) does not. Would it be possible to move `verifyInstructionPredicates` somewhere a bit more generic so that both paths can get this checking?
>
> I wouldn't rely on this recollection much but IIRC I didn't find a place where the final MCInsts were known and the code hadn't already diverged into asm/obj paths. I think the last pass before the paths diverged sometimes emitted different MCInst's down each path too
>
> If we can do it somewhere where it affects both then that'd be great.
Here's a demonstration that it *can* work, by making <Target>MCCodeEmitter::verifyInstructionPredicates static and calling into it from <Target>InstPrinter. But that doesn't seem very clean. I'd prefer to find a more neutral place to put verifyInstructionPredicates, but I'm not too familiar with the various classes that TableGen creates,
https://github.com/jayfoad/llvm-project/commits/verify-predicates (top three commits)
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D25618/new/
https://reviews.llvm.org/D25618
More information about the llvm-commits
mailing list