[PATCH] D44112: TableGen: Type-check BinOps

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 9 03:37:12 PST 2018


nhaehnle added a comment.

In https://reviews.llvm.org/D44112#1030389, @tra wrote:

> This change should come with an update to the docs.


True, I'm adding that.

> One thing jumped at me -- we have three different variants of concatenation -- !con, !listconcat and !strconcat (along with '#'). Do we need all of them? I think we could get by with one, which would glue together items of the same kind.

I agree. The nicest option would be for `#` to cover everything, though it is a bit special in that it does automatic casts to string. Then again, it's probably good enough to just not do those conversions if one of the LHS or RHS are dags or lists.



================
Comment at: test/TableGen/Paste.td:15-24
+class Arithmetic<int i> {
+  string name = "number"#!add(i, 1);
+}
+
+def A : Arithmetic<5>;
+
+// CHECK: def A {
----------------
tra wrote:
> This does not seem to test anything relevant to this patch.Was this change supposed to be here or in some other patch?
This test fails without the change in TGParser.cpp:1957. Previously, parsing the paste passed `string` as required type to ParseValue, which would fail in the stricter checks when parsing the !add.


================
Comment at: test/TableGen/arithmetic.td:14
+
+def A0 : A<63, 1>;
----------------
tra wrote:
> I'm not sure if this file was meant to be in this patch. It does not seem to use 3+operand variants of !con, !add, !and, !or.
This tests that using different operand types on those operations is still allowed. I've added a comment and the full set of ops.


Repository:
  rL LLVM

https://reviews.llvm.org/D44112





More information about the llvm-commits mailing list