[llvm-commits] [llvm] r169049 - /llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

[Pedro Artigas]

Hello All,

I believe the comments at the beginning of the function are descriptive enough, here are the comments copied for your convenience

//
// Detect pattern:
//
// log2(Y*0.5)
//
// And check for corresponding fast math flags
//

static void detectLog2OfHalf(Value *&Op, Value *&Y, IntrinsicInst *&Log2) {

The name of the function is also a pseudo comment, do you feel like it is not enough? What is the problem?

Thanks

Pedro


On Nov 30, 2012, at 2:52 PM, Eric Christopher <echristo at gmail.com> wrote:

> Hi Pedro,
> 
> Looking much better. How about some comments now?
> 
> -eric
> 
> 
> On Fri, Nov 30, 2012 at 2:47 PM, Pedro Artigas <partigas at apple.com> wrote:
> Author: partigas
> Date: Fri Nov 30 16:47:15 2012
> New Revision: 169049
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=169049&view=rev
> Log:
> 
> reversed the logic of the log2 detection routine to reduce the number of nested ifs
> 
> 
> Modified:
>     llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=169049&r1=169048&r2=169049&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Fri Nov 30 16:47:15 2012
> @@ -261,31 +261,35 @@
>  //
> 
>  static void detectLog2OfHalf(Value *&Op, Value *&Y, IntrinsicInst *&Log2) {
> -   if (Op->hasOneUse()) {
> -    if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Op)) {
> -      if (II->getIntrinsicID() == Intrinsic::log2 &&
> -          II->hasUnsafeAlgebra()) {
> -        Log2 = II;
> -        Value *OpLog2Of = II->getArgOperand(0);
> -        if (OpLog2Of->hasOneUse()) {
> -          if (Instruction *I = dyn_cast<Instruction>(OpLog2Of)) {
> -            if (I->getOpcode() == Instruction::FMul &&
> -                I->hasUnsafeAlgebra()) {
> -              ConstantFP *CFP = dyn_cast<ConstantFP>(I->getOperand(0));
> -              if (CFP && CFP->isExactlyValue(0.5)) {
> -                Y = I->getOperand(1);
> -              } else {
> -                CFP = dyn_cast<ConstantFP>(I->getOperand(1));
> -                if (CFP && CFP->isExactlyValue(0.5)) {
> -                  Y = I->getOperand(0);
> -                }
> -              }
> -            }
> -          }
> -        }
> -      }
> -    }
> -  }
> +
> +   if (!Op->hasOneUse())
> +     return;
> +
> +   IntrinsicInst *II = dyn_cast<IntrinsicInst>(Op);
> +   if (!II)
> +     return;
> +   if (II->getIntrinsicID() != Intrinsic::log2 || !II->hasUnsafeAlgebra())
> +     return;
> +   Log2 = II;
> +
> +   Value *OpLog2Of = II->getArgOperand(0);
> +   if (!OpLog2Of->hasOneUse())
> +     return;
> +
> +   Instruction *I = dyn_cast<Instruction>(OpLog2Of);
> +   if (!I)
> +     return;
> +   if (I->getOpcode() != Instruction::FMul || !I->hasUnsafeAlgebra())
> +     return;
> +
> +   ConstantFP *CFP = dyn_cast<ConstantFP>(I->getOperand(0));
> +   if (CFP && CFP->isExactlyValue(0.5)) {
> +     Y = I->getOperand(1);
> +     return;
> +   }
> +   CFP = dyn_cast<ConstantFP>(I->getOperand(1));
> +   if (CFP && CFP->isExactlyValue(0.5))
> +     Y = I->getOperand(0);
>  }
> 
>  Instruction *InstCombiner::visitFMul(BinaryOperator &I) {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121130/3fa9577a/attachment.html>