[PM] Port float2int to the new pass manager

[davidxl]

lgtm

lib/Transforms/Scalar/Float2Int.cpp

537 ↗

(On Diff #61847)

This is a good point. Is there a bug tracking this missing functionality?

541 ↗

(On Diff #61847)

same here.

[davide]

lib/Transforms/Scalar/Float2Int.cpp

73 ↗

(On Diff #61847)

This is not your bug, but still worth mentioning. This pass preserves globalsAA, doesn't depend on it. In other words, this can go away.

537 ↗

(On Diff #61847)

I'd remove the FIXME. No other pass mentions it, and I'm not sure if there's active work on this.

541 ↗

(On Diff #61847)

I don't think there's a bug tracker for this (probably there should be one tho?).
As a general comment, this needs an understanding which passes actually now 'preserve the CFG' (whatever that means, we need to agree on a sane/clear semantic) so I'm not entirely sure the changes here will be NFC.

[mkuper]

Thanks, David and Davide.

lib/Transforms/Scalar/Float2Int.cpp

73 ↗

(On Diff #61847)

Right, I noticed that too, but forgot about it. I'll remove it.

537 ↗

(On Diff #61847)

You're probably right.
I remember some talk about this possibly being handled by the PM, so that run() methods won't even be called when the function should be skipped. I'll remove the FIXME, thanks.

541 ↗

(On Diff #61847)

I don't know about a bug tracker, but there are FIXMEs for this in many ported passes. I think it's good to have these fixmes regardless of what exactly happens in the future, simply to serve as (another) marker for passes where it makes *some* sense.

[davide]

This one looks good modulo two small comments.

lib/Transforms/Scalar/Float2Int.cpp

526 ↗

(On Diff #61847)

This function can be inlined (line 60), no?
bool runOnFunction(Function &F) override;

541 ↗

(On Diff #61847)

Yes a comment here makes sense, thanks! Can you please copy'n'paste from any of the passes I already ported (so that we have the same comment everywhere and it's easier to grep and/or locate when we want to replace)

[mkuper]

lib/Transforms/Scalar/Float2Int.cpp

526 ↗

(On Diff #61847)

Sure, I don't feel strongly about it either way. Will do.

541 ↗

(On Diff #61847)

Unfortunately, it's already very non-uniform. We have at least:

$ find . -name *.cpp | xargs cat | grep // | grep -i preserve | grep CFG

// FIXME: Need a way to preserve CFG analyses here!
// FIXME: There is no setPreservesCFG in the new PM. When that becomes
// FIXME: This pass should preserve the CFG.
// FIXME: BDCE should also 'preserve the CFG'.
  //FIXME: setPreservesCFG is not currently supported in the new PM.
// FIXME: ADCE should also 'preserve the CFG'.
// FIXME: once we have an equivalent of AU.setPreservesCFG() in the
// FIXME: This pass should also 'preserve the CFG'.
  // FIXME: Reassociate should also 'preserve the CFG'.

I can try to find all of those - I'm sure there are some this grep missed - and replace with the same string (as a separate patch, of course), but there'd be no way to enforce it for future ports.
Perhaps it would be better to have a dummy implementation that we can call?

[davide]

LGTM

lib/Transforms/Scalar/Float2Int.cpp

541 ↗

(On Diff #61847)

I'm fine with the dummy implementation but I'd like to hear what David/Chandler/Sean have to say about it. Maybe you can just leave the comment as his and defer that to a different patch (maybe start a thread on llvm-dev)? How does that sound?

[mkuper]

lib/Transforms/Scalar/Float2Int.cpp

541 ↗

(On Diff #61847)

SGTM.

llvm/trunk/include/llvm/InitializePasses.h

 void initializeEdgeBundlesPass(PassRegistry&);
 void initializeEfficiencySanitizerPass(PassRegistry&);
 void initializeEliminateAvailableExternallyLegacyPassPass(PassRegistry &);
 void initializeExpandISelPseudosPass(PassRegistry&);
 void initializeExpandPostRAPass(PassRegistry&);
 void initializeExternalAAWrapperPassPass(PassRegistry&);
 void initializeFinalizeMachineBundlesPass(PassRegistry&);
 void initializeFlattenCFGPassPass(PassRegistry&);
-void initializeFloat2IntPass(PassRegistry&);
+void initializeFloat2IntLegacyPassPass(PassRegistry&);
 void initializeForceFunctionAttrsLegacyPassPass(PassRegistry&);
 void initializeForwardControlFlowIntegrityPass(PassRegistry&);
 void initializeFuncletLayoutPass(PassRegistry &);
 void initializeFunctionImportPassPass(PassRegistry &);
 void initializeGCMachineCodeAnalysisPass(PassRegistry&);
 void initializeGCModuleInfoPass(PassRegistry&);
 void initializeGCOVProfilerLegacyPassPass(PassRegistry&);
 void initializeGVNLegacyPassPass(PassRegistry&);

llvm/trunk/include/llvm/Transforms/Scalar/Float2Int.h

+//===-- Float2Int.h - Demote floating point ops to work on integers -------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides the Float2Int pass, which aims to demote floating
+// point operations to work on integers, where that is losslessly possible.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_SCALAR_FLOAT2INT_H
+#define LLVM_TRANSFORMS_SCALAR_FLOAT2INT_H
+
+#include "llvm/ADT/EquivalenceClasses.h"
+#include "llvm/ADT/MapVector.h"
+#include "llvm/IR/ConstantRange.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+/// Pass to remove unused function declarations.
+class Float2IntPass : public PassInfoMixin<Float2IntPass> {
+public:
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+
+  // Glue for old PM.
+  bool runImpl(Function &F);
+
+private:
+  void findRoots(Function &F, SmallPtrSet<Instruction *, 8> &Roots);
+  ConstantRange seen(Instruction *I, ConstantRange R);
+  ConstantRange badRange();
+  ConstantRange unknownRange();
+  ConstantRange validateRange(ConstantRange R);
+  void walkBackwards(const SmallPtrSetImpl<Instruction *> &Roots);
+  void walkForwards();
+  bool validateAndTransform();
+  Value *convert(Instruction *I, Type *ToTy);
+  void cleanup();
+
+  MapVector<Instruction *, ConstantRange> SeenInsts;
+  SmallPtrSet<Instruction *, 8> Roots;
+  EquivalenceClasses<Instruction *> ECs;
+  MapVector<Instruction *, Value *> ConvertedInsts;
+  LLVMContext *Ctx;
+};
+}
+#endif // LLVM_TRANSFORMS_SCALAR_FLOAT2INT_H

llvm/trunk/lib/Passes/PassBuilder.cpp

 #include "llvm/Transforms/PGOInstrumentation.h"
 #include "llvm/Transforms/SampleProfile.h"
 #include "llvm/Transforms/Scalar/ADCE.h"
 #include "llvm/Transforms/Scalar/AlignmentFromAssumptions.h"
 #include "llvm/Transforms/Scalar/BDCE.h"
 #include "llvm/Transforms/Scalar/DCE.h"
 #include "llvm/Transforms/Scalar/DeadStoreElimination.h"
 #include "llvm/Transforms/Scalar/EarlyCSE.h"
+#include "llvm/Transforms/Scalar/Float2Int.h"
 #include "llvm/Transforms/Scalar/GVN.h"
 #include "llvm/Transforms/Scalar/GuardWidening.h"
 #include "llvm/Transforms/Scalar/IndVarSimplify.h"
 #include "llvm/Transforms/Scalar/JumpThreading.h"
 #include "llvm/Transforms/Scalar/LoopRotation.h"
 #include "llvm/Transforms/Scalar/LoopSimplifyCFG.h"
 #include "llvm/Transforms/Scalar/LowerAtomic.h"
 #include "llvm/Transforms/Scalar/LowerExpectIntrinsic.h"

llvm/trunk/lib/Passes/PassRegistry.def

 FUNCTION_PASS("add-discriminators", AddDiscriminatorsPass())
 FUNCTION_PASS("alignment-from-assumptions", AlignmentFromAssumptionsPass())
 FUNCTION_PASS("bdce", BDCEPass())
 FUNCTION_PASS("dce", DCEPass())
 FUNCTION_PASS("dse", DSEPass())
 FUNCTION_PASS("early-cse", EarlyCSEPass())
 FUNCTION_PASS("instcombine", InstCombinePass())
 FUNCTION_PASS("invalidate<all>", InvalidateAllAnalysesPass())
+FUNCTION_PASS("float2int", Float2IntPass())
 FUNCTION_PASS("no-op-function", NoOpFunctionPass())
 FUNCTION_PASS("loweratomic", LowerAtomicPass())
 FUNCTION_PASS("lower-expect", LowerExpectIntrinsicPass())
 FUNCTION_PASS("guard-widening", GuardWideningPass())
 FUNCTION_PASS("gvn", GVN())
 FUNCTION_PASS("mem2reg", PromotePass())
 FUNCTION_PASS("memcpyopt", MemCpyOptPass())
 FUNCTION_PASS("mldst-motion", MergedLoadStoreMotionPass())

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

 //===- Float2Int.cpp - Demote floating point ops to work on integers ------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the Float2Int pass, which aims to demote floating
 // point operations to work on integers, where that is losslessly possible.
 //
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "float2int"
+
+#include "llvm/Transforms/Scalar/Float2Int.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
-#include "llvm/ADT/EquivalenceClasses.h"
-#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/GlobalsModRef.h"
-#include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"

 
 /// The largest integer type worth dealing with.
 static cl::opt<unsigned>
 MaxIntegerBW("float2int-max-integer-bw", cl::init(64), cl::Hidden,
              cl::desc("Max integer bitwidth to consider in float2int"
                       "(default=64)"));
 
 namespace {
-  struct Float2Int : public FunctionPass {
+  struct Float2IntLegacyPass : public FunctionPass {
     static char ID; // Pass identification, replacement for typeid
-    Float2Int() : FunctionPass(ID) {
-      initializeFloat2IntPass(*PassRegistry::getPassRegistry());
+    Float2IntLegacyPass() : FunctionPass(ID) {
+      initializeFloat2IntLegacyPassPass(*PassRegistry::getPassRegistry());
+    }
+
+    bool runOnFunction(Function &F) override {
+      if (skipFunction(F))
+        return false;
+
+      return Impl.runImpl(F);
     }
 
-    bool runOnFunction(Function &F) override;
     void getAnalysisUsage(AnalysisUsage &AU) const override {
       AU.setPreservesCFG();
       AU.addPreserved<GlobalsAAWrapperPass>();
     }
 
-    void findRoots(Function &F, SmallPtrSet<Instruction*,8> &Roots);
-    ConstantRange seen(Instruction *I, ConstantRange R);
-    ConstantRange badRange();
-    ConstantRange unknownRange();
-    ConstantRange validateRange(ConstantRange R);
-    void walkBackwards(const SmallPtrSetImpl<Instruction*> &Roots);
-    void walkForwards();
-    bool validateAndTransform();
-    Value *convert(Instruction *I, Type *ToTy);
-    void cleanup();
-
-    MapVector<Instruction*, ConstantRange > SeenInsts;
-    SmallPtrSet<Instruction*,8> Roots;
-    EquivalenceClasses<Instruction*> ECs;
-    MapVector<Instruction*, Value*> ConvertedInsts;
-    LLVMContext *Ctx;
+  private:
+    Float2IntPass Impl;
   };
 }
 
-char Float2Int::ID = 0;
-INITIALIZE_PASS_BEGIN(Float2Int, "float2int", "Float to int", false, false)
-INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
-INITIALIZE_PASS_END(Float2Int, "float2int", "Float to int", false, false)
+char Float2IntLegacyPass::ID = 0;
+INITIALIZE_PASS(Float2IntLegacyPass, "float2int", "Float to int", false, false)
 
 // Given a FCmp predicate, return a matching ICmp predicate if one
 // exists, otherwise return BAD_ICMP_PREDICATE.
 static CmpInst::Predicate mapFCmpPred(CmpInst::Predicate P) {
   switch (P) {
   case CmpInst::FCMP_OEQ:
   case CmpInst::FCMP_UEQ:
     return CmpInst::ICMP_EQ;
static Instruction::BinaryOps mapBinOpcode(unsigned Opcode) {
   case Instruction::FAdd: return Instruction::Add;
   case Instruction::FSub: return Instruction::Sub;
   case Instruction::FMul: return Instruction::Mul;
   }
 }
 
 // Find the roots - instructions that convert from the FP domain to
 // integer domain.
-void Float2Int::findRoots(Function &F, SmallPtrSet<Instruction*,8> &Roots) {
+void Float2IntPass::findRoots(Function &F, SmallPtrSet<Instruction*,8> &Roots) {
   for (auto &I : instructions(F)) {
     if (isa<VectorType>(I.getType()))
       continue;
     switch (I.getOpcode()) {
     default: break;
     case Instruction::FPToUI:
     case Instruction::FPToSI:
       Roots.insert(&I);
       break;
     case Instruction::FCmp:
       if (mapFCmpPred(cast<CmpInst>(&I)->getPredicate()) !=
           CmpInst::BAD_ICMP_PREDICATE)
         Roots.insert(&I);
       break;
     }
   }
 }
 
 // Helper - mark I as having been traversed, having range R.
-ConstantRange Float2Int::seen(Instruction *I, ConstantRange R) {
+ConstantRange Float2IntPass::seen(Instruction *I, ConstantRange R) {
   DEBUG(dbgs() << "F2I: " << *I << ":" << R << "\n");
   if (SeenInsts.find(I) != SeenInsts.end())
     SeenInsts.find(I)->second = R;
   else
     SeenInsts.insert(std::make_pair(I, R));
   return R;
 }
 
 // Helper - get a range representing a poison value.
-ConstantRange Float2Int::badRange() {
+ConstantRange Float2IntPass::badRange() {
   return ConstantRange(MaxIntegerBW + 1, true);
 }
-ConstantRange Float2Int::unknownRange() {
+ConstantRange Float2IntPass::unknownRange() {
   return ConstantRange(MaxIntegerBW + 1, false);
 }
-ConstantRange Float2Int::validateRange(ConstantRange R) {
+ConstantRange Float2IntPass::validateRange(ConstantRange R) {
   if (R.getBitWidth() > MaxIntegerBW + 1)
     return badRange();
   return R;
 }
 
 // The most obvious way to structure the search is a depth-first, eager
 // search from each root. However, that require direct recursion and so
 // can only handle small instruction sequences. Instead, we split the search
 // up into two phases:
 //   - walkBackwards:  A breadth-first walk of the use-def graph starting from
 //                     the roots. Populate "SeenInsts" with interesting
 //                     instructions and poison values if they're obvious and
 //                     cheap to compute. Calculate the equivalance set structure
 //                     while we're here too.
 //   - walkForwards:  Iterate over SeenInsts in reverse order, so we visit
 //                     defs before their uses. Calculate the real range info.
 
 // Breadth-first walk of the use-def graph; determine the set of nodes
 // we care about and eagerly determine if some of them are poisonous.
-void Float2Int::walkBackwards(const SmallPtrSetImpl<Instruction*> &Roots) {
+void Float2IntPass::walkBackwards(const SmallPtrSetImpl<Instruction*> &Roots) {
   std::deque<Instruction*> Worklist(Roots.begin(), Roots.end());
   while (!Worklist.empty()) {
     Instruction *I = Worklist.back();
     Worklist.pop_back();
 
     if (SeenInsts.find(I) != SeenInsts.end())
       // Seen already.
       continue;
for (Value *O : I->operands()) {
         seen(I, badRange());
       }
     }
   }
 }
 
 // Walk forwards down the list of seen instructions, so we visit defs before
 // uses.
-void Float2Int::walkForwards() {
+void Float2IntPass::walkForwards() {
   for (auto &It : reverse(SeenInsts)) {
     if (It.second != unknownRange())
       continue;
 
     Instruction *I = It.first;
     std::function<ConstantRange(ArrayRef<ConstantRange>)> Op;
     switch (I->getOpcode()) {
       // FIXME: Handle select and phi nodes.
for (auto &It : reverse(SeenInsts)) {
 
     // Reduce the operands' ranges to a single range and return.
     if (!Abort)
       seen(I, Op(OpRanges));
   }
 }
 
 // If there is a valid transform to be done, do it.
-bool Float2Int::validateAndTransform() {
+bool Float2IntPass::validateAndTransform() {
   bool MadeChange = false;
 
   // Iterate over every disjoint partition of the def-use graph.
   for (auto It = ECs.begin(), E = ECs.end(); It != E; ++It) {
     ConstantRange R(MaxIntegerBW + 1, false);
     bool Fail = false;
     Type *ConvertedToTy = nullptr;
 
for (auto MI = ECs.member_begin(It), ME = ECs.member_end();
          MI != ME; ++MI)
       convert(*MI, Ty);
     MadeChange = true;
   }
 
   return MadeChange;
 }
 
-Value *Float2Int::convert(Instruction *I, Type *ToTy) {
+Value *Float2IntPass::convert(Instruction *I, Type *ToTy) {
   if (ConvertedInsts.find(I) != ConvertedInsts.end())
     // Already converted this instruction.
     return ConvertedInsts[I];
 
   SmallVector<Value*,4> NewOperands;
   for (Value *V : I->operands()) {
     // Don't recurse if we're an instruction that terminates the path.
     if (I->getOpcode() == Instruction::UIToFP ||
Value *Float2IntPass::convert(Instruction *I, Type *ToTy) {
   if (Roots.count(I))
     I->replaceAllUsesWith(NewV);
 
   ConvertedInsts[I] = NewV;
   return NewV;
 }
 
 // Perform dead code elimination on the instructions we just modified.
-void Float2Int::cleanup() {
+void Float2IntPass::cleanup() {
   for (auto &I : reverse(ConvertedInsts))
     I.first->eraseFromParent();
 }
 
-bool Float2Int::runOnFunction(Function &F) {
-  if (skipFunction(F))
-    return false;
-
+bool Float2IntPass::runImpl(Function &F) {
   DEBUG(dbgs() << "F2I: Looking at function " << F.getName() << "\n");
   // Clear out all state.
   ECs = EquivalenceClasses<Instruction*>();
   SeenInsts.clear();
   ConvertedInsts.clear();
   Roots.clear();
 
   Ctx = &F.getParent()->getContext();
 
   findRoots(F, Roots);
 
   walkBackwards(Roots);
   walkForwards();
 
   bool Modified = validateAndTransform();
   if (Modified)
     cleanup();
   return Modified;
 }
 
-FunctionPass *llvm::createFloat2IntPass() { return new Float2Int(); }
+namespace llvm {
+FunctionPass *createFloat2IntPass() { return new Float2IntLegacyPass(); }
+
+PreservedAnalyses Float2IntPass::run(Function &F, FunctionAnalysisManager &) {
+  if (!runImpl(F))
+    return PreservedAnalyses::all();
+  else {
+    //FIXME: setPreservesCFG is not currently supported in the new PM.
+    PreservedAnalyses PA;
+    PA.preserve<GlobalsAA>();
+    return PA;
+  }
+}
+} // End namespace llvm

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

void llvm::initializeScalarOpts(PassRegistry &Registry) {
   initializeSinkingLegacyPassPass(Registry);
   initializeTailCallElimPass(Registry);
   initializeSeparateConstOffsetFromGEPPass(Registry);
   initializeSpeculativeExecutionPass(Registry);
   initializeStraightLineStrengthReducePass(Registry);
   initializeLoadCombinePass(Registry);
   initializePlaceBackedgeSafepointsImplPass(Registry);
   initializePlaceSafepointsPass(Registry);
-  initializeFloat2IntPass(Registry);
+  initializeFloat2IntLegacyPassPass(Registry);
   initializeLoopDistributePass(Registry);
   initializeLoopLoadEliminationPass(Registry);
   initializeLoopSimplifyCFGLegacyPassPass(Registry);
   initializeLoopVersioningPassPass(Registry);
 }
 
 void LLVMInitializeScalarOpts(LLVMPassRegistryRef R) {
   initializeScalarOpts(*unwrap(R));

llvm/trunk/test/Transforms/Float2Int/basic.ll

 ; RUN: opt < %s -float2int -S | FileCheck %s
+; RUN: opt < %s -passes='float2int' -S | FileCheck %s
 
 ;
 ; Positive tests
 ;
 
 ; CHECK-LABEL: @simple1
 ; CHECK:  %1 = zext i8 %a to i32
 ; CHECK:  %2 = add i32 %1, 1