[LLVMdev] Handling of undef in the IR

[Pete Cooper]

Hi David

Your change will actually be a change in behavior as 'C->getType()->isFPOrFPVectorTy()’ will catch float vectors in ConstantAggregateZero.  Also looks like floats can be in ConstantDataVector or maybe even ConstantDataArray although I don’t see an easy way to ensure thats what we parse from an .ll file.

So I think its a good change, but we should probably add a test case for this new behavior, if its even possible to get a 0.0 all the way through to this point, which it might not be.

Thanks,
Pete
> On Jan 14, 2015, at 10:31 PM, David Majnemer <david.majnemer at gmail.com> wrote:
> 
> Hi Mehdi,
> 
> I think this is a simple bug in Reassociate.
> 
> It's assuming that objects which are Constant but not ConstantFP must have a type which isn't floating point.
> 
> The code in question should be dispatching off of the type, not the particular subclass of the Constant hierarchy.
> 
> The following patch seems to work fine and passes all the tests in the repository:
> 
> diff --git a/lib/Transforms/Scalar/Reassociate.cpp b/lib/Transforms/Scalar/Reassociate.cpp
> index 4e02255..f618268 100644
> --- a/lib/Transforms/Scalar/Reassociate.cpp
> +++ b/lib/Transforms/Scalar/Reassociate.cpp
> @@ -917,10 +917,12 @@ void Reassociate::RewriteExprTree(BinaryOperator *I,
>  /// version of the value is returned, and BI is left pointing at the instruction
>  /// that should be processed next by the reassociation pass.
>  static Value *NegateValue(Value *V, Instruction *BI) {
> -  if (ConstantFP *C = dyn_cast<ConstantFP>(V))
> -    return ConstantExpr::getFNeg(C);
> -  if (Constant *C = dyn_cast<Constant>(V))
> +  if (Constant *C = dyn_cast<Constant>(V)) {
> +    if (C->getType()->isFPOrFPVectorTy()) {
> +      return ConstantExpr::getFNeg(C);
> +    }
>      return ConstantExpr::getNeg(C);
> +  }
> 
> On Wed, Jan 14, 2015 at 9:42 PM, Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
> Hi all,
> 
> I have a very simple test case (thanks to bugpoint) that hit an assert in reassociate.
> (the assert is (C->getType()->isIntOrIntVectorTy() && "Cannot NEG a nonintegral value!"), function getNeg)
> 
> The function is taking a Constant as argument, but the assert does not expect an undef. I’m not sure whose responsibility is it to handle that (caller?).
> Do we have to defensively assume that any Constant can be an undef anywhere and I should just add a check here?
> 
> Thanks,
> 
> Mehdi
> 
> PS: IR attached for reference.
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu>         http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150115/d4b82b5a/attachment.html>