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

[David Li via Phabricator via llvm-commits]

davidxl 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.
----------------
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)


================
Comment at: lib/Transforms/Scalar/FastPathLibCalls.cpp:147
+
+  struct FastPathMemOpFramework {
+    BasicBlock *HeadBB;
----------------
chandlerc wrote:
> davidxl wrote:
> > Framework can mean something much different. How about just call it 'Info'?
> It's more than the information. It represents the actual fastpath CFG framework (for lack of a better word) that has been injected into the function and needs to be populated with the particular memop's logic...
> 
> That said, I could totally add a comment. =] Would that help clarify enough?
Sure, some comments will do.


https://reviews.llvm.org/D36059