<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Actually please ignore me. After talking with Mehdi its clear that all the subclasses of Constant I mentioned would have triggered the same assert Mehdi originally found.<div class=""><br class=""></div><div class="">Sorry for the noise.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Pete<br class=""><div><blockquote type="cite" class=""><div class="">On Jan 15, 2015, at 9:31 AM, Pete Cooper <<a href="mailto:peter_cooper@apple.com" class="">peter_cooper@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi David<div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Pete<br class=""><div class=""><blockquote type="cite" class=""><div class="">On Jan 14, 2015, at 10:31 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com" class="">david.majnemer@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hi Mehdi,<div class=""><br class=""></div><div class="">I think this is a simple bug in Reassociate.</div><div class=""><br class=""></div><div class="">It's assuming that objects which are Constant but not ConstantFP must have a type which isn't floating point.</div><div class=""><br class=""></div><div class="">The code in question should be dispatching off of the type, not the particular subclass of the Constant hierarchy.</div><div class=""><br class=""></div><div class="">The following patch seems to work fine and passes all the tests in the repository:</div><div class=""><div class=""><br class=""></div><div class="">diff --git a/lib/Transforms/Scalar/Reassociate.cpp b/lib/Transforms/Scalar/Reassociate.cpp</div><div class="">index 4e02255..f618268 100644</div><div class="">--- a/lib/Transforms/Scalar/Reassociate.cpp</div><div class="">+++ b/lib/Transforms/Scalar/Reassociate.cpp</div><div class="">@@ -917,10 +917,12 @@ void Reassociate::RewriteExprTree(BinaryOperator *I,</div><div class=""> /// version of the value is returned, and BI is left pointing at the instruction</div><div class=""> /// that should be processed next by the reassociation pass.</div><div class=""> static Value *NegateValue(Value *V, Instruction *BI) {</div><div class="">- if (ConstantFP *C = dyn_cast<ConstantFP>(V))</div><div class="">- return ConstantExpr::getFNeg(C);</div><div class="">- if (Constant *C = dyn_cast<Constant>(V))</div><div class="">+ if (Constant *C = dyn_cast<Constant>(V)) {</div><div class="">+ if (C->getType()->isFPOrFPVectorTy()) {</div><div class="">+ return ConstantExpr::getFNeg(C);</div><div class="">+ }</div><div class=""> return ConstantExpr::getNeg(C);</div><div class="">+ }</div></div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Jan 14, 2015 at 9:42 PM, Mehdi Amini <span dir="ltr" class=""><<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi all,<br class="">
<br class="">
I have a very simple test case (thanks to bugpoint) that hit an assert in reassociate.<br class="">
(the assert is (C->getType()->isIntOrIntVectorTy() && "Cannot NEG a nonintegral value!"), function getNeg)<br class="">
<br class="">
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?).<br class="">
Do we have to defensively assume that any Constant can be an undef anywhere and I should just add a check here?<br class="">
<br class="">
Thanks,<br class="">
<br class="">
Mehdi<br class="">
<br class="">
PS: IR attached for reference.<br class="">
<br class="">
<br class="">_______________________________________________<br class="">
LLVM Developers mailing list<br class="">
<a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank" class="">http://llvm.cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class="">
<br class=""></blockquote></div><br class=""></div>
_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" class="">http://llvm.cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class=""></div></blockquote></div><br class=""></div></div></div></blockquote></div><br class=""></div></body></html>