tensorflow
Value Name?
Hello MLIR developers,
I noticed that MLIR Value (declared as in here) does not seem to have a mechanism to have custom names. So right now, it seems like IR built using OpBuilder must produce SSA ID's with names like %0, %1, I wonder if there are better ways to assign meaningful names to MLIR Values?
It seems to me that LLVM Value can be assigned a name via the setName function documented in here, are you planning to have similar support for MLIR Value?
Hi Tian,
On Sat, Oct 12, 2019 at 2:29 PM Tian Jin [email protected] wrote:
Hello MLIR developers,
I noticed that MLIR Value (declared as in here https://github.com/tensorflow/mlir/blob/70e6056497144c1ec89b3584d5e9a57ffb16b020/include/mlir/IR/Value.h) does not seem to have a mechanism to have custom names. So right now, it seems like IR built using OpBuilder must produce SSA ID's with names like %0, %1, I wonder if there are better ways to assign meaningful names to MLIR Values?
It seems to me that LLVM Value can be assigned a name via the setName function documented in here https://llvm.org/doxygen/classllvm_1_1Value.html#a35ee267850af7c235474a8c46c7ac5af, are you planning to have similar support for MLIR Value?
We discussed it extensively, supporting it is a bit complicated so we punted it at the moment (I would welcome it if someone wanted to make it happen, but this will require some design discussions). Note also that this is a feature that in LLVM is intended only for debugging purposes, clang will elide any SSA value name in release builds for example (there is a flag that can be set on the LLVMContext to drop/ignore any value names).
-- Mehdi
Hi @joker-eph thanks for your quick response, would love to know more about the status quo:
- What were some notable conclusions of the extensive discussion? (is transcript available?) I'm curious why such feature warrants extensive discussion.
- Is it complicated in the sense that it is tricky? or in the sense that I'll have to do some readings into how LLVM implements it and come up with a similar design?
- Do you see any reasons for/against following clang's suit to elide SSA value name in release builds?
On Sat, Oct 12, 2019 at 8:21 PM Tian Jin [email protected] wrote:
Hi @joker-eph https://github.com/joker-eph thanks for your quick response, would love to know more about the status quo:
- What were some notable conclusions of the extensive discussion? (is transcript available?) I'm curious why such feature warrants extensive discussion.
Sorry, no transcribe as this predates the open-sourcing.
- Is it complicated in the sense that it is tricky? or in the sense that I'll have to do some readings into how LLVM implements it and come up with a similar design?
I was intentionally vague on why it is complicated because of conflicting views on what is the right thing to do here :) Also this was discussed right before we were thinking about evolving modules and function into operations, which we had to tackle first since we didn't want to over-fit a solution for uniquing names that would be hard-coded for the FuncOp.
I can summarize my position. There are two aspects for the SSA names:
-
description/meaning for the output of an operation:
%quotient, %remainder = idiv(%a, %b)
or %output1:2, %output:2 = multi_ouput()
(This last one is not a valid syntax at the moment, but it could be)
Naming the SSA result of operation like above shouldn't be too complex: I think we can get something reasonable with some hooks on the custom-operation printer, and it can even be generated by the ODS description if needed. We could use it just as a suffix after the current SSA value numbers for example.
This is different from the LLVM SSA Names in that the names above would be defined by the operation itself and not different for every instance of the operation. Also, it can't be customized by the creator of the operation (like in LLVM some passes may suffix values to indicate what created a value or why, like here for example: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp#L2482 ), which leads to the second aspect below.
- tracking/disambiguation
The first point would already be nice to have, but it does not answer the uniquing question. On printing we need a unique name per SSA value. It is fairly easy to just prefix/suffix the existing value numbers with the descriptive name and have something like (eliding types):
func @foo(%a, %b) { %0.quotient, %0.remainder = idiv(%a, %b) %1.output1:2, %1.output2:2 = multi_output() %2.quotient, %2.remainder = idiv(%a, %b) ... }
However that does not provide "stability" of the names across the transformation pipeline. This is what the LLVM implementation offers and that I find to be a very useful properties for debugging:
func @foo(%a, %b) { %my_name.quotient, %my_name.remainder = idiv(%a, %b) return %my_name.quotient }
func @bar(%a, %result) { for %i = 0 ... 5 { %idiv = call @foo(%a, %i) store %idiv, %result[%I] } }
After inlining an fully unrolling we could have:
func @bar(%a, %result) { %my_name.inline.unroll0.quotient, %my_name.inline.unroll0.remainder = idiv(%a, %b) store %my_name.inline.unroll0.quotient, %result[0] %my_name.inline.unroll1.quotient, %my_name.inline.unroll0.remainder = idiv(%a, %b) store %my_name.inline.unroll1.quotient, %result[1] %my_name.inline.unroll2.quotient, %my_name.inline.unroll0.remainder = idiv(%a, %b) store %my_name.inline.unroll2.quotient, %result[2] %my_name.inline.unroll3.quotient, %my_name.inline.unroll0.remainder = idiv(%a, %b) store %my_name.inline.unroll3.quotient, %result[3] %my_name.inline.unroll4.quotient, %my_name.inline.unroll0.remainder = idiv(%a, %b) store %my_name.inline.unroll4.quotient, %result[4] }
However doing requires uniquing names every time we insert an operation into a block. For instance inlining of this:
%0 = call @foo(%a, %i) %1 = call @foo(%a, %i)
Requires to rename the second inlined value:
%my_name.quotient, %my_name.remainder = idiv(%a, %b) %my_name1.quotient, %my_name1.remainder = idiv(%a, %b)
However since MLIR is very nested, this requires to traverse all the parent region and collect all names to compute the new unique one, which seems potentially quite expensive (more than LLVM where there is no nesting and one hard-coded value table on each function).
My proposal for this was to have this enabled by a flag on the Context like in LLVM, so that the cost is only bear during debugging and it should be "free" in the production path. I would make the flag part of the Context creation and not being able to be toggled later, which would simplify a bunch of invariant. Operations would have a tail-allocated pointer to a string, this would be present only if the operation is created in a context with the names enabled, which would make it totally free on the "release" path. Operations would support setName/getName, the former being a no-op when the context has the flag disabled, and the later returning an empty StringRef when there is no name (or the Context flag is disabled).
Alternative proposals at the time included having a generic attribute (__name for example) so that:
%42 = add %0, %1 {__name: "foo"}
would be printed:
%foo = add %0, %1 {__name: "foo"}
This allow to have "per instance" names but the uniquing is still done at printing time (so it does not allow to track a single value across the pipeline.
Another proposal was to look into the "location" of the operation to store the name, but the uniquing aspect wasn't really addressed either.
- Do you see any reasons for/against following clang's suit to elide SSA value name in release builds?
The reason was performance (time and memory usage), see my commit description here: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160307/338361.html (and as I mentioned above, the cost on MLIR can even be higher than for LLVM)
Hope this helps.
-- Mehdi
Another proposal was to look into the "location" of the operation to store the name, but the uniquing aspect wasn't really addressed either.
I was pro using locations :) As we do have namedlocations where one can assign a meaningful name as a location. We also track locations during transformations (for example, creating fused locations) and they serves a lot of the same purposes than the name to me. But as mentioned, locations need not be unique, which could make tracking ambiguous. For that one could, same as a above, have a debug mode/flag that ensures locations are unique (e.g., each location created in this debugging mode could instead be a fusedlocation between a unique ID and the regular location).
This should be a debugging feature which should not be relied on during the regular flow (e.g., these names should not be load bearing).
