- User Since
- Jul 21 2014, 12:07 PM (297 w, 1 d)
Sun, Mar 29
TODO: additional tests.
Mon, Mar 23
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...
Thu, Mar 19
Can you please describe the approach in the description/in a comment?
LGTM, thanks for splitting these up like this!
Wed, Mar 11
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.
Nov 20 2019
Oct 29 2019
Sep 24 2019
Sep 20 2019
Sep 17 2019
Some minor test questions/suggestions. Feel free to commit after addressing.
Sep 6 2019
Does this also apply to right shifts?
Aug 20 2019
No, concerns. Looks good to me too.
Apr 18 2019
Mar 27 2019
LGTM, thanks for fixing this!
Mar 26 2019
Mar 14 2019
Feb 26 2019
Feb 8 2019
Feb 6 2019
LGTM with the comment.
Jan 25 2019
This is great, please add some tests or check for remarks in existing tests (e.g. for the recursive case).
Awesome and thanks for the test. LGTM!
Jan 24 2019
Can you please describe the user experience?
Dec 13 2018
Actually this has been failing for 8 hours. So reverted in r349117. Also reverted your attempt to update the test. It wasn't updating the right test: r349118
Nov 27 2018
Nov 26 2018
Hi Florian, are you saying that in this case (known unsafe dep) we would still vectorize the loop (and always fail at run-time)?
Jul 20 2018
LGTM, you have trailing whitespace in one of the hunks, please clang-format.
Jul 16 2018
May 17 2018
Mostly nits. LGTM with the requested changes.
May 7 2018
Mostly small things except for the question on whether we should only compute this when the remark is actually enabled.
Apr 17 2018
Looks pretty straight-forward.
Mar 20 2018
While it's preferred to use ORE as an analysis pass, sometimes that's hard (e.g because it's a function pass, or simply because it's hard to thread the ORE instance through the many layers). In these cases it's fine to construct one inline. When remarks are requested this will amount to repopulating BFI for the function as the ORE instance is created.
Mar 13 2018
@inglorion, I am inclined to recommit this unless I hear from you in a few days:
Mar 12 2018
Mar 7 2018
@inglorion Is this from a bot, I didn't see any failures? A bit more info would be helpful.