[LLVMdev] Handling of undef in the IR

[Pete Cooper]

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.

Sorry for the noise.

Thanks,
Pete
> On Jan 15, 2015, at 9:31 AM, Pete Cooper <peter_cooper at apple.com> wrote:
> 
> 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 <mailto: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 <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>
> 

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