[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