[llvm] 8d48766 - [CVP] Soften SDiv into a UDiv as long as we know domains of both of the operands.

[Florian Hahn via llvm-commits]

> On Jul 18, 2020, at 16:00, Roman Lebedev via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 
> Author: Roman Lebedev
> Date: 2020-07-18T17:59:56+03:00
> New Revision: 8d487668d09fb0e4e54f36207f07c1480ffabbfd
> 
> URL: https://github.com/llvm/llvm-project/commit/8d487668d09fb0e4e54f36207f07c1480ffabbfd
> DIFF: https://github.com/llvm/llvm-project/commit/8d487668d09fb0e4e54f36207f07c1480ffabbfd.diff
> 
> LOG: [CVP] Soften SDiv into a UDiv as long as we know domains of both of the operands.
> 
> Yes, if operands are non-positive this comes at the extra cost
> of two extra negations. But  a. division is already just
> ridiculously costly, two more subtractions can't hurt much :)
> and  b. we have better/more analyzes/folds for an unsigned division,
> we could end up narrowing it's bitwidth, converting it to lshr, etc.
> 
> This is essentially a take two on 0fdcca07ad2c0bdc2cdd40ba638109926f4f513b,
> which didn't fix the potential regression i was seeing,
> because ValueTracking's computeKnownBits() doesn't make use
> of dominating conditions in it's analysis.
> While i could teach it that, this seems like the more general fix.
> 
> This big hammer actually does catch said potential regression.
> 
> Over vanilla test-suite + RawSpeed + darktable
> (10M IR instrs, 1M IR BB, 1M X86 ASM instrs), this fires/converts 5 more
> (+2%) SDiv's, the total instruction count at the end of middle-end pipeline
> is only +6, so out of +10 extra negations, ~half are folded away,
> and asm instr count is only +1, so practically speaking all extra
> negations are folded away and are therefore free.
> Sadly, all these new UDiv's remained, none folded away.
> But there are two less basic blocks.
> 
> https://rise4fun.com/Alive/VS6
> 
> Name: v0
> Pre: C0 >= 0 && C1 >= 0
> %r = sdiv i8 C0, C1
>  =>
> %r = udiv i8 C0, C1
> 
> Name: v1
> Pre: C0 <= 0 && C1 >= 0
> %r = sdiv i8 C0, C1
>  =>
> %t0 = udiv i8 -C0, C1
> %r = sub i8 0, %t0
> 
> Name: v2
> Pre: C0 >= 0 && C1 <= 0
> %r = sdiv i8 C0, C1
>  =>
> %t0 = udiv i8 C0, -C1
> %r = sub i8 0, %t0
> 
> Name: v3
> Pre: C0 <= 0 && C1 <= 0
> %r = sdiv i8 C0, C1
>  =>
> %r = udiv i8 -C0, -C1
> 
> Added: 
> 
> 
> Modified: 
>    llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
>    llvm/test/Transforms/CorrelatedValuePropagation/sdiv.ll
> 
> Removed: 
> 
> 
> 
> ################################################################################
> diff  --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
> index 284312eaf822..fb7a005708e5 100644
> --- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
> +++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
> @@ -607,6 +607,12 @@ static bool isNonNegative(Value *V, LazyValueInfo *LVI, Instruction *CxtI) {
>   return Result == LazyValueInfo::True;
> }
> 
> +static bool isNonPositive(Value *V, LazyValueInfo *LVI, Instruction *CxtI) {
> +  Constant *Zero = ConstantInt::get(V->getType(), 0);
> +  auto Result = LVI->getPredicateAt(ICmpInst::ICMP_SLE, V, Zero, CxtI);
> +  return Result == LazyValueInfo::True;
> +}
> +
> static bool allOperandsAreNonNegative(BinaryOperator *SDI, LazyValueInfo *LVI) {
>   return all_of(SDI->operands(),
>                 [&](Value *Op) { return isNonNegative(Op, LVI, SDI); });
> @@ -672,24 +678,65 @@ static bool processSRem(BinaryOperator *SDI, LazyValueInfo *LVI) {
> }
> 
> /// See if LazyValueInfo's ability to exploit edge conditions or range
> -/// information is sufficient to prove the both operands of this SDiv are
> -/// positive.  If this is the case, replace the SDiv with a UDiv. Even for local
> +/// information is sufficient to prove the signs of both operands of this SDiv.
> +/// If this is the case, replace the SDiv with a UDiv. Even for local
> /// conditions, this can sometimes prove conditions instcombine can't by
> /// exploiting range information.
> static bool processSDiv(BinaryOperator *SDI, LazyValueInfo *LVI) {
> -  if (SDI->getType()->isVectorTy() || !allOperandsAreNonNegative(SDI, LVI))
> +  if (SDI->getType()->isVectorTy())
>     return false;
> 
> +  enum class Domain { NonNegative, NonPositive, Unknown };
> +  auto getDomain = [&](Value *V) {
> +    if (isNonNegative(V, LVI, SDI))
> +      return Domain::NonNegative;
> +    if (isNonPositive(V, LVI, SDI))
> +      return Domain::NonPositive;
> +    return Domain::Unknown;
> +  };
> +
> +  struct Operand {
> +    Value *V;
> +    Domain Domain;
> +  };
> +  std::array<Operand, 2> Ops;
> +  for (const auto &I : zip(Ops, SDI->operands())) {

It looks like this is causing the following warning:

lvm-project/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:703:20: warning: loop variable 'I' is always a copy because the range of type 'detail::zippy<detail::zip_shortest, array<Operand, 2> &, iterator_range<Use *> >' does not return a reference [-Wrange-loop-analysis]
  for (const auto &I : zip(Ops, SDI->operands())) {