[PATCH] D41728: [TableGen][AsmMatcherEmitter] Remove boolean 'Hack' parameter

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 8 04:48:39 PST 2018


sdesmalen added a comment.

In https://reviews.llvm.org/D41728#968954, @apazos wrote:

> I tested it internally with my test cases and it works fine.
>  I also built all targets and did a 'ninja check-all' and I did not detect any new failures.
>  So I think no target breaks if we remove this Hack flag. Did you confirm it?


We have specific tests for SVE for which this change is needed which all pass. Also, doing a check-all found no issues for other targets, so I think we should be good.

> Now with this issue fixed, I want to raise another related issue (with tied operands) in AsmWriterEmitter::EmitPrintAliasInstruction.
>  Note that code bails out if number of operands in the AsmString is higher than the number of operands in the ResultString:

Are you saying that you have a case where you expect the Alias to have more operands than the actual instruction? Because I can't directly think of a reason why having a duplicate operand would be useful in an instruction alias?

> Thanks Daniel for pushing this patch.

I'm not Daniel ;)



================
Comment at: utils/TableGen/AsmMatcherEmitter.cpp:1121
         errs() << "warning: '" << TheDef->getName() << "': "
                << "ignoring instruction with tied operand '"
                << Tok << "'\n";
----------------
apazos wrote:
> change  << "ignoring instruction with tied operand ' to    << "instruction with tied operand ' or remove the warning...
I think the 'ignoring' is there in the message because the instruction is actually ignored (see the caller of validate()), which is actually a good thing to warn about.


https://reviews.llvm.org/D41728





More information about the llvm-commits mailing list