Page MenuHomePhabricator

Change ASM Op printer to print the operation name in the framework instead of leaving it up to each individual operation

Authored by mehdi_amini on Aug 26 2021, 9:58 PM.



This aligns the printer with the parser contract: the operation isn't part of the user-controllable part of the syntax.

Depends On D108803

Diff Detail

Event Timeline

mehdi_amini created this revision.Aug 26 2021, 9:58 PM
mehdi_amini requested review of this revision.Aug 26 2021, 9:58 PM
rriddle accepted this revision.Aug 26 2021, 10:43 PM
rriddle added inline comments.

Why printEscapedString? AFAIK we aren't going to be able to parse any escaped characters back in.

This revision is now accepted and ready to land.Aug 26 2021, 10:43 PM

Fix for printOneResultOp

lattner accepted this revision.Aug 27 2021, 8:30 AM

Thank you so much for doing this!

Fix parseOneResultSameOperandTypeOp to support parsing the generic form emitted by printOneResultOp

One consequence of the current patch is that using printGenericOp from a custom printer
requires supporting parsing back the generic form from the custom parser, since the op name
is already printed without quotes.

bondhugula accepted this revision.Aug 28 2021, 10:16 AM
bondhugula added a subscriber: bondhugula.
bondhugula added inline comments.

Is this method named correctly? It's not printing the entire op.


Nit: Print the rest of the op now.


Nit: single quotes

mehdi_amini added inline comments.Aug 28 2021, 10:29 AM

With the default value printGenericOp(op) is still printing the entire op, I don't feel that having the ability to write printGenericOp(op, /*printOpName=*/false) really warrants a renaming: how would you rename it?

mehdi_amini marked 2 inline comments as done.

Address Uday's comments

This revision was landed with ongoing or failed builds.Aug 31 2021, 10:53 AM
This revision was automatically updated to reflect the committed changes.