- User Since
- Jun 6 2014, 4:30 PM (294 w, 46 m)
Sun, Jan 19
lgtm, although it would be nice to independently check if isLoopEntryGuardedByCond can be sped up.
Sun, Jan 12
Tue, Jan 7
Nov 17 2019
Can you please describe what was wrong about the unit tests in the commit message?
Oct 18 2019
Oct 4 2019
Sep 18 2019
This patch individually is fine, but why are you setting scalar-evolution-max-iterations in the first place? It is a debug option that end-users should ideally never have to change.
I don't work on statepoints anymore.
Sep 13 2019
Sep 11 2019
Sep 7 2019
Aug 24 2019
Aug 6 2019
Aug 4 2019
Let's make this a friend function, I'm already not super happy with how wide SCEV's API is. You can make the diff lambda a member function of ScalarEvolutionsTest to make this work.
Jul 31 2019
Jul 30 2019
We use computeConstantDifference in some downstream code, and I was surprised to find out that it fails to compute (X - X) difference, unless X is a SCEVAddExpr.
Jul 8 2019
I think Alive and this are complementary. Alive is useful in proving the correctness of _new_ transformations, whereas this pass should let us quickly figure out if a large unreduced application is misbehaving it branches on poison.
Jul 5 2019
Jul 4 2019
I think this is an excellent idea!!
Jun 23 2019
Jun 21 2019
Jun 20 2019
it seems like there are a lot of cases where SCEV's exit count reasoning is stronger than it's isKnownPredicate reasoning.
Jun 17 2019
I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.
Jun 14 2019
Jun 6 2019
I should just include the resultng test change in this commit and not worry about it.
May 15 2019
May 14 2019
May 9 2019
Thanks for the quick fix!
May 8 2019
May 7 2019
May 6 2019
Only minor nitpicky comments, will LGTM once these are fixed.
May 5 2019
LGTM. If possible (and you haven't already) try bootstrapping clang in a few configurations to double check things are okay.
May 4 2019
I'm not sure if the current fix is correct. I'd suggest doing one of the following:
Apr 28 2019
and as I found out also stack overflows
This change in isolation LGTM (modulo some minor comments inline) -- at the very least having separate umin / smin nodes makes min expressions more readable (as demonstrated by the updates to the test cases).
Apr 27 2019
I got confused on the terminology
Apr 18 2019
Apr 13 2019
Apr 10 2019
Apr 4 2019
Ah thanks, I was missing the global nature of physical pointers. I couldn't find this described anywhere (besides some of those things mentioned at a tutorial at EuroLLVM). If this is not described anywhere, do you think it would make sense to add it to the AliasAnalysis documentation page, for example?
Apr 2 2019
Apr 1 2019
Mar 31 2019
I'm not sure about this. You could have:int32* ptr0 = malloc(4); int32* ptr1 = malloc(4); if (ptr0+1 != ptr1) return; int32* ptr = (int*)(int64)(ptr0+1);
in which ptr would alias ptr1. But if you transform ptr to ptr0+1 then it would not alias ptr1.
I am probably missing something, but I am not sure how such a case would be possible with this patch? It specifically looks for a inttoptr(and(ptrtoint, C)) sequence, where C is such that the (logical) destination of the pointer remains unchanged. Unfortunately I do not think the LangRef is clear about 'irrelevant' bits in pointers (due to alignment or address spaces)
Things that are UB in C++ are not necessarily UB in LLVM. For instance you could capture a pointer by doing something like this:
Mar 29 2019
Fix bug: when trying to look up the smax/umax SCEV again (after simplification) we can't use the older ID since it was computed based on the unsimplified operands.
Sorry, this is buggy, one moment.
Mar 20 2019
Mar 19 2019
Mar 18 2019
This change needs a test, as Roman said.
Mar 16 2019
Ok, looks like we're somewhat sloppy with our choice of insertion points here (and it should not be incumbent on you to fix this). Can you please add a comment that mentions that this is due to indvarsimplify?
Mar 14 2019
Apologies for the late review Warren.
Mar 13 2019
langref part seems to be missing.