- User Since
- Aug 18 2016, 4:39 AM (100 w, 3 d)
Fri, Jul 20
Address review comments, thanks!
Address next round of review comments, thanks! Also add some comments.
Update to account for DoesKMove.
Flip default for DoesKMove.
Thu, Jul 19
Address comments, thanks! I will add more comments tomorrow.
Delete allocated InterleaveGroups in destructor.
Moved getMemInstAlignment and getMemInstAddressSpace to IR/Instructions.h which already contains similar helpers. Should I rename them to getLoadStoreAlignment and getLoadStoreAddressSpace to be more in line with the existing getLoadStorePointerOperand?
Renamed KDominatesJ to DoesKMove as suggested by Eli in D47475.
Thanks for all the feedback! My initial plan was to make VPlan based loop vectorization SLP aware, to improve on the cases mentioned earlier, where currently we do not do the best thing in LoopVectorize. That should take a bit of load off SLPVectorize, but would not replace the current SLPVectorizer for now.
Abandoned in favor of D49408
Abandoned in favor of D49408
Wed, Jul 18
ping. The langref now says the following about !nonnull loads: The existence of the !nonnull metadata on the instruction tells the optimizer that the value loaded is known to never be null. If the value is null at runtime, the behavior is undefined.. With the value being null at runetime being UB, keeping nonnull on dominating loads should be ok IIUC.
I just collected stats with this patch on the test-suite with LTO. The only difference in stats counters across IPSCCP and SCCP is that with this patch, we get 2 more instructions eliminated in SCCP (sccp.NumInstRemoved).
Tue, Jul 17
I temporarily reverted this patch in rC337255 because it triggers a static_assert in PointerIntPair on ARM (e.g. http://lab.llvm.org:8011/builders/clang-cmake-armv7-global-isel/builds/1330, http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/1394)
Mon, Jul 16
Thank you very much Eli! There was a problem with the SmallSet iterator because of MSVC STL's std::set iterator not being trivially copy constructible. I'll commit this once I manged to resolve the SmallSet iterator issue.
Sorry for coming back to this just now. Unfortunately I won't have time to turn this into a forward-style analysis before the 7.0 branch. IMO it would be fine to get this in for the 7.0 branch, to limit compile time in edge cases, and creating a ticket to improve the situation. What do you think @davide ? In general, currently we are only supporting splitting a very limited set of call sites, so we would have to limit the forward-analysis to the relevant call sites, otherwise we end up doing (much) more work on average.
Fri, Jul 13
err, hit send too soon: not trivially copy constructible in MSVC's STL.
The windows bots have surfaced another problem: set<T>::const_iterator is not trivially copy constructible... Not sure how to best work around that yet.
Thanks for clarifying this Eli! Treating loading a null value of a nonnull load as UB should allow us to not drop nonnull from loads in some cases (D47339)
Wed, Jul 11
Use SmallSet + AssertingVH
Recommitted as rL336805, with only the subset of is_trivially_XXX provided by GCC 4.8 and llvm/Support/type_traits.h
Tue, Jul 10
Thanks Eli! I am not really happy with the solution, but as discussed in D48541 solving that generally seems quite an invasive change.
I have created D49126 to deal with this problem locally in PredicateInfo, which seems the only place where this is currently causing any problems. It is easier to do there, as we can clean up any ssa_copy calls and declarations when destroying PredicateInfo. I also raised https://bugs.llvm.org/show_bug.cgi?id=38117 for a general solution.
Mon, Jul 9
Thanks @chandlerc, I think it is too fragile and I want to avoid unnecessary changes here. To get PredicateInfo working with unnamed types, it is probably easier to use a custom mangling for ssa_copy there; we can rely on the order we create ssa_copy calls and clean up all declarations we created after the predicateinfo is destroyed.
Fri, Jul 6
Thu, Jul 5
I've updated the patch to generate the IDs for unnamed types at creation time and also updated the bitcode writer to write the unamed types first, in the order they were created. This way, the IDs matche between serializing and de-serializing.
Wed, Jul 4
Tue, Jul 3
I've finally had some time to come back to this change (sorry for the long delay!) and committed an updated version of this patch as rL336098.