[llvm-commits] [llvm] r147036 - in /llvm/trunk: lib/Analysis/ValueTracking.cpp lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/SpeculativeExec.ll

[Dan Gohman]

On Dec 20, 2011, at 9:52 PM, Nick Lewycky wrote:

> Author: nicholas
> Date: Tue Dec 20 23:52:02 2011
> New Revision: 147036
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=147036&view=rev
> Log:
> Make some intrinsics safe to speculatively execute.
> 
> Modified:
>    llvm/trunk/lib/Analysis/ValueTracking.cpp
>    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>    llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExec.ll
> 
> Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=147036&r1=147035&r2=147036&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Tue Dec 20 23:52:02 2011
> @@ -1912,11 +1912,31 @@
>       return false;
>     return LI->getPointerOperand()->isDereferenceablePointer();
>   }
> -  case Instruction::Call:
> +  case Instruction::Call: {
> +   if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
> +     switch (II->getIntrinsicID()) {
> +       case Intrinsic::bswap:
> +       case Intrinsic::ctlz:
> +       case Intrinsic::ctpop:
> +       case Intrinsic::cttz:
> +       case Intrinsic::objectsize:
> +       case Intrinsic::sadd_with_overflow:
> +       case Intrinsic::smul_with_overflow:
> +       case Intrinsic::ssub_with_overflow:
> +       case Intrinsic::uadd_with_overflow:
> +       case Intrinsic::umul_with_overflow:
> +       case Intrinsic::usub_with_overflow:
> +         return true;
> +       // TODO: some fp intrinsics are marked as having the same error handling
> +       // as libm. They're safe to speculate when they won't error.
> +       // TODO: are convert_{from,to}_fp16 safe?
> +       // TODO: can we list target-specific intrinsics here?

Just checking mayHaveSideEffects() should cover almost everything here.

> +       default: break;
> +     }
> +   }
>     return false; // The called function could have undefined behavior or
> -                  // side-effects.
> -                  // FIXME: We should special-case some intrinsics (bswap,
> -                  // overflow-checking arithmetic, etc.)
> +                  // side-effects, even if marked readnone nounwind.
> +  }
>   case Instruction::VAArg:
>   case Instruction::Alloca:
>   case Instruction::Invoke:
> 
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=147036&r1=147035&r2=147036&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Tue Dec 20 23:52:02 2011
> @@ -293,6 +293,7 @@
>     Cost = 1;
>     break;   // These are all cheap and non-trapping instructions.
> 
> +  case Instruction::Call:

cttz, ctlz, etc. are very expensive on many targets. Having SimplifyCFG
speculate them here could cause substantial pessimizations.

Dan