This is an archive of the discontinued LLVM Phabricator instance.

[PoC][TabgleGen] Add new bang operator !apply
AbandonedPublic

Authored by wangpc on Mar 29 2023, 3:02 AM.

Details

Summary

There is no way to pass function as a value in TableGen, but we can
treat DAGs as S-expression and evaluate them.

In this patch, a new bang operator is added:

type value = !apply<type>(dag[, param]);

We can treat dag as function and evaluate it on params.

To not increase complexity, we map DAGs to subset of existed bang
operators. But we may support more bang operator or even user-definded
subroutines.

The need of !apply has been discussed in D146198.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2023, 3:02 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
pcwang-thead requested review of this revision.Mar 29 2023, 3:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2023, 3:02 AM
tra added a comment.Mar 29 2023, 10:26 AM

It would be great to update tablegen documentation, too.

On the surface, I think we're reinventing a lambda here.

llvm/lib/TableGen/Record.cpp
1898

How about mul, div, and mod ?

1898

Does it mean that we make all these ops a reserved keyword which would no longer be available to users for other purposes?

E.g. here: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SIRegisterInfo.td#L123

pcwang-thead marked 2 inline comments as done.Mar 30 2023, 12:06 AM

It would be great to update tablegen documentation, too.

Thanks. This patch is still in the early stage, doc and more operations will be added later.

On the surface, I think we're reinventing a lambda here.

Yes, so I post this patch here to gather feedbacks to see if I am on the right track. :-)

llvm/lib/TableGen/Record.cpp
1898

mul, div, mod and others will be added later.

1898

No, I think they won't be reserved keywords. These ops are valid only if they are in the context of !apply. And they are just mapped to existed bang operators and the semantic meanings are target-independent.
For the example you pointed out, they are some set operations and implemented in SetTheory.h/.cpp and the semantic meanings are target-dependent.

This is an interesting proposal, but I think it really needs a bit more thought on the overall design. If we're going to add something like this, it should be nicely orthogonal and general.

Some examples of problems in what's there right now:

  • Parallel hard-coding of add, sub etc. in the parsing of IDs. Not great, since it affects parsing everywhere -- we could do that, but then what's the overall direction here with regards to !apply vs. the existing ops?
  • Hard-coding of argument names $aN.

I agree with @tra that in a way this is reinventing lambdas, so perhaps we should instead just literally take lambdas? That means a syntax for defining lambdas and a syntax for applying them. Strawman riffing off apply.td, something like:

defvar add_lambda = (lambda (lhs, rhs) !add(lhs, rhs));

class A<int a, int b> {
  int add_result = !apply(add_lambda, a, b);
}

What I'm thinking here is that the expression (lambda (args...) expr) is not a DAG but a new expression/init kind, with lambda being the keyword to detect that. The expression is evaluated with a scope where the arguments are substituted.

We could consider alternative syntaxes, two possibilities that would probably work:

defvar add_lambda = ($lhs, $rhs) -> !add(lhs, rhs);
defvar add_lambda = |lhs, rhs| !add(lhs, rhs);

And perhaps we can shortcut the evaluation syntax to just:

class A<int a, int b> {
  int add_result = !(add_lambda a b);
}

Or, perhaps:

class A<int a, int b> {
  int add_result = !add_lambda(a, b);
};

In other words, the syntax is ! expression ( args... ) where the expression must evaluate to a lambda. We could turn most (but not all) bang operators into builtin variables and turn a few exceptional cases into reserved keywords.

I haven't thought this through fully, just throwing some ideas out there that hopefully help suggest a cleaner and more robust design. I think this is large enough that it would also merit having a discussion on Discourse for visibility.

pcwang-thead marked 2 inline comments as done.Mar 30 2023, 5:21 AM

This is an interesting proposal, but I think it really needs a bit more thought on the overall design. If we're going to add something like this, it should be nicely orthogonal and general.

Some examples of problems in what's there right now:

  • Parallel hard-coding of add, sub etc. in the parsing of IDs. Not great, since it affects parsing everywhere -- we could do that, but then what's the overall direction here with regards to !apply vs. the existing ops?
  • Hard-coding of argument names $aN.

I agree with @tra that in a way this is reinventing lambdas, so perhaps we should instead just literally take lambdas? That means a syntax for defining lambdas and a syntax for applying them. Strawman riffing off apply.td, something like:

defvar add_lambda = (lambda (lhs, rhs) !add(lhs, rhs));

class A<int a, int b> {
  int add_result = !apply(add_lambda, a, b);
}

What I'm thinking here is that the expression (lambda (args...) expr) is not a DAG but a new expression/init kind, with lambda being the keyword to detect that. The expression is evaluated with a scope where the arguments are substituted.

We could consider alternative syntaxes, two possibilities that would probably work:

defvar add_lambda = ($lhs, $rhs) -> !add(lhs, rhs);
defvar add_lambda = |lhs, rhs| !add(lhs, rhs);

And perhaps we can shortcut the evaluation syntax to just:

class A<int a, int b> {
  int add_result = !(add_lambda a b);
}

Or, perhaps:

class A<int a, int b> {
  int add_result = !add_lambda(a, b);
};

In other words, the syntax is ! expression ( args... ) where the expression must evaluate to a lambda. We could turn most (but not all) bang operators into builtin variables and turn a few exceptional cases into reserved keywords.

I haven't thought this through fully, just throwing some ideas out there that hopefully help suggest a cleaner and more robust design. I think this is large enough that it would also merit having a discussion on Discourse for visibility.

Thanks! It's really helpful and inspiring!
I will redesign this and make it much more complete before discussing on Discourse.

tra added a comment.Mar 30 2023, 10:52 AM

While we're on the subject of lambdas, the pattern we currently use to deal with the lack of lambdas or functions is to instantiate a record with a field evaluated to be a particular value. A lot of those case could be better served by lambdas, but they would need to be able to deal with all tablegen types, not just integers.

I think the current patch's functionality could already be done using this approach, even if it's a bit more verbose.
E.g: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SIInstrInfo.td#L305
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SIInstrInfo.td#L2469

We may be just looking for a better syntax, not a new functionality.

wangpc commandeered this revision.Jul 13 2023, 7:53 PM
wangpc abandoned this revision.
wangpc added a reviewer: pcwang-thead.
wangpc removed a reviewer: pcwang-thead.

Close it since I will continue finishing similar feature in D148915.