- User Since
- Nov 16 2012, 6:02 PM (330 w, 5 d)
The audience of this are people who are defining new target intrinsics.
I don't understand what this is supposed to mean to who the audience of this statement is?
Tue, Mar 19
We need to make progress on this, and I'd like to suggest a path forward...
Fri, Mar 15
LGTM. Maybe we can also add the test case from the PR as one of Clang's regression tests?
Thu, Mar 14
I'm really afraid that this isn't sound. We've had a number of issues in this space, and we've always resisted attempts to make BasicAA look through inttoptr/ptrtotint. Once you convert to an integer, control dependencies can effectively add additional underlying objects. In cases where this is sound, why not fold away the whole inttoptr(and(ptrtoint)) in the first place?
Thu, Mar 7
Thanks to everyone who worked on this! LGTM.
Tue, Mar 5
Mon, Mar 4
Thu, Feb 28
Tue, Feb 26
Mon, Feb 25
Is this STW instruction something that instruction scheduling might move? (@nemanjai ?)
Feb 12 2019
Feb 7 2019
Feb 6 2019
Feb 5 2019
LGTM. I suppose that I thought that this was needed at some point to parse (1, 0) instead of (-1, 0), but if this is not needed in practice, then it should be simplified.
Feb 4 2019
Jan 30 2019
Jan 28 2019
Jan 26 2019
Jan 25 2019
Agreed, but it might be worthwhile to choose a guinea pig (openmp) first. I find it difficult to think of all the possible consequences of changes like this.
Jan 24 2019
libgomp.so, libiomp5.so: My understanding is that these symlinks exist solely for backward compatibility. This patch currently doesn't affect them (doesn't bother to install them to Clang-dedicated directories). Any reason to change that?
Have you measured the compile-time impact?
Jan 23 2019
In general, this makes sense to me.
Jan 22 2019
Jan 21 2019
Jan 17 2019
Please, in the future, make sure you post full-context patches.
Can we run EarlyCSE after CGP (instead of trying to do these kinds of point fixes)?
Jan 11 2019
Jan 10 2019
Jan 9 2019
Jan 7 2019
Jan 4 2019
Jan 2 2019
Jan 1 2019
Dec 31 2018
Dec 30 2018
Dec 29 2018
So this rell-request
Dec 28 2018
Dec 27 2018
Please fixup the comments, but LGTM.
Dec 21 2018
I can certainly split this into more patches if that is preferred. I'm unsure though how to test the metadata and abstract call sites without a user.
A high level strategy comment. I think you're trying to go a bit too far in one patch. I'd advise first writing a patch which does nothing but implement the IPConstProp adjustments "the hard way" (i.e. without the AbstractCallSite) and focus on defining the metadata with the semantics you once. To really justify the complexity of the abstraction, you need more than one user. You could add another user to this patch, but doing so will just slow down the review. Separating this into a series of patches (IPConstProp w/metadata def, second user, then introduce abstraction) will make it much easier to see the necessity and make each piece more easily reviewable.
LGTM. This certainly looks like a better solution; the previous code was indeed trying to make sure that the containing CR field was defined before the mfocrf. Thanks!
Dec 20 2018
Thanks for updating this patch. It will be very useful!
Dec 19 2018
Aside from the requested renaming (see below), this LGTM.
Dec 18 2018
Minor typo noted below, but otherwise, LGTM (to avoid any misunderstanding: this should be committed after the LLVM change lands).
Yes, volatile loads and stores access the "same" state as inaccessiblememonly functions, I think.
Also, cost model changes can be tested directly. See tests in test/Analysis/CostModel/PowerPC/
Dec 17 2018
LGTM. Thanks, I think this naming is more accurate and useful.
Such explicit transformations should take precedence over disabling heuristics.
Dec 14 2018
I think that this is okay. There's no semantic change nor a change in control dependence. For this to cause a problem, there would need to be metadata that would change meaning or validity when the instruction changes from an invoke to a call (where the invoke is semantically equivalent to the call).