[PATCH] D28275: [EarlyCSE] infer conditional equalities within basic blocks

[Daniel Berlin via llvm-commits]

On Tue, Jan 3, 2017 at 7:20 PM, Philip Reames via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> reames created this revision.
> reames added reviewers: sanjoy, hfinkel, gberry, majnemer.
> reames added a subscriber: llvm-commits.
> Herald added a subscriber: mcrosier.
>
> Both llvm.experimental.guard and llvm.assume intrinsics can introduce
> facts in the middle of a basic block.  Previously, EarlyCSE was not
> exploiting these facts when visiting later instructions in the same block.
>
> Worth noting is that the choice to visit each operand does make the
> algorithm O(n^2) in the number of instructions.  However, this is not new.
> We have to hash the operands, so we're already O(n^2).  Apparently, most
> real code doesn't consist of long series of calls which consume each
> previous instruction.  Who knew?
>
>
Can you quantify how much this buys us vs compile time cost?

These should all be caught by GVN, so i'd like to understand what the
motivation is.
If it's essentially free, great. Otherwise, i'd like to understand what the
end state is (IE when we declare EarlyCSE good enough).


> https://reviews.llvm.org/D28275
>
> Files:
>   lib/Transforms/Scalar/EarlyCSE.cpp
>   test/Transforms/EarlyCSE/guards.ll
>
>
> Index: test/Transforms/EarlyCSE/guards.ll
> ===================================================================
> --- test/Transforms/EarlyCSE/guards.ll
> +++ test/Transforms/EarlyCSE/guards.ll
> @@ -180,3 +180,20 @@
>    store i32 600, i32* %ptr
>    ret void
>  }
> +
> +define i32 @test7(i32 %val) {
> +; After a guard has executed the condition it was guarding is known to
> +; be true.  Unlike test3, uses the same condition for both guards and
> +; return value.
> +
> +; CHECK-LABEL: @test7(
> +; CHECK-NEXT:  %cond0 = icmp slt i32 %val, 40
> +; CHECK-NEXT:  call void (i1, ...) @llvm.experimental.guard(i1 %cond0) [
> "deopt"() ]
> +; CHECK-NEXT:  ret i32 -1
> +
> +  %cond0 = icmp slt i32 %val, 40
> +  call void(i1,...) @llvm.experimental.guard(i1 %cond0) [ "deopt"() ]
> +  call void(i1,...) @llvm.experimental.guard(i1 %cond0) [ "deopt"() ]
> +  %rval = sext i1 %cond0 to i32
> +  ret i32 %rval
> +}
> Index: lib/Transforms/Scalar/EarlyCSE.cpp
> ===================================================================
> --- lib/Transforms/Scalar/EarlyCSE.cpp
> +++ lib/Transforms/Scalar/EarlyCSE.cpp
> @@ -600,13 +600,6 @@
>              DEBUG(dbgs() << "EarlyCSE CVP: Add conditional value for '"
>                    << CondInst->getName() << "' as " <<
> *ConditionalConstant
>                    << " in " << BB->getName() << "\n");
> -            // Replace all dominated uses with the known value.
> -            if (unsigned Count =
> -                    replaceDominatedUsesWith(CondInst,
> ConditionalConstant, DT,
> -                                             BasicBlockEdge(Pred, BB))) {
> -              Changed = true;
> -              NumCSECVP = NumCSECVP + Count;
> -            }
>            }
>
>    /// LastStore - Keep track of the last non-volatile store that we
> saw... for
> @@ -622,6 +615,20 @@
>    for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E;) {
>      Instruction *Inst = &*I++;
>
> +    // Use the available value table to replace any operands we can.  This
> +    // kicks in when we've inferred a control dependent equivelence
> fact.  Note
> +    // that this does make the algorithm O(I^2) in the worst case (a
> series of
> +    // calls which each use the previous instructions), but this was
> already
> +    // true given we have to hash all the operands as well.
> +    for (Value *V : Inst->operands())
> +      if (Instruction *I = dyn_cast<Instruction>(V))
> +        if (SimpleValue::canHandle(I))
> +          // See if the instruction has an available value.  If so, use
> it.
> +          if (Value *Rep = AvailableValues.lookup(I)) {
> +            Inst->replaceUsesOfWith(V, Rep);
> +            NumCSECVP++;
> +          }
> +
>      // Dead instructions should just be removed.
>      if (isInstructionTriviallyDead(Inst, &TLI)) {
>        DEBUG(dbgs() << "EarlyCSE DCE: " << *Inst << '\n');
> @@ -659,8 +666,14 @@
>                dyn_cast<Instruction>(cast<CallInst>(Inst)->getArgOperand(0)))
> {
>          // The condition we're on guarding here is true for all dominated
>          // locations.
> -        if (SimpleValue::canHandle(CondI))
> -          AvailableValues.insert(CondI, ConstantInt::getTrue(BB->
> getContext()));
> +        if (SimpleValue::canHandle(CondI)) {
> +          DEBUG(dbgs() << "EarlyCSE CVP: Add conditional value for '"
> +                << CondI->getName() << "' as true after "
> +                << Inst->getName()  << "\n");
> +
> +          auto *True = ConstantInt::getTrue(BB->getContext());
> +          AvailableValues.insert(CondI, True);
> +        }
>        }
>
>        // Guard intrinsics read all memory, but don't write any memory.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170104/f53e866c/attachment.html>