User Details
- User Since
- Jan 7 2020, 7:38 AM (54 w, 6 d)
Today
Adding Thomas and Christian to see whether they have comments wrt. the GPU operations.
Thanks for cleaning this up.
Thu, Jan 21
Thanks!
Hmm, this turned out more complex than I had thought. I had a simple push/pop in mind. If that is not enough, lets keep it at the simple version for now.
Wed, Jan 20
Thanks for cleaning this up!
Thanks for the explanations!
Please fix comments.
Tue, Jan 19
Thanks!
This still seems a bit like a hack to me but it is better than twiddling with runtime paths.
Thanks for adding these. Beyond coding style this looks great!
Mon, Jan 18
I don't mind adding this generic default cast but I hope dialect conversion will remain configurable as to which cast is being used.
Nice cleanup, thanks!
Fri, Jan 15
This also adds the lowering to LLVM. Maybe mention this in the commit message or split it out.
Tue, Jan 12
Could you wordwrap this to make it easier to read?
Mon, Jan 11
Thu, Jan 7
Thanks for fixing these. Just some remarks while looking through the changes.
Tue, Jan 5
Thanks.
Dec 23 2020
Thank you! Big improvement in naming.
Thanks for cleaning this up!
Dec 22 2020
Dec 21 2020
Dec 16 2020
So, to conclude here, it seems to me the best way forward is to move this dialect conversion ops to their own dialect and make it clear that they are essentially cast operations between tensor and memref for the purpose of conversion. This is a discussion for discourse, though.
Thanks!
Dec 15 2020
lgtm with extra test.
Thanks!
Dec 14 2020
There should be some test that captures this.
Dec 10 2020
Dec 9 2020
First, the work @silvas did on improving how bufferization works in MLIR core brought a great cleanup with it and the outcome is much nicer than what we had before. Thank you for investing your time and thought on it. A lot of what got cleaned up had to do with how operations get bufferized and how we can do this partially and incrementally, a functionality that we did not have before and that we are already heavily using now. Awesome!
This seems to do the right thing, I just don't understand why.
Can you add a test?
Dec 8 2020
Dec 4 2020
I disagree with @silvas on this. This particular change does not take any opinion on the discussion you reference. It merely makes the hard-wired current behavior configurable by choosing different ops. It does not mandate how the decision which free operation to use is done.
Dec 3 2020
Dec 2 2020
Thanks!
Dec 1 2020
Thanks, much clearer now. I am confused a bit by the types that get used but otherwise this looks great.
Nov 30 2020
Nov 27 2020
Cleaning this up in a follow-on is fine too. Please let @ftynse comment, too.
Nov 26 2020
Fix test
PTAL. This fixes a use after free.
Clean up test invocation
I realized that we cannot rewrite all ReturnLike operations, as they require rewriting of their parent which cannot be done generically. PTAL the changes.
Do not rewrite ReturnLike ops in general.
Thanks for the reviews!
Comments
Thank you for cleaning this up. I was not aware that the llvm conversion does not have Op specific patterns, yet.
Frederik, can you take a look? Sean was happy for this to move forward with his comments addressed, which I did.
Split out finalizing pass.
Nov 25 2020
PTAL.
Rework to support terminators that implement interfaces.
Thanks. Just some nits.
Nov 24 2020
I am fine either way, just a suggestion.
I assumed these operations are in the same category as the LLVM::DialectCastOp (also known as llvm.mlir.cast). There it says
Nov 23 2020
Thank you!
Nov 20 2020
Nice, thanks!
Thanks for the reviews!
Comments