[InstSimplify] Don't constant fold or DCE calls that are marked nobuiltin

[andrew.w.kaylor]

Summary

There were a number of places that calls to math library functions were being constant folded or DCE'd without regard to the nobuiltin attribute. This patch attempts to fix that.

[efriedma]

Would it be possible to pass in a CallSite to canConstantFoldCallTo, ConstantFoldCall, and SimplifyCall, to reduce the number of places we check this?

lib/Analysis/ConstantFolding.cpp

2076 ↗

(On Diff #100910)

Would it make sense to add an overload of getLibFunc() which takes a CallSite?

[andrew.w.kaylor]

In D33737#769217, @efriedma wrote:

Would it be possible to pass in a CallSite to canConstantFoldCallTo, ConstantFoldCall, and SimplifyCall, to reduce the number of places we check this?

It's certainly possible. The reason I didn't do it was that I didn't want to make assumptions about how the existing functions might be used. They all seem to have deliberately avoided passing the actual call instruction as an argument. In the case of canConstantFoldCallTo it seems like the function is answering the abstract question of can calls to this Function be folded. Including the CallSite changes the semantics of the function, though it is arguably an improvement. In the case of ConstantFoldCall and SimplifyCall the expectation seems to be that they will construct the result of folding the given call, if possible, and the caller will be responsible for making actual changes to the active IR.

That said, I would favor including the CallSite in these calls if there are no objections because the current implementation is definitely vulnerable to future errors.

[efriedma]

The other alternative is that you could pass in an AttributeList (the result of getAttributes()) to the functions... that makes it more clear what exactly is getting used, but it's more complicated (and probably involves a bunch of refactoring to expose a function to check isNoBuiltin given a Function/AttributeList pair).

But either way, I would prefer a solution that involves checking for isNoBuiltin() in as few places as possible.

lib/Transforms/Utils/Evaluator.cpp

363 ↗

(On Diff #100910)

This isn't doing what you want: we don't call ConstantFoldCall on all paths.

[andrew.w.kaylor]

In D33737#769328, @efriedma wrote:

The other alternative is that you could pass in an AttributeList (the result of getAttributes()) to the functions... that makes it more clear what exactly is getting used, but it's more complicated (and probably involves a bunch of refactoring to expose a function to check isNoBuiltin given a Function/AttributeList pair).

But either way, I would prefer a solution that involves checking for isNoBuiltin() in as few places as possible.

I'd like to avoid the AttributeList because of the complications when the function declaration is nobuiltin but the CallSite is builtin. I'll implement your first suggestion and post an updated patch. I agree that it is better to have fewer places to check the attributes.

lib/Transforms/Utils/Evaluator.cpp

363 ↗

(On Diff #100910)

My reasoning is that if the call is marked as nobuiltin then we can't make any assumptions about what it does. To be honest though, I wasn't sure about this function.

Is this used by LLDB and similar clients for live code evaluation?

[efriedma]

lib/Transforms/Utils/Evaluator.cpp

363 ↗

(On Diff #100910)

If we have a definition where "isDefinitionExact()" is true, then we know the function does by examining the definition, so nobuiltin isn't relevant. (Actually, looking more closely, I think we're missing the "isDefinitionExact()" check.)

This isn't really a general-purpose IR interpreter; it's used by GlobalOpt to simplify C++ global initialization.

[andrew.w.kaylor]

Re-wrote the patch to move the isNoBuiltin checks into canConstantFoldCallTo and ConstantFoldCall.

[efriedma]

LGTM.

llvm/trunk/include/llvm/Analysis/ConstantFolding.h

 class CallSite;
 class Constant;
 class ConstantExpr;
 class ConstantVector;
 class DataLayout;
 class Function;
 class GlobalValue;
 class Instruction;
+class ImmutableCallSite;
 class TargetLibraryInfo;
 class Type;
 
 /// If this constant is a constant offset from a global, return the global and
 /// the constant. Because of constantexprs, this function is recursive.
 bool IsConstantOffsetFromGlobal(Constant *C, GlobalValue *&GV, APInt &Offset,
                                 const DataLayout &DL);
 

 /// indices (with an *implied* zero pointer index that is not in the list),
 /// return the constant value being addressed by a virtual load, or null if
 /// something is funny and we can't decide.
 Constant *ConstantFoldLoadThroughGEPIndices(Constant *C,
                                             ArrayRef<Constant *> Indices);
 
 /// canConstantFoldCallTo - Return true if its even possible to fold a call to
 /// the specified function.
-bool canConstantFoldCallTo(const Function *F);
+bool canConstantFoldCallTo(ImmutableCallSite CS, const Function *F);
 
 /// ConstantFoldCall - Attempt to constant fold a call to the specified function
 /// with the specified arguments, returning null if unsuccessful.
-Constant *ConstantFoldCall(Function *F, ArrayRef<Constant *> Operands,
+Constant *ConstantFoldCall(ImmutableCallSite CS, Function *F,
+                           ArrayRef<Constant *> Operands,
                            const TargetLibraryInfo *TLI = nullptr);
 
 /// \brief Check whether the given call has no side-effects.
 /// Specifically checks for math routimes which sometimes set errno.
 bool isMathLibCallNoop(CallSite CS, const TargetLibraryInfo *TLI);
 }
 
 #endif

llvm/trunk/include/llvm/Analysis/InstructionSimplify.h

 
 namespace llvm {
 class Function;
 template <typename T, typename... TArgs> class AnalysisManager;
 template <class T> class ArrayRef;
 class AssumptionCache;
 class DominatorTree;
 class Instruction;
+class ImmutableCallSite;
 class DataLayout;
 class FastMathFlags;
 struct LoopStandardAnalysisResults;
 class OptimizationRemarkEmitter;
 class Pass;
 class TargetLibraryInfo;
 class Type;
 class Value;

 /// Given operands for an FP BinaryOperator, fold the result or return null.
 /// In contrast to SimplifyBinOp, try to use FastMathFlag when folding the
 /// result. In case we don't need FastMathFlags, simply fall to SimplifyBinOp.
 Value *SimplifyFPBinOp(unsigned Opcode, Value *LHS, Value *RHS,
                        FastMathFlags FMF, const SimplifyQuery &Q);
 
 /// Given a function and iterators over arguments, fold the result or return
 /// null.
-Value *SimplifyCall(Value *V, User::op_iterator ArgBegin,
+Value *SimplifyCall(ImmutableCallSite CS, Value *V, User::op_iterator ArgBegin,
                     User::op_iterator ArgEnd, const SimplifyQuery &Q);
 
 /// Given a function and set of arguments, fold the result or return null.
-Value *SimplifyCall(Value *V, ArrayRef<Value *> Args, const SimplifyQuery &Q);
+Value *SimplifyCall(ImmutableCallSite CS, Value *V, ArrayRef<Value *> Args,
+                    const SimplifyQuery &Q);
 
 /// See if we can compute a simplified version of this instruction. If not,
 /// return null.
 Value *SimplifyInstruction(Instruction *I, const SimplifyQuery &Q,
                            OptimizationRemarkEmitter *ORE = nullptr);
 
 /// Replace all uses of 'I' with 'SimpleV' and simplify the uses recursively.
 ///

llvm/trunk/lib/Analysis/ConstantFolding.cpp

Constant *ConstantFoldInstOperandsImpl(const Value *InstOrCE, unsigned Opcode,
   if (auto *CE = dyn_cast<ConstantExpr>(InstOrCE))
     return CE->getWithOperands(Ops);
 
   switch (Opcode) {
   default: return nullptr;
   case Instruction::ICmp:
   case Instruction::FCmp: llvm_unreachable("Invalid for compares");
   case Instruction::Call:
-    if (auto *F = dyn_cast<Function>(Ops.back()))
-      if (canConstantFoldCallTo(F))
-        return ConstantFoldCall(F, Ops.slice(0, Ops.size() - 1), TLI);
+    if (auto *F = dyn_cast<Function>(Ops.back())) {
+      ImmutableCallSite CS(cast<CallInst>(InstOrCE));
+      if (canConstantFoldCallTo(CS, F))
+        return ConstantFoldCall(CS, F, Ops.slice(0, Ops.size() - 1), TLI);
+    }
     return nullptr;
   case Instruction::Select:
     return ConstantExpr::getSelect(Ops[0], Ops[1], Ops[2]);
   case Instruction::ExtractElement:
     return ConstantExpr::getExtractElement(Ops[0], Ops[1]);
   case Instruction::InsertElement:
     return ConstantExpr::getInsertElement(Ops[0], Ops[1], Ops[2]);
   case Instruction::ShuffleVector:
llvm::ConstantFoldLoadThroughGEPIndices(Constant *C,
   }
   return C;
 }
 
 //===----------------------------------------------------------------------===//
 //  Constant Folding for Calls
 //
 
-bool llvm::canConstantFoldCallTo(const Function *F) {
+bool llvm::canConstantFoldCallTo(ImmutableCallSite CS, const Function *F) {
+  if (CS.isNoBuiltin())
+    return false;
   switch (F->getIntrinsicID()) {
   case Intrinsic::fabs:
   case Intrinsic::minnum:
   case Intrinsic::maxnum:
   case Intrinsic::log:
   case Intrinsic::log2:
   case Intrinsic::log10:
   case Intrinsic::exp:
Constant *ConstantFoldVectorCall(StringRef Name, unsigned IntrinsicID,
   }
 
   return ConstantVector::get(Result);
 }
 
 } // end anonymous namespace
 
 Constant *
-llvm::ConstantFoldCall(Function *F, ArrayRef<Constant *> Operands,
+llvm::ConstantFoldCall(ImmutableCallSite CS, Function *F,
+                       ArrayRef<Constant *> Operands,
                        const TargetLibraryInfo *TLI) {
+  if (CS.isNoBuiltin())
+    return nullptr;
   if (!F->hasName())
     return nullptr;
   StringRef Name = F->getName();
 
   Type *Ty = F->getReturnType();
 
   if (auto *VTy = dyn_cast<VectorType>(Ty))
     return ConstantFoldVectorCall(Name, F->getIntrinsicID(), VTy, Operands,
                                   F->getParent()->getDataLayout(), TLI);
 
   return ConstantFoldScalarCall(Name, F->getIntrinsicID(), Ty, Operands, TLI);
 }
 
 bool llvm::isMathLibCallNoop(CallSite CS, const TargetLibraryInfo *TLI) {
   // FIXME: Refactor this code; this duplicates logic in LibCallsShrinkWrap
   // (and to some extent ConstantFoldScalarCall).
+  if (CS.isNoBuiltin())
+    return false;
   Function *F = CS.getCalledFunction();
   if (!F)
     return false;
 
   LibFunc Func;
   if (!TLI || !TLI->getLibFunc(*F, Func))
     return false;
 

llvm/trunk/lib/Analysis/InlineCost.cpp

 /// analyzing the arguments and call itself with instsimplify. Returns true if
 /// it has simplified the callsite to some other entity (a constant), making it
 /// free.
 bool CallAnalyzer::simplifyCallSite(Function *F, CallSite CS) {
   // FIXME: Using the instsimplify logic directly for this is inefficient
   // because we have to continually rebuild the argument list even when no
   // simplifications can be performed. Until that is fixed with remapping
   // inside of instsimplify, directly constant fold calls here.
-  if (!canConstantFoldCallTo(F))
+  if (!canConstantFoldCallTo(CS, F))
     return false;
 
   // Try to re-map the arguments to constants.
   SmallVector<Constant *, 4> ConstantArgs;
   ConstantArgs.reserve(CS.arg_size());
   for (CallSite::arg_iterator I = CS.arg_begin(), E = CS.arg_end(); I != E;
        ++I) {
     Constant *C = dyn_cast<Constant>(*I);
     if (!C)
       C = dyn_cast_or_null<Constant>(SimplifiedValues.lookup(*I));
     if (!C)
       return false; // This argument doesn't map to a constant.
 
     ConstantArgs.push_back(C);
   }
-  if (Constant *C = ConstantFoldCall(F, ConstantArgs)) {
+  if (Constant *C = ConstantFoldCall(CS, F, ConstantArgs)) {
     SimplifiedValues[CS.getInstruction()] = C;
     return true;
   }
 
   return false;
 }
 
 bool CallAnalyzer::visitCallSite(CallSite CS) {

llvm/trunk/lib/Analysis/InstructionSimplify.cpp

case Intrinsic::masked_load: {
     return nullptr;
   }
   default:
     return nullptr;
   }
 }
 
 template <typename IterTy>
-static Value *SimplifyCall(Value *V, IterTy ArgBegin, IterTy ArgEnd,
-                           const SimplifyQuery &Q, unsigned MaxRecurse) {
+static Value *SimplifyCall(ImmutableCallSite CS, Value *V, IterTy ArgBegin,
+                           IterTy ArgEnd, const SimplifyQuery &Q,
+                           unsigned MaxRecurse) {
   Type *Ty = V->getType();
   if (PointerType *PTy = dyn_cast<PointerType>(Ty))
     Ty = PTy->getElementType();
   FunctionType *FTy = cast<FunctionType>(Ty);
 
   // call undef -> undef
   // call null -> undef
   if (isa<UndefValue>(V) || isa<ConstantPointerNull>(V))
     return UndefValue::get(FTy->getReturnType());
 
   Function *F = dyn_cast<Function>(V);
   if (!F)
     return nullptr;
 
   if (F->isIntrinsic())
     if (Value *Ret = SimplifyIntrinsic(F, ArgBegin, ArgEnd, Q, MaxRecurse))
       return Ret;
 
-  if (!canConstantFoldCallTo(F))
+  if (!canConstantFoldCallTo(CS, F))
     return nullptr;
 
   SmallVector<Constant *, 4> ConstantArgs;
   ConstantArgs.reserve(ArgEnd - ArgBegin);
   for (IterTy I = ArgBegin, E = ArgEnd; I != E; ++I) {
     Constant *C = dyn_cast<Constant>(*I);
     if (!C)
       return nullptr;
     ConstantArgs.push_back(C);
   }
 
-  return ConstantFoldCall(F, ConstantArgs, Q.TLI);
+  return ConstantFoldCall(CS, F, ConstantArgs, Q.TLI);
 }
 
-Value *llvm::SimplifyCall(Value *V, User::op_iterator ArgBegin,
-                          User::op_iterator ArgEnd, const SimplifyQuery &Q) {
-  return ::SimplifyCall(V, ArgBegin, ArgEnd, Q, RecursionLimit);
+Value *llvm::SimplifyCall(ImmutableCallSite CS, Value *V,
+                          User::op_iterator ArgBegin, User::op_iterator ArgEnd,
+                          const SimplifyQuery &Q) {
+  return ::SimplifyCall(CS, V, ArgBegin, ArgEnd, Q, RecursionLimit);
 }
 
-Value *llvm::SimplifyCall(Value *V, ArrayRef<Value *> Args,
-                          const SimplifyQuery &Q) {
-  return ::SimplifyCall(V, Args.begin(), Args.end(), Q, RecursionLimit);
+Value *llvm::SimplifyCall(ImmutableCallSite CS, Value *V,
+                          ArrayRef<Value *> Args, const SimplifyQuery &Q) {
+  return ::SimplifyCall(CS, V, Args.begin(), Args.end(), Q, RecursionLimit);
 }
 
 /// See if we can compute a simplified version of this instruction.
 /// If not, this returns null.
 
 Value *llvm::SimplifyInstruction(Instruction *I, const SimplifyQuery &SQ,
                                  OptimizationRemarkEmitter *ORE) {
   const SimplifyQuery Q = SQ.CxtI ? SQ : SQ.getWithInstruction(I);
Result = SimplifyShuffleVectorInst(SVI->getOperand(0), SVI->getOperand(1),
                                        SVI->getMask(), SVI->getType(), Q);
     break;
   }
   case Instruction::PHI:
     Result = SimplifyPHINode(cast<PHINode>(I), Q);
     break;
   case Instruction::Call: {
     CallSite CS(cast<CallInst>(I));
-    Result = SimplifyCall(CS.getCalledValue(), CS.arg_begin(), CS.arg_end(), Q);
+    Result = SimplifyCall(CS, CS.getCalledValue(), CS.arg_begin(), CS.arg_end(),
+                          Q);
     break;
   }
 #define HANDLE_CAST_INST(num, opc, clas) case Instruction::opc:
 #include "llvm/IR/Instruction.def"
 #undef HANDLE_CAST_INST
     Result =
         SimplifyCastInst(I->getOpcode(), I->getOperand(0), I->getType(), Q);
     break;

llvm/trunk/lib/Analysis/ScalarEvolution.cpp

 static bool CanConstantFold(const Instruction *I) {
   if (isa<BinaryOperator>(I) || isa<CmpInst>(I) ||
       isa<SelectInst>(I) || isa<CastInst>(I) || isa<GetElementPtrInst>(I) ||
       isa<LoadInst>(I))
     return true;
 
   if (const CallInst *CI = dyn_cast<CallInst>(I))
     if (const Function *F = CI->getCalledFunction())
-      return canConstantFoldCallTo(F);
+      return canConstantFoldCallTo(CI, F);
   return false;
 }
 
 /// Determine whether this instruction can constant evolve within this loop
 /// assuming its operands can all constant evolve.
 static bool canConstantEvolve(Instruction *I, const Loop *L) {
   // An instruction outside of the loop can't be derived from a loop PHI.
   if (!L->contains(I)) return false;

llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp

Instruction *InstCombiner::visitVACopyInst(VACopyInst &I) {
   return nullptr;
 }
 
 /// CallInst simplification. This mostly only handles folding of intrinsic
 /// instructions. For normal calls, it allows visitCallSite to do the heavy
 /// lifting.
 Instruction *InstCombiner::visitCallInst(CallInst &CI) {
   auto Args = CI.arg_operands();
-  if (Value *V = SimplifyCall(CI.getCalledValue(), Args.begin(), Args.end(),
-                              SQ.getWithInstruction(&CI)))
+  if (Value *V = SimplifyCall(&CI, CI.getCalledValue(), Args.begin(),
+                              Args.end(), SQ.getWithInstruction(&CI)))
     return replaceInstUsesWith(CI, V);
 
   if (isFreeCall(&CI, &TLI))
     return visitFree(CI);
 
   // If the caller function is nounwind, mark the call as nounwind, even if the
   // callee isn't.
   if (CI.getFunction()->doesNotThrow() && !CI.doesNotThrow()) {

llvm/trunk/lib/Transforms/Scalar/SCCP.cpp

void SCCPSolver::visitCallSite(CallSite CS) {
   if (!F || F->isDeclaration()) {
 CallOverdefined:
     // Void return and not tracking callee, just bail.
     if (I->getType()->isVoidTy()) return;
 
     // Otherwise, if we have a single return value case, and if the function is
     // a declaration, maybe we can constant fold it.
     if (F && F->isDeclaration() && !I->getType()->isStructTy() &&
-        canConstantFoldCallTo(F)) {
+        canConstantFoldCallTo(CS, F)) {
 
       SmallVector<Constant*, 8> Operands;
       for (CallSite::arg_iterator AI = CS.arg_begin(), E = CS.arg_end();
            AI != E; ++AI) {
         LatticeVal State = getValueState(*AI);
 
         if (State.isUnknown())
           return;  // Operands are not resolved yet.
         if (State.isOverdefined())
           return markOverdefined(I);
         assert(State.isConstant() && "Unknown state!");
         Operands.push_back(State.getConstant());
       }
 
       if (getValueState(I).isOverdefined())
         return;
 
       // If we can constant fold this, mark the result of the call as a
       // constant.
-      if (Constant *C = ConstantFoldCall(F, Operands, TLI)) {
+      if (Constant *C = ConstantFoldCall(CS, F, Operands, TLI)) {
         // call -> undef.
         if (isa<UndefValue>(C))
           return;
         return markConstant(I, C);
       }
     }
 
     // Otherwise, we don't know anything about this call, mark it overdefined.

llvm/trunk/lib/Transforms/Utils/Evaluator.cpp

if (StoreInst *SI = dyn_cast<StoreInst>(CurInst)) {
       }
 
       SmallVector<Constant*, 8> Formals;
       for (User::op_iterator i = CS.arg_begin(), e = CS.arg_end(); i != e; ++i)
         Formals.push_back(getVal(*i));
 
       if (Callee->isDeclaration()) {
         // If this is a function we can constant fold, do it.
-        if (Constant *C = ConstantFoldCall(Callee, Formals, TLI)) {
+        if (Constant *C = ConstantFoldCall(CS, Callee, Formals, TLI)) {
           InstResult = C;
           DEBUG(dbgs() << "Constant folded function call. Result: " <<
                 *InstResult << "\n");
         } else {
           DEBUG(dbgs() << "Can not constant fold function call.\n");
           return false;
         }
       } else {

llvm/trunk/test/Transforms/DCE/calls-errno.ll

 
 ; cos(0) is 1
   %cos1 = call double @cos(double 0.000000e+00)
 
 ; cos(inf) is a domain error
 ; CHECK-NEXT: %cos2 = call double @cos(double 0x7FF0000000000000)
   %cos2 = call double @cos(double 0x7FF0000000000000)
 
+; cos(0) nobuiltin may have side effects 
+; CHECK-NEXT: %cos3 = call double @cos(double 0.000000e+00)
+  %cos3 = call double @cos(double 0.000000e+00) nobuiltin
+
 ; pow(0, 1) is 0
   %pow1 = call double @pow(double 0x7FF0000000000000, double 1.000000e+00)
 
 ; pow(0, -1) is a pole error
 ; FIXME: It fails on mingw host. Suppress checking.
 ; %pow2 = call double @pow(double 0.000000e+00, double -1.000000e+00)
 
 ; fmod(inf, nan) is nan

llvm/trunk/test/Transforms/InstCombine/constant-fold-libfunc.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+declare double @acos(double)
+
+; Check that functions without any function attributes are simplified.
+
+define double @test_simplify_acos() {
+; CHECK-LABEL: @test_simplify_acos
+  %pi = call double @acos(double -1.000000e+00)
+; CHECK-NOT: call double @acos
+; CHECK: ret double 0x400921FB54442D18
+  ret double %pi
+}
+
+define double @test_acos_nobuiltin() {
+; CHECK-LABEL: @test_acos_nobuiltin
+  %pi = call double @acos(double -1.000000e+00) nobuiltin 
+; CHECK: call double @acos(double -1.000000e+00)
+  ret double %pi
+}