[PATCH] D21809: [SystemZ] Add support for the .insn directive.

[Ulrich Weigand via llvm-commits]

uweigand added a comment.

In https://reviews.llvm.org/D21809#502103, @uweigand wrote:

> I don't really like the code duplication between the Insn / AsmInsn patterns.  I can see why we need a isCodeGenOnly as well as a isAsmParserOnly pattern (so that the disassembler doesn't get confused), but those should then get generated from a multiclass template.  See e.g. how this is done with the CompareBranches multiclass.
>
> In addition, it doesn't seem a good idea to have the AsmInsn patterns recognize "instructions" like InsnE.  This means that assembly files that contain the literal string InsnE will get recognized here, which is not really what we want.  I'm wondering if we cannot simply also use ".insn e,..." as the string for the AsmInsn patterns as well -- if we do that, I think the auto-generated matchers should use literal string matching to detect the correct pattern to use, which means that the extra table getOpcodeFromFormat would no longer be necessary -- another duplication eliminated.


Following up on myself here, actually I don't think we even need the isCodeGenOnly variants at all.  Simply having the isAsmParserOnly variants should suffice (we only need it for the asm parser after all ...).


https://reviews.llvm.org/D21809