[PATCH] D148915: [WIP][TableGen] Introduce function and lambda

[Wang Pengcheng via Phabricator via llvm-commits]

pcwang-thead marked 5 inline comments as done.
pcwang-thead added a comment.

In D148915#4288193 <https://reviews.llvm.org/D148915#4288193>, @tra wrote:

> This is really nice!
>
> Now that we have functions, I would really love to see some sort of `dump` function which would print the value it's given when it's evaluated. This would make debugging tablegen files so much easier.

Yeah, this is a good advice. But I think it won't be implemented as `function`, but something like `assert` statements. I will put it to may future work list. :-)



================
Comment at: llvm/docs/TableGen/ProgRef.rst:302
+``function<>``\ *rettype*\ ``[,`` *argtype*\ ``]*>``
+    This type represents a function who returns a value of the *rettype* and may
+    have several argument types *argtype* (can be empty). The return type and
----------------
tra wrote:
> Do we need to define some sort of `void` type? 
> 
All statements of TableGen(except `assert`) should produce a value. It may be worthwhile to add `void`, but I may add it to future work list.


================
Comment at: llvm/test/TableGen/function.td:31
+  int default_arg_value = default_arg(add_func(2, 3));
+  int no_arg_value = no_arg();
+}
----------------
tra wrote:
> Can we ignore the value of a function call? E.g. one use case would be to use a function to consolidate various assertion checks without actually returning anything useful otherwise.
> 
Same as above reply.
All statements of TableGen (except `assert`) should produce a value. It could be hard to implement what you suggest, but I may have a try later.


================
Comment at: llvm/test/TableGen/function.td:381
+
+// ---- Test recursive function ---- //
+
----------------
tra wrote:
> Do we limit recursion depth or prevent runaway evaluation otherwise? 
Currently no. It would make the implementation hard to understand if we try to limit recursion depth.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148915