- User Since
- Apr 30 2013, 5:34 PM (516 w, 4 d)
I'm curious about @jpienaar 's thoughts on the overall direction here
Thu, Mar 23
Wed, Mar 22
LGTM, but @rriddle should have another look
Tue, Mar 21
Mon, Mar 20
I rather not write code that is over-restrictive unnecessarily (if you think about it, the entirety of MLIR exists because we generalize early, otherwise we'd have rebuilt LLVM or XLA ;) )
Ping here @frgossen ?
Wed, Mar 15
Tue, Mar 14
You should look into the title and description of the commit: https://cbea.ms/git-commit/
I'll wait to hear at least from @mravishankar @nicolasvasilache here.
Please get reviews from @rriddle here.
This LGTM, but River may have thoughts on simplifying getChildOpIndex(), let's wait to hear back from him.
Mon, Mar 13
This assumes less threads than shards overall right?
Can you add tests please?
One more comment addressed
Rebase & address comments
Minor fix to PAssExecutionAction
Use ... instead of "/*skip region*/" comment
Sun, Mar 12
Update with one extra no-inline indirection
This LGTM, but @rriddle and @Mogball had concerned earlier, it would be great if they could take another look here!
My feeling is that we explicitly do not want a pass to depend on a specific IRDL dialect. In fact, this is already something that we do not want to have for DynamicDialect.
Having a pass relying on a dynamic dialect would be too dangerous and error-prone, and if you are already paying the cost of recompiling to write your pass, you are willing to pay the cost of compiling your dialect to C++ (in my opinion).
Also, manipulating IRDL dialects in C++ is a bit annoying, since you need to use the generic way of accessing operands/results/attributes, meaning that you have almost no guarantee that those operands/results/attributes exist in the first place.
Sat, Mar 11
Fri, Mar 10
I feel like you're using "register" when it is actually more "loading" here. (I'm not sure IRDL has a concept equivalent to "registering" dialects?)
Right now you have test failures. On my Mac locally I see:
Do you have commit access or do you need help to land this?
LGTM, thanks for being so patient through the reviews!
In general I am quite concerned about messing with the optimizer this way, this should be extremely exceptional. This'll be ad-hoc, forces a performance tradeoff specific to a given use case, and couple the "heuristic" to the exact compiler you're using (what does it do on Windows? On Mac?).
Have you tried building your project with -Os? -Oz? PGO? FullLTO?
~3.4 MB reduction in binary in release mode.
Thu, Mar 9
Tue, Mar 7
My personal motivation was to be able to ship MLIR without any static ctor.
Fixing all of LLVM seemed too much, but MLIR (when not needing the JIT) does only depends on libSupport, which was very much doable (in particular a key piece was that the command line parsing entry point is part of the lib, so we can lazy load libSupport options)
Address more comments