[PATCH] D156420: [TableGen] Add `!dump` and `dump`.

Francesco Petrogalli via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 28 02:15:14 PDT 2023


fpetrogalli added a comment.

In D156420#4541608 <https://reviews.llvm.org/D156420#4541608>, @wangpc wrote:

> In D156420#4541511 <https://reviews.llvm.org/D156420#4541511>, @fpetrogalli wrote:
>
>> In D156420#4541117 <https://reviews.llvm.org/D156420#4541117>, @wangpc wrote:
>>
>>> Thanks! The implementation is just what I have imagined before!
>>
>> Good good! Thank you for the review.
>>
>>> One suggestion: can we accept variable-length arguments (at least one) and the first needn't to be a string? The output is just concatenation of all arguments and we make current implementation a special case.  So that we can dump multiple values in a single line.
>>
>> I'd keep it simple and avoid having to deal with variable-length args, because  once D156429 <https://reviews.llvm.org/D156429> is in, we can achieve the same with `!dump("the concat is = ", !format("{0} {1}...", vars...))`
>
> Or loose the constraint that first operand shoule be a string? for your example, it can be simpler: `!dump(!format("the concat is = {0} {1}...", vars...))`. `XDump` can still be a binary operator and set second operand to `StringInit("")` when only one operand is provided.
> By doing this, it won't be imcompatible if we want to extend it to be with variable-length arguments in the future.

We could do what you suggest, however please notice that `dump` is just a facility for TD debugging, not for string formatting. You are writing some TD file, you get unexpected results out of it and you want to debug what is going on in your code by checking the content of your variables, one by one. You find the bug, fix the code, remove the `dump` statements. If someone will need to debug more than a variable at once, they can use `!debug` + `!format`. Adding varargs to `!debug` seems to duplicate the varargs functionality of `format` at the cost of making the implementation of `dump` more complicated.



================
Comment at: llvm/lib/TableGen/Record.cpp:1189
+  case DUMP: {
+    if (isa<StringInit>(LHS) && RHS->isConcrete()) {
+      errs() << LHS->getAsUnquotedString();
----------------
wangpc wrote:
> Can we print it in debug mode and mute it in release mode?
> A CLI option may be added to control it and set it via cmake build type.
That would make the TableGen language depending on its build configuration. I am not sure that's the right thing to do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156420/new/

https://reviews.llvm.org/D156420



More information about the llvm-commits mailing list