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>