[llvm-commits] [llvm] r138340 - in /llvm/trunk: include/llvm/Constant.h lib/Analysis/ConstantFolding.cpp lib/VMCore/Constants.cpp test/Transforms/InstCombine/bitcast.ll

[Duncan Sands]

Hi Nadav, thanks for doing this.

> Address Duncan's CR request:
> 1. Cleanup the tests in ConstantFolding.cpp
> 2. Implement isAllOnes for Constant, ConstantFP, ConstantVector
>
>
>
>
> Modified:
>      llvm/trunk/include/llvm/Constant.h
>      llvm/trunk/lib/Analysis/ConstantFolding.cpp
>      llvm/trunk/lib/VMCore/Constants.cpp
>      llvm/trunk/test/Transforms/InstCombine/bitcast.ll
>
> Modified: llvm/trunk/include/llvm/Constant.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Constant.h?rev=138340&r1=138339&r2=138340&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Constant.h (original)
> +++ llvm/trunk/include/llvm/Constant.h Tue Aug 23 12:48:43 2011
> @@ -51,6 +51,9 @@
>     /// isNullValue - Return true if this is the value that would be returned by
>     /// getNullValue.
>     bool isNullValue() const;

I think there should be a blank line here.

> +  /// isAllOnesValue - Return true if this is the value that would be returned by
> +  /// getAllOnesValue.
> +  bool isAllOnesValue() const;
>
>     /// isNegativeZeroValue - Return true if the value is what would be returned
>     /// by getZeroValueForNegation.
>

> Modified: llvm/trunk/lib/VMCore/Constants.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Constants.cpp?rev=138340&r1=138339&r2=138340&view=diff
> ==============================================================================
> --- llvm/trunk/lib/VMCore/Constants.cpp (original)
> +++ llvm/trunk/lib/VMCore/Constants.cpp Tue Aug 23 12:48:43 2011
> @@ -62,6 +62,21 @@
>     return isa<ConstantAggregateZero>(this) || isa<ConstantPointerNull>(this);
>   }
>
> +bool Constant::isAllOnesValue() const {
> +  // Check for -1 integers
> +  if (const ConstantInt *CI = dyn_cast<ConstantInt>(this))
> +    return CI->isAllOnesValue();

I think the ConstantInt version of isAllOnesValue should be removed, and the
logic put here instead (like for isNullValue).

> +
> +  // +0.0 is null.

This comment doesn't belong here.

> +  if (const ConstantFP *CFP = dyn_cast<ConstantFP>(this))
> +    return CFP->getValueAPF().bitcastToAPInt().isAllOnesValue();
> +
> +  // Check for constant vectors
> +  if (const ConstantVector *CV = dyn_cast<ConstantVector>(this))
> +    return CV->isAllOnesValue();

I think the ConstantVector version of isAllOnesValue should be removed, and the
logic put here instead (like for isNullValue).

> +
> +  return false;
> +}
>   // Constructor to create a '0' constant of arbitrary type...
>   Constant *Constant::getNullValue(Type *Ty) {
>     switch (Ty->getTypeID()) {
> @@ -126,7 +141,7 @@
>     SmallVector<Constant*, 16>  Elts;
>     VectorType *VTy = cast<VectorType>(Ty);
>     Elts.resize(VTy->getNumElements(), getAllOnesValue(VTy->getElementType()));
> -  assert(Elts[0]&&  "Not a vector integer type!");
> +  assert(Elts[0]&&  "Invalid AllOnes value!");
>     return cast<ConstantVector>(ConstantVector::get(Elts));
>   }

Please add support for all types, like for getNullValue.

Ciao, Duncan.