[SeparateConstOffsetFromGEP] Added statistic and small refactoring

[mkazantsev]

LG

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

1169

I'm sure this is not needed, you will get function it runs on with -print-before-all

[eklepilkina]

• Removed

[eklepilkina]

Added statistics

[eklepilkina]

Update

[eklepilkina]

Fix format

[eklepilkina]

Hello! Decided to change this reviews stack. Could you please have a look?

[anton-afanasyev]

LGTM, with nit: it's not NFC now, adjust commit title.

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

1143

Line length is > 80

[eklepilkina]

• Fix format

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

 #include "llvm/IR/PatternMatch.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include <cassert>
 #include <cstdint>
 #include <string>
 
 using namespace llvm;
 using namespace llvm::PatternMatch;
 
+#define DEBUG_TYPE "separate-const-offset-from-gep"
+
 static cl::opt<bool> DisableSeparateConstOffsetFromGEP(
     "disable-separate-const-offset-from-gep", cl::init(false),
     cl::desc("Do not separate the constant offset from a GEP instruction"),
     cl::Hidden);
 
 // Setting this flag may emit false positives when the input module already
 // contains dead instructions. Therefore, we set it only in unit tests that are
 // free of dead code.
ConstantOffsetExtractor(Instruction *InsertionPt, const DominatorTree *DT)
       : IP(InsertionPt), DL(InsertionPt->getModule()->getDataLayout()), DT(DT) {
   }
 
   /// Searches the expression that computes V for a non-zero constant C s.t.
   /// V can be reassociated into the form V' + C. If the searching is
   /// successful, returns C and update UserChain as a def-use chain from C to V;
   /// otherwise, UserChain is empty.
   ///
-  /// \p V            The given expression
-  /// \p SignExtended Whether V will be sign-extended in the computation of the
-  ///                 GEP index
-  /// \p ZeroExtended Whether V will be zero-extended in the computation of the
-  ///                 GEP index
-  /// \p NonNegative  Whether V is guaranteed to be non-negative. For example,
-  ///                 an index of an inbounds GEP is guaranteed to be
-  ///                 non-negative. Levaraging this, we can better split
-  ///                 inbounds GEPs.
+  /// \p V                    The given expression
+  /// \p SignExtended         Whether V will be sign-extended in the computation
+  ///                         of the GEP index
+  /// \p ZeroExtended         Whether V will be zero-extended in the computation
+  ///                         of the GEP index
+  /// \p NonNegative          Whether V is guaranteed to be non-negative. For
+  ///                         example, an index of an inbounds GEP is guaranteed
+  ///                         to be non-negative. Levaraging this, we can better
+  ///                         split inbounds GEPs.
   APInt find(Value *V, bool SignExtended, bool ZeroExtended, bool NonNegative);
 
   /// A helper function to look into both operands of a binary operator.
   APInt findInEitherOperand(BinaryOperator *BO, bool SignExtended,
                             bool ZeroExtended);
 
   /// After finding the constant offset C from the GEP index I, we build a new
   /// index I' s.t. I' + C = I. This function builds and returns the new
if (AccumulativeByteOffset % ElementTypeSizeOfGEP == 0) {
     NewGEP->copyMetadata(*GEP);
     // Inherit the inbounds attribute of the original GEP.
     cast<GetElementPtrInst>(NewGEP)->setIsInBounds(GEPWasInBounds);
     if (GEP->getType() != I8PtrTy)
       NewGEP = new BitCastInst(NewGEP, GEP->getType(), GEP->getName(), GEP);
   }
 
   GEP->replaceAllUsesWith(NewGEP);
   GEP->eraseFromParent();
 
   return true;
 }
 
 bool SeparateConstOffsetFromGEPLegacyPass::runOnFunction(Function &F) {
   if (skipFunction(F))
     return false;
   auto *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
   auto *SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
   auto *LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
   auto *TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(F);
   auto GetTTI = [this](Function &F) -> TargetTransformInfo & {
     return this->getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
   };
   SeparateConstOffsetFromGEP Impl(DT, SE, LI, TLI, GetTTI, LowerGEP);
   return Impl.run(F);
 }
 
 bool SeparateConstOffsetFromGEP::run(Function &F) {
   if (DisableSeparateConstOffsetFromGEP)
     return false;
 
   DL = &F.getParent()->getDataLayout();
   bool Changed = false;
+
+  LLVM_DEBUG(dbgs() << "========= Function " << F.getName() << " =========\n");
+
   for (BasicBlock &B : F) {
     if (!DT->isReachableFromEntry(&B))
       continue;
 
     for (Instruction &I : llvm::make_early_inc_range(B))
       if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(&I))
         Changed |= splitGEP(GEP);
     // No need to split GEP ConstantExprs because all its indices are constant