* Specify a DAG with (...)
* Catenate DAGs with !con().
* Construct a DAG with !dag().
* Iterate over a DAG with !foreach().
* Get and set the DAG operator with !getop() and !setop().
What if you could also get and set a DAG's operands individually, by position or by $xxx name? Would that add enough power/convenience that you would build more interesting DAGs in TableGen, or is it just easier to do such building in the backends?
_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
It is some work, for sure.
Let me post a modest proposal here in the next few days. I'd appreciate comments and suggestions. The idea is to make it easier to manipulate complex DAGs in TableGen.
1. Add a new value suffix.
value(index)
The value must be a DAG. The index specifies the operator or an operand,
whose value is produced. The index can be
0 produce the operator
1...n produce operand by position
$name produce operator/operand by its variable name
string produce operator/operand by a string containing its
variable name
If the item does not exist, ? (uninitialized) is produced.
Note that multiple value suffixes are allowed, so, for example,
DagList[i](1) would produce the first operand of the i-th dag
in the list.
2. Add the !getdag() bang operator.
!getdag(dag, index [, default])
This bang operator produces the same result as the (...) suffix.
However, the default value can be specified as the third argument.
If it is not specified, ? is used.
3. Add the !setdag bang operator.
!setdag(dag, index1, value1, index2, value2, ...)
This bang operator creates a copy of the top-level dag node and
then replaces the operator and/or operands with new values. Each
replacement is specified by an index and a value. The new dag
is produced.
The two new bang operators replace !getop() and !setop(), which could be
deprecated.
4. The !size operator will be extended to accept a DAG and produce
the number of operands in it.
On Sun, Oct 11, 2020 at 4:55 PM Paul C. Anagnostopoulos via llvm-dev
<llvm...@lists.llvm.org> wrote:
> This is a proposal to enhance TableGen's ability to analyze and manipulate
> DAGs. Hopefully this will allows more complex DAGs to be built in TableGen.
>
> 1. Add a new value suffix.
>
> value(index)
>
> The value must be a DAG. The index specifies the operator or an operand,
> whose value is produced. The index can be
>
> 0 produce the operator
> 1...n produce operand by position
This seems reasonable.
> $name produce operator/operand by its variable name
> string produce operator/operand by a string containing its
> variable name
I don't like this part, because to me is seems to conflate what the
indices vs. name operands in DAGs are for. We can often think of DAG
operators as functions, and the operands are arguments of the
function. So using a numeric index would return an argument of a given
index. However, the $names are _not_ names of function arguments! They
are a mechanism for tagging DAG nodes that are interesting as part of
pattern matching.
So please don't add this functionality, and definitely don't add it in
this way. If there is a convincing reason for extracting DAG nodes by
name, then it should be done via a different ! accessor that performs
a deep search of the DAG (i.e., it can produce DAG nodes inside
arbitrarily deeply nested children).
> If the item does not exist, ? (uninitialized) is produced.
I think it would be better for this to fail instead (i.e., not fold
and produce an error at final resolution). Especially since you
propose !getdag below.
> Note that multiple value suffixes are allowed, so, for example,
> DagList[i](1) would produce the first operand of the i-th dag
> in the list.
>
> 2. Add the !getdag() bang operator.
>
> !getdag(dag, index [, default])
>
> This bang operator produces the same result as the (...) suffix.
> However, the default value can be specified as the third argument.
> If it is not specified, ? is used.
As above, I would suggest having to specify ? explicitly.
> 3. Add the !setdag bang operator.
>
> !setdag(dag, index1, value1, index2, value2, ...)
>
> This bang operator creates a copy of the top-level dag node and
> then replaces the operator and/or operands with new values. Each
> replacement is specified by an index and a value. The new dag
> is produced.
>
> The two new bang operators replace !getop() and !setop(), which could be
> deprecated.
>
> 4. The !size operator will be extended to accept a DAG and produce
> the number of operands in it.
Sounds good.
Cheers,
Nicolai
>
> _______________________________________________
> LLVM Developers mailing list
> llvm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
--
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
Perhaps the people actually coding these DAGs have it all down in their minds by position anyway.
------------------------------------
Agreed.
>> If the item does not exist, ? (uninitialized) is produced.
>
>I think it would be better for this to fail instead (i.e., not fold
>and produce an error at final resolution). Especially since you
>propose !getdag below.
----------------------------------------
Agreed.
>> 2. Add the !getdag() bang operator.
>>
>> !getdag(dag, index [, default])
>>
>> This bang operator produces the same result as the (...) suffix.
>> However, the default value can be specified as the third argument.
>> If it is not specified, ? is used.
>
>As above, I would suggest having to specify ? explicitly.
_______________________________________________
My point is precisely that the $names don't work that way. Your
reasoning would be valid if the $names were function/operator argument
names, like in programming languages where you can pass function
arguments based on their order but also by naming them (e.g.
"functionName(argName=x, otherArgName=y)"). However, this is _not_ how
$names work!
Their most prominent application is for instruction selection pattern
matching, e.g. taken at random from AMDGPU/SOPInstructions.td:
def : GCNPat <
(i32 (smax i32:$x, (i32 (ineg i32:$x)))),
(S_ABS_I32 SReg_32:$x)
>;
The $x is _not_ the name of the argument to smax, ineg, or S_ABS_I32.
For example, if you look at how S_ABS_I32 is defined, you'll see that
its input operand is called $src0.
Instead, the name allows us to tie three locations in the DAG together
for purposes of pattern matching. The name is only meaningful in the
context of this pattern. You could substitute $x by $y or $whatever
without changing the meaning of the DAG.
That the name is the name of an operator argument is an understandable
misunderstanding, but it _is_ a misunderstanding. If you were to add
that particular feature, you would encourage this misunderstanding
even more.
Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
So DAG indexes are integers only.
----------------------------------------------------------------
Windfall Paul C. Anagnostopoulos
----------------------------------------------------------
Software 978 369-0839
www.windfall.com
----------------------------------------------------------------
My life has been filled with calamities,
some of which actually happened.
---Mark Twain
Guga 'mzimba, sala 'nhliziyo
At 10/13/2020 04:46 AM, Madhur Amilkanthwar wrote:
>What do you guys think about the below enhancements?
>
>5. !getdagrestype(dag [, index]) - Returns type of result value. If the DAG computes multiple values then return type of 'index'th result.
_______________________________________________
All of these sound like operations that are specific to TableGen
backend interpretations of what a DAG means. This discussion is about
!ops which are a part of the TableGen frontend, so I don't think any
of these apply here.
Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
If we have two operators to get and set DAG operator/operands, does it make sense to add to more operators to get/set the $names of operands? They would still specify the operand by integer index.
At 10/13/2020 10:16 AM, Nicolai Hähnle wrote:
>On Tue, Oct 13, 2020 at 10:47 AM Madhur Amilkanthwar
><madhu...@gmail.com> wrote:
>> What do you guys think about the below enhancements?
>>
>> 5. !getdagrestype(dag [, index]) - Returns type of result value. If the DAG computes multiple values then return type of 'index'th result.
>>
>> 6. !setdagrestype(dag target_dag, type T [, index]) - Set return type of target_dag to T. Use of 'index' is as in 5.(Coupled with the existing (or enhanced?) foreach construct we can construct multiple DAGs with different return types.)
>>
>> .7 !setdagchild(dag target_dag, dag new_dag, index) - Set child 'index' numbered of target_dag to new_dag. I think this is more or less similar to 3 you suggested but I feel it is more convenient and concise.
>>
>> 8. !setdagchildcond(dag target_dag, dag new_dag, index, {C++ code}) - Similar to 7 above but do it only if the C++ code returns true. This is useful to check if the result type of `new_dag` and that of the operand type of 'index' child of 'target_dag' are compatible. Users can define compatibility using C++ code. For example, it is okay to set dag even if there is mismatch between signedness of types.
>
>All of these sound like operations that are specific to TableGen
>backend interpretations of what a DAG means. This discussion is about
>!ops which are a part of the TableGen frontend, so I don't think any
>of these apply here.
>
>Cheers,
>Nicolai
_______________________________________________
>Cheers,
>Nicolai
Madhur, could you describe !getdagrestype in more detail? In particular, I don't know what "type of result value" means.
At 10/13/2020 11:33 PM, Madhur Amilkanthwar wrote:
>On Tue, Oct 13, 2020, 10:44 PM Paul C. Anagnostopoulos <<mailto:pa...@windfall.com>pa...@windfall.com> wrote:
>Madhur, could you describe !getdagrestype in more detail? In particular, I don't know what "type of result value" means.
>
>I mean the data type of the result computed by a DAG node. A DAG node may receive a bunch of values from others nodes, applies its operator and returns an 'int', so its restype is 'int', for example.
>
>Does that make sense?
_______________________________________________
You're right that 7 is like !cond (and actually seems like it's
identical to the !setdag proposed by Paul).
8 doesn't work though because you can't just run C++ code at TableGen
frontend runtime.
Cheers,
Nicolai
>
> 5 and 6 are subject to debate but since it's a language an addition like this could be useful.
>
>
>> >Cheers,
>> >Nicolai
>>
>
>
> --
> Disclaimer: Views, concerns, thoughts, questions, ideas expressed in this mail are of my own and my employer has no take in it.
> Thank You.
> Madhur D. Amilkanthwar
>
--
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
Yes, I do think that would be helpful. For setting, I think you'd want
to set the child and name at the same time, i.e. something like
!setdag(dag_to_modify, index1, value1, name1, index2, value2, name2,
...). Either valueN or nameN can be `?` (unset).
Cheers,
Nicolai
>
> At 10/13/2020 10:16 AM, Nicolai Hähnle wrote:
> >On Tue, Oct 13, 2020 at 10:47 AM Madhur Amilkanthwar
> ><madhu...@gmail.com> wrote:
> >> What do you guys think about the below enhancements?
> >>
> >> 5. !getdagrestype(dag [, index]) - Returns type of result value. If the DAG computes multiple values then return type of 'index'th result.
> >>
> >> 6. !setdagrestype(dag target_dag, type T [, index]) - Set return type of target_dag to T. Use of 'index' is as in 5.(Coupled with the existing (or enhanced?) foreach construct we can construct multiple DAGs with different return types.)
> >>
> >> .7 !setdagchild(dag target_dag, dag new_dag, index) - Set child 'index' numbered of target_dag to new_dag. I think this is more or less similar to 3 you suggested but I feel it is more convenient and concise.
> >>
> >> 8. !setdagchildcond(dag target_dag, dag new_dag, index, {C++ code}) - Similar to 7 above but do it only if the C++ code returns true. This is useful to check if the result type of `new_dag` and that of the operand type of 'index' child of 'target_dag' are compatible. Users can define compatibility using C++ code. For example, it is okay to set dag even if there is mismatch between signedness of types.
> >
> >All of these sound like operations that are specific to TableGen
> >backend interpretations of what a DAG means. This discussion is about
> >!ops which are a part of the TableGen frontend, so I don't think any
> >of these apply here.
> >
> >Cheers,
> >Nicolai
>
--
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
Should there be !getdagvalue and !getdagname, or should !getdag produce a pair?
At 10/14/2020 09:44 AM, Nicolai Hähnle wrote:
>On Tue, Oct 13, 2020 at 4:37 PM Paul C. Anagnostopoulos
><pa...@windfall.com> wrote:
>> Nicolai:
>>
>> If we have two operators to get and set DAG operator/operands, does it make sense to add to more operators to get/set the $names of operands? They would still specify the operand by integer index.
>
>Yes, I do think that would be helpful. For setting, I think you'd want
>to set the child and name at the same time, i.e. something like
>!setdag(dag_to_modify, index1, value1, name1, index2, value2, name2,
>...). Either valueN or nameN can be `?` (unset).
_______________________________________________
I don't have a strong opinion, but would lean slightly towards
!getdagvalue / !getdagname because of how things tend to fit together
in TableGen in general.
Cheers,
Nicolai
>
> At 10/14/2020 09:44 AM, Nicolai Hähnle wrote:
> >On Tue, Oct 13, 2020 at 4:37 PM Paul C. Anagnostopoulos
> ><pa...@windfall.com> wrote:
> >> Nicolai:
> >>
> >> If we have two operators to get and set DAG operator/operands, does it make sense to add to more operators to get/set the $names of operands? They would still specify the operand by integer index.
> >
> >Yes, I do think that would be helpful. For setting, I think you'd want
> >to set the child and name at the same time, i.e. something like
> >!setdag(dag_to_modify, index1, value1, name1, index2, value2, name2,
> >...). Either valueN or nameN can be `?` (unset).
>
--
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
1. Add a new value suffix.
value(index)
The value must be a DAG. The index specifies the operator or an operand,
whose value is produced. The index can be
0 produce the operator
1...n produce operand by position
If the item does not exist, an error is reported.
Note that multiple value suffixes are allowed, so, for example,
DagList[i](1) would produce the first operand of the i-th dag
in the list.
2. Add the !getdagvalue() bang operator.
!getdagvalue(dag, index [, default])
This bang operator produces the same result as the (...) suffix.
However, the default value can be specified as the third argument.
If the item does not exist and no default is specified, an error
is reported.
2. Add the !getdagname() bang operator.
!getdagname(dag, index [, default])
This bang operator produces the name of the indexed operator/operand,
as a string without the leading dollar sign ($). The default value
can be specified as the third argument. If the item does not exist
and no default is specified, an error is reported.
3. Add the !setdag bang operator.
!setdag(dag, index1, value1, name1, index2, value2, name2, ...)
This bang operator creates a copy of the top-level dag node and
then replaces the operator and/or operands with new values and names.
Each replacement is specified by an index, a value, and a name.
The value and/or name can be ? (uninitialized). The new dag
is produced. If an index is invalid, an error is reported.
!getdagvalue and !setdagvalue replace !getop() and !setop(), which could be
deprecated.
4. The !size operator will be extended to accept a DAG node and produce
the number of operands in it.
One option is to index the operator as -1 and the operands from 0.
A second option is to remove the ability to index the operator and retain the !getop and !setop bangs for that purpose, possibly renamed to !getdagop and !setdagop.
Opinions, please.
On Sat, Oct 17, 2020 at 2:05 PM Paul C. Anagnostopoulos via llvm-dev
<llvm...@lists.llvm.org> wrote:
> Now I'm second-guessing myself about the indexes used to access the operator and operands. All the C++ functions used to access the operands index them from 0, as expected. It seems foolish and confusing to index them from 1 in these bang operators.
>
> One option is to index the operator as -1 and the operands from 0.
>
> A second option is to remove the ability to index the operator and retain the !getop and !setop bangs for that purpose, possibly renamed to !getdagop and !setdagop.
That's a good thought. It seems like a good idea to be consistent with
how things look from a backend's perspective in C++, so separate
!get(dag)op and !set(dag)op seems slightly better. Just my opinion :)
Cheers,
Nicolai
>
> Opinions, please.
>
> _______________________________________________
> LLVM Developers mailing list
> llvm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
--
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
Meanwhile, I will rename !getdagop and !setdagop just for consistency. The old !getop and !setop will remain, of course, but deprecated. !getop is used 0 times and !setop 4 times.
~~ Paul
At 10/19/2020 12:39 PM, Nicolai Hähnle wrote:
>I generally like the proposal.
>
>On Sat, Oct 17, 2020 at 2:05 PM Paul C. Anagnostopoulos via llvm-dev
><llvm...@lists.llvm.org> wrote:
>> Now I'm second-guessing myself about the indexes used to access the operator and operands. All the C++ functions used to access the operands index them from 0, as expected. It seems foolish and confusing to index them from 1 in these bang operators.
>>
>> One option is to index the operator as -1 and the operands from 0.
>>
>> A second option is to remove the ability to index the operator and retain the !getop and !setop bangs for that purpose, possibly renamed to !getdagop and !setdagop.
>
>That's a good thought. It seems like a good idea to be consistent with
>how things look from a backend's perspective in C++, so separate
>!get(dag)op and !set(dag)op seems slightly better. Just my opinion :)
>
>Cheers,
>Nicolai
_______________________________________________
If it's really only used 4 times, it may be worth just removing those
instances instead of keeping the deprecated versions around.
Cheers,
Nicolai
>
> ~~ Paul
>
>
> At 10/19/2020 12:39 PM, Nicolai Hähnle wrote:
>> I generally like the proposal.
>>
>> On Sat, Oct 17, 2020 at 2:05 PM Paul C. Anagnostopoulos via llvm-dev
>> <llvm...@lists.llvm.org> wrote:
>>> Now I'm second-guessing myself about the indexes used to access the operator and operands. All the C++ functions used to access the operands index them from 0, as expected. It seems foolish and confusing to index them from 1 in these bang operators.
>>>
>>> One option is to index the operator as -1 and the operands from 0.
>>>
>>> A second option is to remove the ability to index the operator and retain the !getop and !setop bangs for that purpose, possibly renamed to !getdagop and !setdagop.
>>
>> That's a good thought. It seems like a good idea to be consistent with
>> how things look from a backend's perspective in C++, so separate
>> !get(dag)op and !set(dag)op seems slightly better. Just my opinion :)
>>
>> Cheers,
>> Nicolai
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
At 10/19/2020 01:25 PM, Nicolai Hähnle wrote:
>On 19.10.20 18:53, Paul C. Anagnostopoulos wrote:
>>I think I will do both. In order that dag(i) can work and fetch the operator, an integer index is needed for it. So -1 for the operator. Then that should be supported in the other bang operators.
>>Meanwhile, I will rename !getdagop and !setdagop just for consistency. The old !getop and !setop will remain, of course, but deprecated. !getop is used 0 times and !setop 4 times.
>
>If it's really only used 4 times, it may be worth just removing those instances instead of keeping the deprecated versions around.
>
>Cheers,
>Nicolai
_______________________________________________
True, it's fair to give them a heads-up. Depending on the user, it may
already be good enough to split the change up into two: first, a chancge
to add !getdagop/!setdagop; second, a change to remove the old form.
That provides at least one "moment in time" where both versions can be
used, which can help with migrations.
If you leave a few weeks between committing the two changes, it's even
nicer.
Cheers,
Nicolai
>
> At 10/19/2020 01:25 PM, Nicolai Hähnle wrote:
>> On 19.10.20 18:53, Paul C. Anagnostopoulos wrote:
>>> I think I will do both. In order that dag(i) can work and fetch the operator, an integer index is needed for it. So -1 for the operator. Then that should be supported in the other bang operators.
>>> Meanwhile, I will rename !getdagop and !setdagop just for consistency. The old !getop and !setop will remain, of course, but deprecated. !getop is used 0 times and !setop 4 times.
>>
>> If it's really only used 4 times, it may be worth just removing those instances instead of keeping the deprecated versions around.
>>
>> Cheers,
>> Nicolai
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
At 10/19/2020 01:47 PM, Nicolai Hähnle wrote:
>On 19.10.20 19:33, Paul C. Anagnostopoulos wrote:
>>But aren't we concerned about users of TableGen outside of the LLVM project? I've come across at least one in my travels through the interwebs.
>
>True, it's fair to give them a heads-up. Depending on the user, it may already be good enough to split the change up into two: first, a chancge to add !getdagop/!setdagop; second, a change to remove the old form. That provides at least one "moment in time" where both versions can be used, which can help with migrations.
>
>If you leave a few weeks between committing the two changes, it's even nicer.
>
>Cheers,
>Nicolai
_______________________________________________