[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