[PATCH] D73590: [mlir] Add a document detailing the design of the SymbolTable.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/docs/SymbolsAndSymbolTables.md:41
+[`SymbolTable`](#symbol-table). The name of a symbol *must* be unique within the
+parent `SymbolTable`. This name is semantically similarly to an SSA result
+value, and may be referred to by other operations to provide a symbolic link, or
----------------
mehdi_amini wrote:
> ```
> within the closest parent operation defining a `SymbolTable`
> ```
Used "immediately within" instead, as "closest parent operation" implies that a symbol can be outside of the immediate region of a symbol table, i.e. at depth>1


================
Comment at: mlir/docs/SymbolsAndSymbolTables.md:73
+is that dialects may need to ensure that their operations support `SymbolRefs`
+and SSA values, or provide operations that materializes an SSA value from a
+symbol reference. Each have different trade offs depending on the situation. A
----------------
mehdi_amini wrote:
> The part about "dialects may need to ensure that their operations support `SymbolRefs` and SSA values," isn't clear to me.
Reworded a bit. It is essentially bringing note to the different trade-offs now that there are two different ways to refer to operations.


================
Comment at: mlir/docs/SymbolsAndSymbolTables.md:88
+As described above, `Symbols` act as an auxiliary way of defining uses of
+operations to the traditional SSA use-list. As such, it is imperative to provide
+similar functionality to manipulate and inspect the list of uses and the users.
----------------
nicolasvasilache wrote:
> Unrelated question, would this (be a reasonable mechanism for | could be extended to support) aliasing for buffer-like semantics?
> 
> One of the difficulties when writing DRR for Linalg on tensors is that there are no use-def chains but instead aliasings coming from taking subviews and from a memref being an "input or an output operand" of a linalg op.
> 
> Some of the mechanisms you put in place here seem like they could apply and maybe make things significantly better re writing chains of read/write dependencies.
> The semantics of linalg guarantee full buffer writes at that level of abstraction so it "behaves like SSA" (hand-wavy) from this point of view. 
> 
> The big missing thing would be supporting control-flow but it's not like I'm doing anything with that anyway atm.
> I also see there is a big difference between aliasing and "unique string names" but I can't shake the feeling this may also help more generally.
> 
> Do you see a connection between the 2 or am I imagining things and pattern-matching too much on "alternative to traditional SSA use-list"?
Currently no. The current infrastructure is mainly focused on efficiently/properly supporting global value style references. For example, there is no way to refer to a symbol defined above the current symbol table. That means that mid-function symbol tables would lose the ability to reference other functions. This is an interesting use case, but I'm hesitant to try and generalize when we haven't fully served the existing needs.


================
Comment at: mlir/docs/SymbolsAndSymbolTables.md:114
+
+Described above are `Symbols`, which reside within a region of an operation
+defining a `SymbolTable`. A `SymbolTable` operation provides the container for
----------------
mehdi_amini wrote:
> The transitively nested aspect is ignored here again.
A symbol is not transitive, a symbol reference is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73590