- User Since
- Apr 15 2020, 5:28 PM (62 w, 5 h)
Dec 23 2020
Thanks for the review! Could you please commit this for me? Layton Kifer <email@example.com>
Dec 17 2020
Thank you everyone for looking at this! Could one of you please commit this for me? Layton Kifer <firstname.lastname@example.org>
Dec 15 2020
Add test case.
Dec 14 2020
Dec 6 2020
Thanks you for this! I apologise I hadn't gotten to it yet.
Dec 3 2020
are we excluding vector types intentionally (do we have any test coverage?) or is that planned as a follow-up? Either way, we should add a code comment to explain.
Thanks for the review! Could you commit this for me please? Layton Kifer <email@example.com>
Dec 1 2020
Thanks for the review! If no one else objects, could you commit this for me? Layton Kifer <firstname.lastname@example.org>
Nov 30 2020
Nov 27 2020
The extra lines were only because I thought it was more readable. Not needing to pass the VT would also help in that regard though.
I'm not sure I understand the concern here. If its just the requirement to specify a VT, maybe we should add an overload of getLogicalNOT that doesn't require one? (and maybe getNOT while we're at it) It seems reusing the VT of whatever you're flipping is probably what you want most of the time anyway.
Nov 26 2020
Avoid folding cases where (not i1 x) can already be folded
Nov 24 2020
Nov 23 2020
Remove speculatively create (not i1 x), when not used.
Nov 20 2020
Nov 19 2020
Nov 18 2020
Nov 17 2020
@RKSimon could you commit these for me? Thanks in advance!
Nov 16 2020
Nov 3 2020
Rebase after rG40f7ac1a8f61
Nov 2 2020
Prevents some regressions is D90113
Oct 29 2020
Oct 28 2020
Oct 26 2020
Fixed some of the regressions.
Oct 24 2020
First attempt, feedback appreciated.
Oct 21 2020
Thanks for the review! Could you commit this for me? Layton Kifer <email@example.com>
Oct 20 2020
Sep 24 2020
Thanks for the review! Could you commit this for me?
Sep 4 2020
Aug 29 2020
Jul 29 2020
Jul 20 2020
Jul 13 2020
Remove hasAddressTaken check
Jun 25 2020
Jun 24 2020
Jun 23 2020
I'll wait and rebase after D82085.
Jun 22 2020
Jun 20 2020
Jun 18 2020
Relevant llvm-dev thread. Noncapture use of locals disabling TailRecursionElimination
Jun 4 2020
Fixed by D80844
Jun 2 2020
I added your example as a test case. It is worth noting that even at -O1 clang hoists and merges the recursive calls and then branches to the correct accumulator, which would prevent either from getting removed. Also, the multiply by 2 gets converted to a shift, which would also prevent that path from getting removed. I'm going to take a look at creating another patch after this to convert shifts back to multiplies if it would enable an elimination.
Jun 1 2020
What happens if there are multiple tail calls in a function using different accumulators?
Add test case for multiple return values while accumulating.
May 29 2020
May 27 2020
I don't currently have a bugzilla account but I will try to sign up tomorrow so I can reply to this. I'll give a quick run down below though.
Was there anything else I needed to do for this?
May 16 2020
Thank you. I appreciate the help.
May 15 2020
Address comments. Added a test for multiple eliminations.
May 8 2020
May 5 2020
Name and email would be great. Thank you.
Whoops, deleted one to many static keywords.
Nevermind, I'm an idiot.
Ah, I misunderstood, fixed now.
May 4 2020
In general I agree that keeping mutable state in the class like this is not optimal. In this case though, I do believe it is better than needing to trace the variable up though several functions to find were it is declared, when in is only used in the bottom few functions. I added a comment stating that these variables are populated in createTailRecurseLoopHeader to hopefully make them a little easier to understand. As for moving the functions out of the class, I was trying to avoid cluttering the code with a bunch of "TRE->" everywhere, but if we think it is cleaner to have this be a struct that is passes around instead of a class with member functions, I am okay with that.
Added anonymous namespace. Renamed two variables to be more clear. Added some comments.
May 2 2020
Apr 29 2020
I have gotten a version of this that does the accumulation at the end mostly working. I will finish cleaning it up and try to split it into a few different patches as I reworked a fair bit, and found a few addition improvements I think we can make. I went ahead and updated this diff in case we want to consider it a first step of incremental improvement.
Apr 27 2020
I'm trying this approach now, but I'm having a hard time seeing a good way to transform this case:
Apr 24 2020
Suppose, instead, the accumulator always starts at zero.
This now fixes both repos of the crash. I removed a test that relied on the bug and actually didn't produce valid output. Added the case with a single branch as a test case. However, all this switch handling really does is propagate a constant. Maybe we should remove it entirely and expect that the constant propagation passes do this for us.
Apr 23 2020
Apr 15 2020
noticed minor typo in the regression test