[PATCH] D36059: [memops] Add a new pass to inject fast-path code for specific library function calls.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Transforms/Scalar/FastPathLibCalls.cpp:90
+  /// Checks whether a value is known non-zero at a particular location.
+  bool isKnownNonZero(Value *V, Instruction &I) {
+    // If value tracking knows enough, we're done.
----------------
davidxl wrote:
> chandlerc wrote:
> > davidxl wrote:
> > > Is it better to rely on instcombine and cfg simplification to get rid of the redundant zero guard which can be more general?
> > Sadly, they're *less* powerful than this. You'd need PRE or something similar to get it, powered by PredicateInfo and GVN. Maybe JumpThreading and LVI could get it?
> > 
> > All of these seem really heavy weight to run this late in the pipeline. =/
> Cases like the following can be handled by -instcombine + -simplfycfg.   Wrapping the second redundant test into another check also works fine.
> 
> What are the interesting cases that can not be handled?
> 
> efine void @set1_nonzero1(i8* %ptr, i64 %size) {
> ; CHECK-LABEL: define void @set1_nonzero1(
> entry:
>   %zero_cond = icmp eq i64 %size, 0
>   br i1 %zero_cond, label %exit, label %test
> 
> test:
>   %nonzero_cond = icmp ne i64 %size, 0
>   br i1 %nonzero_cond, label %call, label %exit
> 
> call:
>   call void @llvm.memset.p0i8.i64(i8* %ptr, i8 15, i64 %size, i32 1, i1 false)
>   ret void
> 
> exit:
>   ret void
> }
> 
> declare void @llvm.memset.p0i8.i64(i8* writeonly, i8, i64, i32, i1)
I'm honestly surprised instcombine+simplify-cfg would get this. It seems outside of their purview to do this kind of predicate analysis... I couldn't find where it does this in a cursory glance through the code.

Anyways, we don't run instcombine after this pass, but only inst simplify. Not sure we want to add that expensive of a pass when this code can just handle the specific cases it wants.


https://reviews.llvm.org/D36059