User Details
- User Since
- Jan 2 2020, 7:36 AM (168 w, 3 d)
Tue, Mar 7
Jan 16 2023
Dec 13 2022
Dec 12 2022
Dec 1 2022
I would suggest to revise the summary and/or the title to describe that this patch now also addresses the endless interchange problem, in addition to correction in isProfitableForVectorization().
Nov 17 2022
Nov 11 2022
Hi @bmahjour and @Meinersbur , would you like to take a look on the patch?
Nov 7 2022
Nov 3 2022
According to the discussion at the LoopOptWG, I will land the patch to fix the miscompile for now. Will look into how to fully support such reduction scenarios as the next steps.
Nov 1 2022
Just one more question on the summary: when combining the two dependencies [<,=] and [=,<] that generated [<=,<=], I think we could still interchange the combined dependency? [<=,<=] is non-negative and swapping the two elements <= and <= gives a non-negative dependency. Thus we are not converting a positive dependency to a negative dependency, hence not violating the dependency rules for a valid interchange. Perhaps with the current validDepInterchange() we cannot interchange [<=,<=], but like I mentioned in the past meetings I'm rewriting validDepInterchange() to make the legality decisions be purely based on the signs of the dependence vectors before/after interchange. With the rewritten validDepInterchange() I think we can interchange [<=,<=].
Oct 31 2022
Oct 17 2022
Oct 4 2022
Sep 29 2022
Sep 26 2022
Sep 21 2022
LGTM but I'd suggest to wait a couple of days to see if people have other comments. @fhahn would you like to comment on this patch?
Sep 19 2022
Sep 16 2022
Sep 13 2022
Sep 12 2022
ping @aeubanks :)
I'm wondering if you have comments on my most recent change?
Sep 6 2022
Like I said I won't block the patch, I'm fine if other reviewers accept this patch.
Sep 2 2022
Rebased on D132581.
Sep 1 2022
Will rebase on D132581 once it is landed.
Aug 25 2022
Aug 24 2022
The changes made in this patch, notably canHoistInst() and canSinkInst() seems to have duplicate functionality from CodeMoverUtils.cpp. I'm wondering if we can reuse the code from CodeMoverUtils.cpp? Or is there something that canHoistInst() and canSinkInst() can do but functions from CodeMoverUtils.cpp cannot do?
Aug 22 2022
Aug 18 2022
Aug 17 2022
Aug 12 2022
Aug 3 2022
Updated to take the bool return value of normalize() into account, will land soon.
Addressed comments, will land soon.
Jul 29 2022
Updated the patch according to the most recent update in D130188
Hi @bmahjour @Meinersbur, I've updated the patch such that normalize() changes the Dependence object in place, and I've also leveraged the capability of the NPM to parse pass options instead of using cl::opt. I'd appreciate it if you could take a look at this version, thanks a lot!
Jul 26 2022
Jul 25 2022
Thanks @bmahjour, I've tried to address all your comments in the updated version.
Jul 24 2022
Jul 20 2022
Jul 18 2022
Thanks Michael for your suggestion, I've updated the patch accordingly.
Jul 13 2022
Updated the patch according to the discussion in the loopopt meeting. @Meinersbur I would appreciate it if you could take a second look, thanks a lot!
Jul 8 2022
Hi folks, my apologies for the delay, and thanks for the input. I do agree with you that we need to treat this scev expansion more carefully, although for some places where getNoopOrAnyExtend() is called (like the piece of code that Bjorn posted above), both Stride and TripCount are positive so it might be straightforward to use SE.getNoopOrZeroExtend(), or maybe just SE.getNoopOrAnyExtend() as how it is used now.