[SCCP][FuncSpec] Switch away from Instruction::isSafeToRemove()

[kpn]

Summary

In D115737 I found that I needed to teach Instruction::isSafeToRemove() about strictfp/constrained intrinsics. It was pointed out that this is probably the wrong function to use isInstructionTriviallyDead(). It doesn't make sense to have a "second, worse implementation".

I also believe that the Instruction class is the wrong place for this functionality. The information about whether or not an instruction can be removed is in the transform passes and should stay there.

I do not understand this pass well enough to add constrained support at this time, but this at least gets us away from the problematic API.

[kpn]

Ping.

[kpn]

Move more of Instruction::isSafeToRemove() into this function. The remainder of isSafeToRemove() isn't needed because we filter out function calls at the top of the changed function.

[nikic]

Can we use wouldInstructionBeTriviallyDead here as well? Relying on the CallBase bailout will probably come back bite us (at least, it's not really obvious to me why it is needed).

[kpn]

Update for review comment: just use wouldInstructionBeTriviallyDead().

[labrinea]

We did the same in IPSCCP a while ago D118387 and the verdict was to use the following:

static bool canRemoveInstruction(Instruction *I) {
  if (wouldInstructionBeTriviallyDead(I))
    return true;

  // Some instructions can be handled but are rejected above. Catch
  // those cases by falling through to here.
  // TODO: Mark globals as being constant earlier, so
  // TODO: wouldInstructionBeTriviallyDead() knows that atomic loads
  // TODO: are safe to remove.
  return isa<LoadInst>(I);
}

[kpn]

D118387 was my ticket, and it was handled that way to preserve the existing behavior. I don't believe a wrapper is needed in this case.

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

 #include "llvm/Analysis/InlineCost.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueLattice.h"
 #include "llvm/Analysis/ValueLatticeUtils.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/Transforms/Scalar/SCCP.h"
 #include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/SCCPSolver.h"
 #include "llvm/Transforms/Utils/SizeOpts.h"
 #include <cmath>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "function-specialization"
 
bool tryToReplaceWithConstant(Value *V) {
 
     V->replaceAllUsesWith(Const);
 
     for (auto *I : UseInsts)
       Solver.visit(I);
 
     // Remove the instruction from Block and Solver.
     if (auto *I = dyn_cast<Instruction>(V)) {
-      if (I->isSafeToRemove()) {
+      if (wouldInstructionBeTriviallyDead(I)) {
         ReplacedWithConstant.push_back(I);
         Solver.removeLatticeValueFor(I);
       }
     }
     return true;
   }
 
 private: