[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