User Details
- User Since
- Jan 17 2017, 8:49 PM (208 w, 3 d)
Today
LGTM
The change makes sense..!
Since the patch is more succinct, I'm in favor of applying the patch than this one.
After applying D94859 + D94860 + this patch, the diff looks like this:
https://gist.github.com/aqjune/39bcc99bf7339bef74652597c9e57122 (@nikic thanks for creating the _logical functions!)
I think the majority of sound transformations are now supported.
I'll leave the diff result after setting -instcombine-unsafe-select-transform=0 at another patch that's going to follow this.
Wed, Jan 13
LGTM
Tue, Jan 12
Address comments, leave tests with FIXME
Leave a comment about the issue
LGTM
Yes, I see that such pattern is quite prevalent. I'll make a patch for this.
Thanks!
Yes, a test should be added.
I guess
ashr <4 x i16> %induction, select (i1 icmp sgt (i16 ptrtoint (i16* @c to i16), i16 1), <4 x i16> zeroinitializer, <4 x i16> <i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16)>)
is the problematic part?
If it is non-trivial to further reduce the input, you can also add this instruction to the list at ValueTrackingTest.cpp 's TEST(ValueTracking, canCreatePoisonOrUndef).
Thank you!
@jdoerfert Would it be okay if I land this after a week? D91480 also uses the semantics described in this patch as well.
Mon, Jan 11
Sun, Jan 10
Update
Leave updates in InstCombineVectorOps.cpp only
Address comments
Thu, Jan 7
Thinking about @jdoerfert 's suggestion again, it still implies that objects may have overlapping addresses regardless of lifetime. Am I understanding correctly?
It is memset(undef) in other cases
I have a question - is it possible to have the same optimization power with !nocapture only? In other words, is it purely for making analysis flow-insensitive?
Wed, Jan 6
@efriedma ping
Yeah, I believe passing null to nonnull should not immediately raise UB; it will block useful analyses. The patch is D90529, and I need to push it... Maybe it is time to reduce the number of patches that are still open by me.
To fix the old bug quite a few patches in LLVM have landed so far and it is still ongoing.
Tue, Jan 5
Resolve failures
A few of those was regressions. It was because ConstantVector::get(Arr) was unable to understand Poison array. I updated those.
LGTM
Use function_ref
Oh yes, I'll look into them
Address comments
Fixed, thanks :)
Mon, Jan 4
Updates
Rebase
Address comment
Sun, Jan 3
The first patch is here: D94002
Minor update
LGTM - Thanks!
Rebase
I'll continue splitting after working on lifetime patches
For loop unswitch, I'll work on it after D93764 is done