[PATCH] [AArch64] Add logical pseudo instructions to MC AsmParser
Tim Northover
t.p.northover at gmail.com
Thu Jul 10 07:02:59 PDT 2014
Hi Arnaud,
> For example, with this patch, the closest match for the assembler is the pseudo
> in one of the basic-a64-diagnostics tests, which leads to a weird diagnostic. Would
> there be a non convoluted way to disambiguate this, or give a lower priority to
> pseudos ?
Not that I'm aware of. The diagnostic doesn't look too bad to me.
There are certainly worse around in LLVM's assemblers.
I don't think "pseudo-instruction" or related terms is right here
though. LLVM actually has pseudo-instructions of various kinds, and
they're not used in this patch (fortunately -- that would have been
the wrong way to do it and I was a little worried before I opened up
the attachment).
+def logical_imm32_not_XFORM : SDNodeXForm<imm, [{
SDNodeXForms only apply to CodeGen, but you're not actually changing
any patterns so I think these are unnecessary. Similarly for the
PatLeaf instantiations. Operands can be much more minimal if they're
only used in InstAliases.
+ let PrintMethod = "printLogicalImm32";
I hope these are unnecessary too. Or do they get referred to in
generated files even if they're not called?
+ string pseudo> {
Following my first comments, I think this is the wrong name. Perhaps
NegAlias or something? Even just "Alias" would be more accurate.
+ let AddedComplexity = 6 in
With no new CodeGen patterns, this seems unnecessary. Also untested,
since there aren't any .ll tests.
+ const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(getImm());
+ assert(MCE && "Invalid logical immediate operand!");
If you use "cast" instead of "dyn_cast" it includes an assert.
Other than that, it looks reasonable to me.
Cheers.
Tim.
More information about the llvm-commits
mailing list