[InstCombine] Minor cleanup of cast simplification code [NFC]

[mreisinger]

Summary

This patch cleans up parts of InstCombine to raise its compliance with the LLVM coding standards and to increase its readability. The changes and according rationale are summarized in the following:

• Rename ShouldOptimizeCast() to shouldOptimizeCast() since functions should start with a lower case letter.

• Move shouldOptimizeCast() from InstCombineCasts.cpp to InstCombineAndOrXor.cpp since it's only used there.

• Simplify interface of shouldOptimizeCast().

• Minor code style adaptions in shouldOptimizeCast().

• Remove the documentation on the function definition of shouldOptimizeCast() since it just repeats the documentation on its declaration. Also enhance the documentation on its declaration with more information describing its intended use and make it doxygen-compliant.

• Change a comment in foldCastedBitwiseLogic() from fold (logic (cast A), (cast B)) -> (cast (logic A, B)) to fold logic(cast(A), cast(B)) -> cast(logic(A, B)) since the surrounding comments use this format.

• Remove comment Only do this if the casts both really cause code to be generated. in foldCastedBitwiseLogic() since it just repeats parts of the documentation of shouldOptimizeCast() and does not help to improve readability.

• Simplify the interface of isEliminableCastPair().

• Removed the documentation on the function definition of isEliminableCastPair() which only contained obvious statements about its implementation. Instead added more general doxygen-compliant documentation to its declaration.

• Renamed parameter DoXform of transformZExtIcmp() to DoTransform to make its intention clearer.

• Moved documentation of transformZExtIcmp() from its definition to its declaration and made it doxygen-compliant.

[grosser]

Hi Matthias,

thank you for the patch. It is technically very solid and despite the comments I will have, is also style-wise already at a good level. I still will provide you with plenty of comments -- most of them further suggestions on style, with the motivation to help you learn more about the style we use in LLVM and also to push your patch from "good" to "exceptional" quality. Future reviews will likely work a lot faster after general concepts have been discussed in the first commits. Feel free to raise concerns if you disagree with some of my suggestions.

One general comment: It seems 95% of your patch consists of documentation and code-style improvements that independently improve code-quality and which are not inter-tangled with the actual change you suggest. I personally prefer to split such changes out of an actual functionality change to upstream them separately. This has the following advantages:

• The cleanups can be marked as "NFC", no functional change, which makes them a lot easier to review
• Should the functionality change be incorrect and needs to be reverted, the beneficial cleanups remain in the code base
• The actual functionality change becomes a lot simpler (in your case 5 lines of code), which allows people skimming over the patch to quickly focus on the central part of this change.

I also suggest - despite the simplicity of the change - to expand your commit message with some more background on why you do something. Here some content that seems interesting for the reader:

InstCombine: Improve documentation of cast-folding logic

• Why do you rename the function?
• Why do you move the function between translation units - It seems the function is today only used by the casting logic and consequently belongs conceptually there. Is this right?

• Mention that you add full doxygen comments
• Mention that you simplify the interface of isEliminableCastPair

Enable folding ...

The commit message you have today already describes the core change that you apply. However, you spent a lot of time going through commit history to understand _why_ the logic as it exists today was introduced, why the isa<ICmpInst> are around, and why the existing solution seems to be a shortcut that aims to do exactly what you propose, but without making the check if casts can be folded explicit. Instead it bails out for any ICmpInst, even though in many cases this is not necessary and consequently optimization opportunities are missed. Also, you found that some of the functionality you propose today existed earlier, but was dropped in a later commit due to lack of test cases covering this. You could also refer to the original commit messages (SVN ids) in the context of this brief history lesson.

In general commit messages have two purposes. 1) Describe the actual change 2) establish knowledge on how LLVM works and establish a common coding style. Most likely several hundred people will skim over your commit message. Enabling them to learn from your commit (messages) increases the value of your change for LLVM.

It might make sense to make this review about the style changes and introduce a second review that for the actual functionality change.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

1222

• The 'braces' seem unnecessary.
• The comment does not talk about the innermost condition, no? It seems this is the most important part of this check.

lib/Transforms/InstCombine/InstCombineCasts.cpp

237

Nice cleanup!

265

Nice cleanup.

552

• DoTransform is clearly more readable

• What was the motivation of switching operand order here?

lib/Transforms/InstCombine/InstCombineInternal.h

358

• Drop '\brief'. In general we try to match local style as close as possible, but in respect of LLVM tries to move to

\brief comments without the explcit \brief keyword.

• "worthy" does not seem to be the right word, I would stay with "worth".

• Also, please add @param documentations (even though the surrounding code is clearly not very well documented).

Finally, the content of this comment seems slightly confusing. The function you document here contains the code:

  // If this cast is paired with another cast that can be eliminated, we prefer
	​  // to have it eliminated.
	​  if (const CastInst *PrecedingCI = dyn_cast<CastInst>(CastSrc))
	​    if (isEliminableCastPair(PrecedingCI, CI))
	​      return false;

which checks to me if two casts can be "elimintated" -- which to me is the same as "folded together". So in case casts are "folded". the function returns false, which is contrary to what your \brief comment seems to say.

364

Use @returns to describe the return value.

394

• Use @param to describe DoTransform and @returns to describe the return value.

• Drop "you just want to", as it does not carry any information

418

• Describe (briefly) what the function does in the first sentence, rather than that it is implemented as a wrapper to CastInst::isEliminableCastPair.

• Empty line between single "brief" sentence and the rest.

• Use @see to refer to other function.

• Document parameters with @params and return value with @returns.

test/Transforms/InstCombine/fold-casts-of-icmps.ll

5 ↗

(On Diff #64252)

I would suggest to add these examples to one of the existing files. Which of the test files is covering the "// fold (logic (cast A), (cast B)) -> (cast (logic A, B))" pattern for the currently handled cases? Is it cast.ll?

[mreisinger]

Thank you a lot for this extensive feedback, Tobias! I agree that it makes to make this an NFC for the style changes for now. As soon as this is merged I will open a separate PR for the functional changes then. Before updating the patch I also have a few questions (also see my responses to your inline comments).

Why do you move the function between translation units - It seems the function is today only used by the casting logic and consequently belongs conceptually there. Is this right?

shouldFoldCast (or ShouldOptimizeCast when using its original name) only seems to be a helper function for this specific optimization within InstCombineAndOrXor.cpp which is the reason why I preferred to move it there rather than leaving it in InstCombineCast.cpp. However, since its functionality is "cast centric" I could also let it stay there. What would be the more fitting choice in your opinion?

lib/Transforms/InstCombine/InstCombineCasts.cpp

552

What was the motivation of switching operand order here?

IMO it seemed a little bit unpleasant that in the function name we have ZExt before ICmp but the parameter order did not reflect that.

lib/Transforms/InstCombine/InstCombineInternal.h

358

Drop '\brief'. In general we try to match local style as close as possible, but in respect of LLVM tries to move to \brief comments without the explcit \brief keyword.

I was trying to comply with LLVM's "Coding Standards" document here http://llvm.org/docs/CodingStandards.html which proposes the use of the \brief keyword. Maybe this document does not yet reflect the new style goals?

which checks to me if two casts can be "elimintated" -- which to me is the same as "folded together". So in case casts are "folded". the function returns false, which is contrary to what your \brief comment seems to say.

You are right, this seems a little bit confusing. Since shouldFoldCast seems to be only used for this special "(logic (cast icmp), (cast icmp)) to (cast (logic (icmp), (icmp)))" scenario we should maybe make this intention explicit here? For example by also renaming this to something like shouldFoldCastWithinBitwiseLogic or shouldMoveCastAfterBitwiseLogic which would reflect its dedicated use in foldCastedBitwiseLogic more directly? Please also feel free to propose a better naming.

418

This is the original comment, I just moved it from the function definition to its declaration (just added a "\brief"). However, this is no excuse and I'm also in favor of making this comment a little more meaningful. However, since it basically just contains a call to CastInst::isEliminableCastPair (http://llvm.org/docs/doxygen/html/classllvm_1_1CastInst.html#a8b805c54dc1ead67b711a4b1cb72f492) one might be tempted to let it just refer to the documentation there. I will try to provide something useful here.

test/Transforms/InstCombine/fold-casts-of-icmps.ll

5 ↗

(On Diff #64252)

That's a good point. Unfortunately, I currently don't know where the original functionality is tested. I will try to figure this out. Since I will separate the tested functionality from this NFC, this will then be part of the next PR.

[mreisinger]

I updated the patch now to only contain style changes (see title and summary for detailed information). And you were right Tobias (regarding \brief), since autobrief is enabled \brief can safely be dropped here.

Please feel free to post further comments. I should also mention that I do not yet have commit access, so in case these changes can be merged, somebody with the according rights might have to do this. Thank you!

Best,
Matthias

[mreisinger]

Fixed some typos.

[grosser]

This patch looks great. Just a couple of minor comments left:

• I would suggest a slightly more descriptive title: [InstCombine] Minor cleanup of cast simplification code [NFC]

• Two typos (see below)

• Use present tense in commit message (you are describing the commit, not an event in the past)

lib/Transforms/InstCombine/InstCombineInternal.h

394

interested

395

interested

423

Add reference @see CastInst::isEliminableCastPair

[mreisinger]

Thank you for the feedback, I updated the patch accordingly.

Best,
Matthias

[majnemer]

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

1200

Please put returns on their own line.

1204

const auto * would be more concise.

1262–1265

Please clang-format this.

[grosser]

LGTM.

[mreisinger]

Thank you for bringing this to my notice @majnemer.

[grosser]

Committed in r275964

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

if (A->getType()->isIntegerTy(1) && B->getType()->isIntegerTy(1) &&
       Value *Not = Builder->CreateNot(LogicOp);
       return CastInst::CreateZExtOrBitCast(Not, I.getType());
     }
   }
 
   return nullptr;
 }
 
+bool InstCombiner::shouldOptimizeCast(CastInst *CI) {
+  Value *CastSrc = CI->getOperand(0);
+
+  // Noop casts and casts of constants should be eliminated trivially.
+  if (CI->getSrcTy() == CI->getDestTy() || isa<Constant>(CastSrc)) return false;
+
+  // If this cast is paired with another cast that can be eliminated, we prefer
+  // to have it eliminated.
+  if (const CastInst *PrecedingCI = dyn_cast<CastInst>(CastSrc))
+    if (isEliminableCastPair(PrecedingCI, CI))
+      return false;
+
+  // If this is a vector sext from a compare, then we don't want to break the
+  // idiom where each element of the extended vector is either zero or all ones.
+  if (CI->getOpcode() == Instruction::SExt &&
+      isa<CmpInst>(CastSrc) && CI->getDestTy()->isVectorTy())
+    return false;
+
+  return true;
+}
+
 Instruction *InstCombiner::foldCastedBitwiseLogic(BinaryOperator &I) {
   auto LogicOpc = I.getOpcode();
   assert((LogicOpc == Instruction::And || LogicOpc == Instruction::Or ||
           LogicOpc == Instruction::Xor) &&
          "Unexpected opcode for bitwise logic folding");
 
   Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
   CastInst *Cast0 = dyn_cast<CastInst>(Op0);
   if (!Cast0)
     return nullptr;
 
   // This must be a cast from an integer or integer vector source type to allow
   // transformation of the logic operation to the source type.
   Type *DestTy = I.getType();
Instruction *InstCombiner::foldCastedBitwiseLogic(BinaryOperator &I) {
   // same type for reduction.
   auto CastOpcode = Cast0->getOpcode();
   if (CastOpcode != Cast1->getOpcode() || SrcTy != Cast1->getSrcTy())
     return nullptr;
 
   Value *Cast0Src = Cast0->getOperand(0);
   Value *Cast1Src = Cast1->getOperand(0);
 
-  // fold (logic (cast A), (cast B)) -> (cast (logic A, B))
-
-  // Only do this if the casts both really cause code to be generated.
+  // fold logic(cast(A), cast(B)) -> cast(logic(A, B))
   if ((!isa<ICmpInst>(Cast0Src) || !isa<ICmpInst>(Cast1Src)) &&
-      ShouldOptimizeCast(CastOpcode, Cast0Src, DestTy) &&
-      ShouldOptimizeCast(CastOpcode, Cast1Src, DestTy)) {
+      shouldOptimizeCast(Cast0) &&
+      shouldOptimizeCast(Cast1)) {
     Value *NewOp = Builder->CreateBinOp(LogicOpc, Cast0Src, Cast1Src,
                                         I.getName());
     return CastInst::Create(CastOpcode, NewOp, DestTy);
   }
 
   // For now, only 'and'/'or' have optimizations after this.
   if (LogicOpc == Instruction::Xor)
     return nullptr;
 

lib/Transforms/InstCombine/InstCombineCasts.cpp

default:
     // TODO: Can handle more cases here.
     llvm_unreachable("Unreachable!");
   }
 
   Res->takeName(I);
   return InsertNewInstWith(Res, *I);
 }
 
+Instruction::CastOps InstCombiner::isEliminableCastPair(const CastInst *CI1,
+                                                        const CastInst *CI2) {
+  Type *SrcTy = CI1->getSrcTy();
+  Type *MidTy = CI1->getDestTy();
+  Type *DstTy = CI2->getDestTy();
 
-/// This function is a wrapper around CastInst::isEliminableCastPair. It
-/// simply extracts arguments and returns what that function returns.
-static Instruction::CastOps
-isEliminableCastPair(const CastInst *CI, ///< First cast instruction
-                     unsigned opcode,    ///< Opcode for the second cast
-                     Type *DstTy,        ///< Target type for the second cast
-                     const DataLayout &DL) {
-  Type *SrcTy = CI->getOperand(0)->getType();   // A from above
-  Type *MidTy = CI->getType();                  // B from above
-
-  // Get the opcodes of the two Cast instructions
-  Instruction::CastOps firstOp = Instruction::CastOps(CI->getOpcode());
-  Instruction::CastOps secondOp = Instruction::CastOps(opcode);
+  Instruction::CastOps firstOp = Instruction::CastOps(CI1->getOpcode());
+  Instruction::CastOps secondOp = Instruction::CastOps(CI2->getOpcode());
   Type *SrcIntPtrTy =
       SrcTy->isPtrOrPtrVectorTy() ? DL.getIntPtrType(SrcTy) : nullptr;
   Type *MidIntPtrTy =
       MidTy->isPtrOrPtrVectorTy() ? DL.getIntPtrType(MidTy) : nullptr;
   Type *DstIntPtrTy =
       DstTy->isPtrOrPtrVectorTy() ? DL.getIntPtrType(DstTy) : nullptr;
   unsigned Res = CastInst::isEliminableCastPair(firstOp, secondOp, SrcTy, MidTy,
                                                 DstTy, SrcIntPtrTy, MidIntPtrTy,
                                                 DstIntPtrTy);
 
   // We don't want to form an inttoptr or ptrtoint that converts to an integer
   // type that differs from the pointer size.
   if ((Res == Instruction::IntToPtr && SrcTy != DstIntPtrTy) ||
       (Res == Instruction::PtrToInt && DstTy != SrcIntPtrTy))
     Res = 0;
 
   return Instruction::CastOps(Res);
 }
 
-/// Return true if the cast from "V to Ty" actually results in any code being
-/// generated and is interesting to optimize out.
-/// If the cast can be eliminated by some other simple transformation, we prefer
-/// to do the simplification first.
-bool InstCombiner::ShouldOptimizeCast(Instruction::CastOps opc, const Value *V,
-                                      Type *Ty) {
-  // Noop casts and casts of constants should be eliminated trivially.
-  if (V->getType() == Ty || isa<Constant>(V)) return false;
-
-  // If this is another cast that can be eliminated, we prefer to have it
-  // eliminated.
-  if (const CastInst *CI = dyn_cast<CastInst>(V))
-    if (isEliminableCastPair(CI, opc, Ty, DL))
-      return false;
-
-  // If this is a vector sext from a compare, then we don't want to break the
-  // idiom where each element of the extended vector is either zero or all ones.
-  if (opc == Instruction::SExt && isa<CmpInst>(V) && Ty->isVectorTy())
-    return false;
-
-  return true;
-}
-
-
 /// @brief Implement the transforms common to all CastInst visitors.
 Instruction *InstCombiner::commonCastTransforms(CastInst &CI) {
   Value *Src = CI.getOperand(0);
 
   // Many cases of "cast of a cast" are eliminable. If it's eliminable we just
   // eliminate it now.
   if (CastInst *CSrc = dyn_cast<CastInst>(Src)) {   // A->B->C cast
     if (Instruction::CastOps opc =
-            isEliminableCastPair(CSrc, CI.getOpcode(), CI.getType(), DL)) {
+            isEliminableCastPair(CSrc, &CI)) {
       // The first cast (CSrc) is eliminable so we need to fix up or replace
       // the second cast (CI). CSrc will then have a good chance of being dead.
       return CastInst::Create(opc, CSrc->getOperand(0), CI.getType());
     }
   }
 
   // If we are casting a select then fold the cast into the select
   if (SelectInst *SI = dyn_cast<SelectInst>(Src))
Instruction *InstCombiner::visitTrunc(TruncInst &CI) {
   }
 
   if (Instruction *I = foldVecTruncToExtElt(CI, *this, DL))
     return I;
 
   return nullptr;
 }
 
-/// Transform (zext icmp) to bitwise / integer operations in order to eliminate
-/// the icmp.
-Instruction *InstCombiner::transformZExtICmp(ICmpInst *ICI, Instruction &CI,
-                                             bool DoXform) {
+Instruction *InstCombiner::transformZExtICmp(ICmpInst *ICI, ZExtInst &CI,
+                                             bool DoTransform) {
   // If we are just checking for a icmp eq of a single bit and zext'ing it
   // to an integer, then shift the bit to the appropriate place and then
   // cast to integer to avoid the comparison.
   if (ConstantInt *Op1C = dyn_cast<ConstantInt>(ICI->getOperand(1))) {
     const APInt &Op1CV = Op1C->getValue();
 
     // zext (x <s  0) to i32 --> x>>u31      true if signbit set.
     // zext (x >s -1) to i32 --> (x>>u31)^1  true if signbit clear.
     if ((ICI->getPredicate() == ICmpInst::ICMP_SLT && Op1CV == 0) ||
         (ICI->getPredicate() == ICmpInst::ICMP_SGT && Op1CV.isAllOnesValue())) {
-      if (!DoXform) return ICI;
+      if (!DoTransform) return ICI;
 
       Value *In = ICI->getOperand(0);
       Value *Sh = ConstantInt::get(In->getType(),
                                    In->getType()->getScalarSizeInBits() - 1);
       In = Builder->CreateLShr(In, Sh, In->getName() + ".lobit");
       if (In->getType() != CI.getType())
         In = Builder->CreateIntCast(In, CI.getType(), false/*ZExt*/);
 
if ((Op1CV == 0 || Op1CV.isPowerOf2()) &&
         ICI->isEquality()) {
       // If Op1C some other power of two, convert:
       uint32_t BitWidth = Op1C->getType()->getBitWidth();
       APInt KnownZero(BitWidth, 0), KnownOne(BitWidth, 0);
       computeKnownBits(ICI->getOperand(0), KnownZero, KnownOne, 0, &CI);
 
       APInt KnownZeroMask(~KnownZero);
       if (KnownZeroMask.isPowerOf2()) { // Exactly 1 possible 1?
-        if (!DoXform) return ICI;
+        if (!DoTransform) return ICI;
 
         bool isNE = ICI->getPredicate() == ICmpInst::ICMP_NE;
         if (Op1CV != 0 && (Op1CV != KnownZeroMask)) {
           // (X&4) == 2 --> false
           // (X&4) != 2 --> true
           Constant *Res = ConstantInt::get(Type::getInt1Ty(CI.getContext()),
                                            isNE);
           Res = ConstantExpr::getZExt(Res, CI.getType());
if (IntegerType *ITy = dyn_cast<IntegerType>(CI.getType())) {
       APInt KnownZeroRHS(BitWidth, 0), KnownOneRHS(BitWidth, 0);
       computeKnownBits(LHS, KnownZeroLHS, KnownOneLHS, 0, &CI);
       computeKnownBits(RHS, KnownZeroRHS, KnownOneRHS, 0, &CI);
 
       if (KnownZeroLHS == KnownZeroRHS && KnownOneLHS == KnownOneRHS) {
         APInt KnownBits = KnownZeroLHS | KnownOneLHS;
         APInt UnknownBit = ~KnownBits;
         if (UnknownBit.countPopulation() == 1) {
-          if (!DoXform) return ICI;
+          if (!DoTransform) return ICI;
 
           Value *Result = Builder->CreateXor(LHS, RHS);
 
           // Mask off any bits that are set and won't be shifted away.
           if (KnownOneLHS.uge(UnknownBit))
             Result = Builder->CreateAnd(Result,
                                         ConstantInt::get(ITy, UnknownBit));
 

lib/Transforms/InstCombine/InstCombineInternal.h

private:
   bool ShouldChangeType(unsigned FromBitWidth, unsigned ToBitWidth) const;
   bool ShouldChangeType(Type *From, Type *To) const;
   Value *dyn_castNegVal(Value *V) const;
   Value *dyn_castFNegVal(Value *V, bool NoSignedZero = false) const;
   Type *FindElementAtOffset(PointerType *PtrTy, int64_t Offset,
                             SmallVectorImpl<Value *> &NewIndices);
   Instruction *FoldOpIntoSelect(Instruction &Op, SelectInst *SI);
 
-  /// \brief Classify whether a cast is worth optimizing.
+  /// Classify whether a cast is worth optimizing.
   ///
-  /// Returns true if the cast from "V to Ty" actually results in any code
-  /// being generated and is interesting to optimize out. If the cast can be
-  /// eliminated by some other simple transformation, we prefer to do the
-  /// simplification first.
-  bool ShouldOptimizeCast(Instruction::CastOps opcode, const Value *V,
-                          Type *Ty);
+  /// This is a helper to decide whether the simplification of
+  /// logic(cast(A), cast(B)) to cast(logic(A, B)) should be performed.
+  ///
+  /// \param CI The cast we are interested in.
+  ///
+  /// \return true if this cast actually results in any code being generated and
+  /// if it cannot already be eliminated by some other transformation.
+  bool shouldOptimizeCast(CastInst *CI);
 
   /// \brief Try to optimize a sequence of instructions checking if an operation
   /// on LHS and RHS overflows.
   ///
   /// If this overflow check is done via one of the overflow check intrinsics,
   /// then CtxI has to be the call instruction calling that intrinsic.  If this
   /// overflow check is done by arithmetic followed by a compare, then CtxI has
   /// to be the arithmetic instruction.
   ///
   /// If a simplification is possible, stores the simplified result of the
   /// operation in OperationResult and result of the overflow check in
   /// OverflowResult, and return true.  If no simplification is possible,
   /// returns false.
   bool OptimizeOverflowCheck(OverflowCheckFlavor OCF, Value *LHS, Value *RHS,
                              Instruction &CtxI, Value *&OperationResult,
                              Constant *&OverflowResult);
 
   Instruction *visitCallSite(CallSite CS);
   Instruction *tryOptimizeCall(CallInst *CI);
   bool transformConstExprCastCall(CallSite CS);
   Instruction *transformCallThroughTrampoline(CallSite CS,
                                               IntrinsicInst *Tramp);
-  Instruction *transformZExtICmp(ICmpInst *ICI, Instruction &CI,
-                                 bool DoXform = true);
+
+  /// Transform (zext icmp) to bitwise / integer operations in order to
+  /// eliminate it.
+  ///
+  /// \param ICI The icmp of the (zext icmp) pair we are intersted in.
+  /// \parem CI The zext of the (zext icmp) pair we are intersted in.
+  /// \param DoTransform Pass false to just test whether the given (zext icmp)
+  /// would be transformed. Pass true to actually perform the transformation.
+  ///
+  /// \return null if the transformation cannot be performed. If the
+  /// transformation can be performed the new instruction that replaces the
+  /// (zext icmp) pair will be returned (if \p DoTransform is false the
+  /// unmodified \p ICI will be returned in this case).
+  Instruction *transformZExtICmp(ICmpInst *ICI, ZExtInst &CI,
+                                 bool DoTransform = true);
+
   Instruction *transformSExtICmp(ICmpInst *ICI, Instruction &CI);
   bool WillNotOverflowSignedAdd(Value *LHS, Value *RHS, Instruction &CxtI);
   bool WillNotOverflowSignedSub(Value *LHS, Value *RHS, Instruction &CxtI);
   bool WillNotOverflowUnsignedSub(Value *LHS, Value *RHS, Instruction &CxtI);
   bool WillNotOverflowSignedMul(Value *LHS, Value *RHS, Instruction &CxtI);
   Value *EmitGEPOffset(User *GEP);
   Instruction *scalarizePHI(ExtractElementInst &EI, PHINode *PN);
   Value *EvaluateInDifferentElementOrder(Value *V, ArrayRef<int> Mask);
   Instruction *foldCastedBitwiseLogic(BinaryOperator &I);
 
+  /// Determine if a pair of casts can be replaced by a single cast.
+  ///
+  /// \param CI1 The first of a pair of casts.
+  /// \param CI2 The second of a pair of casts.
+  ///
+  /// \return 0 if the cast pair cannot be eliminated, otherwise returns a
+  /// Instruction::CastOps value for a cast that can replace the pair, casting
+  /// CI1->SrcTy() to CI2->DstTy().
+  Instruction::CastOps isEliminableCastPair(const CastInst *CI1,
+                                            const CastInst *CI2);
+
 public:
   /// \brief Inserts an instruction \p New before instruction \p Old
   ///
   /// Also adds the new instruction to the worklist and returns \p New so that
   /// it is suitable for use as the return from the visitation patterns.
   Instruction *InsertNewInstBefore(Instruction *New, Instruction &Old) {
     assert(New && !New->getParent() &&
            "New instruction already inserted into a basic block!");