llvm dev - Oct 2020 - Manipulating DAGs in TableGen

I included the ability to get/set an operand by name because I thought it would
be easier to copy+modify an existing DAG by specifying the name of the operand
you want to replace rather than having to remember its position. For example, if
you want to replace the first source, isn't it easier to specify $src than
remember it's the second operand?

Perhaps the people actually coding these DAGs have it all down in their minds by
position anyway.

At 10/12/2020 12:40 PM, Nicolai Hähnle wrote:
>Hi Paul,
>
>On Sun, Oct 11, 2020 at 4:55 PM Paul C. Anagnostopoulos via llvm-dev
><llvm-dev at 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).
------------------------------------

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.

On Mon, Oct 12, 2020 at 7:37 PM Paul C. Anagnostopoulos
<paul at windfall.com> wrote:
> I included the ability to get/set an operand by name because I thought it
would be easier to copy+modify an existing DAG by specifying the name of the
operand you want to replace rather than having to remember its position. For
example, if you want to replace the first source, isn't it easier to specify
$src than remember it's the second operand?
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

>
> Perhaps the people actually coding these DAGs have it all down in their
minds by position anyway.
>
> At 10/12/2020 12:40 PM, Nicolai Hähnle wrote:
> >Hi Paul,
> >
> >On Sun, Oct 11, 2020 at 4:55 PM Paul C. Anagnostopoulos via llvm-dev
> ><llvm-dev at 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).
>
> ------------------------------------
>
> 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.
>

-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.

I understood that the name is a matching tag for the operand and not its name
(as in named macro or function arguments). However, I was assuming that the
names in any one DAG node had to be unique and so could serve as selectors for
operands. But a quick investigation shows that I was wrong: names can be
duplicated in the same node.

So DAG indexes are integers only.


At 10/12/2020 01:46 PM, Nicolai Hähnle wrote:
>On Mon, Oct 12, 2020 at 7:37 PM Paul C. Anagnostopoulos
><paul at windfall.com> wrote:
>> I included the ability to get/set an operand by name because I thought
it would be easier to copy+modify an existing DAG by specifying the name of the
operand you want to replace rather than having to remember its position. For
example, if you want to replace the first source, isn't it easier to specify
$src than remember it's the second operand?
>
>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.

----------------------------------------------------------------
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