- User Since
- Jul 21 2014, 12:07 PM (360 w, 3 d)
Fri, Jun 11
Fri, Jun 4
Thu, Jun 3
Thu, May 27
Tue, May 25
Mon, May 24
Address Florian's comments.
Fri, May 21
May 18 2021
May 17 2021
May 7 2021
Congrats ;). Yes, I will commit it for you. Can you please rebase and retest, looks like the patch is not applying cleanly.
May 6 2021
Address Florian's comments. Also fix the testcase that I copied mine from re: --check-prefix=RM
Looks great, thanks!
May 3 2021
Apr 22 2021
Glad, you were able to get back to this.
Oct 23 2020
How about adding a total count as well? That should help quickly eye ball the ratios.
Oct 21 2020
LGTM, you may want to wait a little for other potential comments. Especially some ideas on getting the actual mnemonic.
Oct 7 2020
Aug 14 2020
@hnrklssn hi, are you panning to get back to this at some point?
Jul 29 2020
Looks reasonable to me.
Jul 20 2020
I guess they could in theory also be added directly to LowerMatrixIntrinsics.cpp. But I think it makes sense to have a separate file for various matrix-related utilities that are not directly tied to the intrinsics. I don't anticipate users outside of LowerMatrixIntrinsics.cpp anytime soon upstream, so I could also move them there. I think in the long-term we want to specify specialized, target-specific lowerings outside of LowerMatrixIntrinsics.cpp, so we might as well start with more generally accessible helpers.
Jul 17 2020
Jul 15 2020
Is this used anywhere in the patch? If not please explain the plan in the description.
Jul 14 2020
BTW, will this be shared between .cpp's? I.e. what is the justification for a header?
Jul 13 2020
Jul 8 2020
Jun 16 2020
LGTM! @nicolasvasilache can you please OK the MLIR parts if you're happy with them?
Jun 15 2020
Is there a test for passing alignment?
May 13 2020
Sorry about the delay, just a few small nits on the tests, this is pretty close. Thanks for pushing this!
Apr 29 2020
Thanks for improving this. Can you please add a small test highlighting where your change improve the diagnostics? TIA!
Apr 28 2020
Apr 5 2020
Mar 31 2020
Mar 29 2020
TODO: additional tests.
Mar 23 2020
I have a few specific comments below but overall it would be great if we could simplify VisitBBFusion to avoid recursion and invalidating the iterator...
Mar 19 2020
Can you please describe the approach in the description/in a comment?
LGTM, thanks for splitting these up like this!
Mar 11 2020
In the description when you say:
Feb 26 2020
Jan 31 2020
On the description: I would first explain what you do and then how to do, i.e. have "To motivate" and the example before the paragprah "For a given function, we traverse".
Jan 23 2020
I think that in the description/commit log you want to say a bit on the motivation, like why it's useful to observe these matrix expression in this form.
Jan 17 2020
LGTM, however it would better if this patch also converted existing code to use this new API.
Jan 8 2020
As Francis mentioned it before it would be good derive the pass name from the remark type (diag::remark_gvn_load_elim -> gvn) . I.e. I would drop the DEBUG_TYPE argument.
Jan 7 2020
Dec 20 2019
Dec 19 2019
I am confused about this patch and the previous one (D70899). Looks like they both introduce VisitLoad. Is this intentional?
Dec 17 2019
We should be able to use fmuladd instead of fmul/fadd, as this just
reduces the rounding error between operations.
Dec 12 2019
Dec 9 2019
Dec 5 2019
LGTM too. You may want to a wait a few days to give other people a chance to comment further.
Dec 3 2019
Also the existing test diffs are hard to read, please explain what's going on there.