[llvm] r254456 - IR: Clean up some duplicated code in ConstantDataSequential creation. NFC

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 1 16:44:06 PST 2015


> On 2015-Dec-01, at 12:20, Justin Bogner via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: bogner
> Date: Tue Dec  1 14:20:49 2015
> New Revision: 254456
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=254456&view=rev
> Log:
> IR: Clean up some duplicated code in ConstantDataSequential creation. NFC
> 
> ConstantDataArray::getImpl and ConstantDataVector::getImpl had a lot
> of copy pasta in how they handled sequences of constants. Break that
> out into a couple of simple functions.
> 

This is way better.

> Modified:
>    llvm/trunk/lib/IR/Constants.cpp
> 
> Modified: llvm/trunk/lib/IR/Constants.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Constants.cpp?rev=254456&r1=254455&r2=254456&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Constants.cpp (original)
> +++ llvm/trunk/lib/IR/Constants.cpp Tue Dec  1 14:20:49 2015
> @@ -857,6 +857,57 @@ static bool rangeOnlyContains(ItTy Start
>   return true;
> }
> 
> +template <typename SequentialTy, typename ElementTy>
> +static Constant *getIntSequenceIfElementsMatch(ArrayRef<Constant *> V) {
> +  assert(!V.empty() && "Cannot get empty int sequence.");
> +
> +  SmallVector<ElementTy, 16> Elts;
> +  for (Constant *C : V)
> +    if (auto *CI = dyn_cast<ConstantInt>(C))
> +      Elts.push_back(CI->getZExtValue());
> +    else
> +      return nullptr;
> +  return SequentialTy::get(V[0]->getContext(), Elts);
> +}
> +
> +template <typename SequentialTy, typename ElementTy>
> +static Constant *getFPSequenceIfElementsMatch(ArrayRef<Constant *> V) {
> +  assert(!V.empty() && "Cannot get empty FP sequence.");
> +
> +  SmallVector<ElementTy, 16> Elts;
> +  for (Constant *C : V)
> +    if (auto *CFP = dyn_cast<ConstantFP>(C))
> +      Elts.push_back(CFP->getValueAPF().bitcastToAPInt().getLimitedValue());
> +    else
> +      return nullptr;
> +  return SequentialTy::getFP(V[0]->getContext(), Elts);
> +}
> +
> +template <typename SequenceTy>
> +static Constant *getSequenceIfElementsMatch(Constant *C,
> +                                            ArrayRef<Constant *> V) {
> +  // We speculatively build the elements here even if it turns out that there is
> +  // a constantexpr or something else weird, since it is so uncommon for that to
> +  // happen.
> +  if (ConstantInt *CI = dyn_cast<ConstantInt>(C)) {
> +    if (CI->getType()->isIntegerTy(8))
> +      return getIntSequenceIfElementsMatch<SequenceTy, uint8_t>(V);
> +    else if (CI->getType()->isIntegerTy(16))
> +      return getIntSequenceIfElementsMatch<SequenceTy, uint16_t>(V);
> +    else if (CI->getType()->isIntegerTy(32))
> +      return getIntSequenceIfElementsMatch<SequenceTy, uint32_t>(V);
> +    else if (CI->getType()->isIntegerTy(64))
> +      return getIntSequenceIfElementsMatch<SequenceTy, uint64_t>(V);
> +  } else if (ConstantFP *CFP = dyn_cast<ConstantFP>(C)) {
> +    if (CFP->getType()->isFloatTy())
> +      return getFPSequenceIfElementsMatch<SequenceTy, uint32_t>(V);
> +    else if (CFP->getType()->isDoubleTy())
> +      return getFPSequenceIfElementsMatch<SequenceTy, uint64_t>(V);
> +  }
> +
> +  return nullptr;
> +}
> +
> ConstantArray::ConstantArray(ArrayType *T, ArrayRef<Constant *> V)
>   : Constant(T, ConstantArrayVal,
>              OperandTraits<ConstantArray>::op_end(this) - V.size(),
> @@ -874,6 +925,7 @@ Constant *ConstantArray::get(ArrayType *
>     return C;
>   return Ty->getContext().pImpl->ArrayConstants.getOrCreate(Ty, V);
> }
> +
> Constant *ConstantArray::getImpl(ArrayType *Ty, ArrayRef<Constant*> V) {
>   // Empty arrays are canonicalized to ConstantAggregateZero.
>   if (V.empty())
> @@ -896,74 +948,8 @@ Constant *ConstantArray::getImpl(ArrayTy
> 
>   // Check to see if all of the elements are ConstantFP or ConstantInt and if
>   // the element type is compatible with ConstantDataVector.  If so, use it.
> -  if (ConstantDataSequential::isElementTypeCompatible(C->getType())) {
> -    // We speculatively build the elements here even if it turns out that there
> -    // is a constantexpr or something else weird in the array, since it is so
> -    // uncommon for that to happen.
> -    if (ConstantInt *CI = dyn_cast<ConstantInt>(C)) {
> -      if (CI->getType()->isIntegerTy(8)) {
> -        SmallVector<uint8_t, 16> Elts;
> -        for (unsigned i = 0, e = V.size(); i != e; ++i)
> -          if (ConstantInt *CI = dyn_cast<ConstantInt>(V[i]))
> -            Elts.push_back(CI->getZExtValue());
> -          else
> -            break;
> -        if (Elts.size() == V.size())
> -          return ConstantDataArray::get(C->getContext(), Elts);
> -      } else if (CI->getType()->isIntegerTy(16)) {
> -        SmallVector<uint16_t, 16> Elts;
> -        for (unsigned i = 0, e = V.size(); i != e; ++i)
> -          if (ConstantInt *CI = dyn_cast<ConstantInt>(V[i]))
> -            Elts.push_back(CI->getZExtValue());
> -          else
> -            break;
> -        if (Elts.size() == V.size())
> -          return ConstantDataArray::get(C->getContext(), Elts);
> -      } else if (CI->getType()->isIntegerTy(32)) {
> -        SmallVector<uint32_t, 16> Elts;
> -        for (unsigned i = 0, e = V.size(); i != e; ++i)
> -          if (ConstantInt *CI = dyn_cast<ConstantInt>(V[i]))
> -            Elts.push_back(CI->getZExtValue());
> -          else
> -            break;
> -        if (Elts.size() == V.size())
> -          return ConstantDataArray::get(C->getContext(), Elts);
> -      } else if (CI->getType()->isIntegerTy(64)) {
> -        SmallVector<uint64_t, 16> Elts;
> -        for (unsigned i = 0, e = V.size(); i != e; ++i)
> -          if (ConstantInt *CI = dyn_cast<ConstantInt>(V[i]))
> -            Elts.push_back(CI->getZExtValue());
> -          else
> -            break;
> -        if (Elts.size() == V.size())
> -          return ConstantDataArray::get(C->getContext(), Elts);
> -      }
> -    }
> -
> -    if (ConstantFP *CFP = dyn_cast<ConstantFP>(C)) {
> -      if (CFP->getType()->isFloatTy()) {
> -        SmallVector<uint32_t, 16> Elts;
> -        for (unsigned i = 0, e = V.size(); i != e; ++i)
> -          if (ConstantFP *CFP = dyn_cast<ConstantFP>(V[i]))
> -            Elts.push_back(
> -                CFP->getValueAPF().bitcastToAPInt().getLimitedValue());
> -          else
> -            break;
> -        if (Elts.size() == V.size())
> -          return ConstantDataArray::getFP(C->getContext(), Elts);
> -      } else if (CFP->getType()->isDoubleTy()) {
> -        SmallVector<uint64_t, 16> Elts;
> -        for (unsigned i = 0, e = V.size(); i != e; ++i)
> -          if (ConstantFP *CFP = dyn_cast<ConstantFP>(V[i]))
> -            Elts.push_back(
> -                CFP->getValueAPF().bitcastToAPInt().getLimitedValue());
> -          else
> -            break;
> -        if (Elts.size() == V.size())
> -          return ConstantDataArray::getFP(C->getContext(), Elts);
> -      }
> -    }
> -  }
> +  if (ConstantDataSequential::isElementTypeCompatible(C->getType()))

Do you need this check, or can you just return all the time?

(Same question for `ConstantVector::getImpl()`.)

> +    return getSequenceIfElementsMatch<ConstantDataArray>(C, V);
> 
>   // Otherwise, we really do want to create a ConstantArray.
>   return nullptr;



More information about the llvm-commits mailing list