[LLVMdev] Bug in InsertElement constant propagation?

[Raoux, Thomas F]

So right now the constant is converted to a float using convertToFloat (which uses bitCasttoAPInt) so the original float value is fine but then when it gets passed around it may change. 

if (ConstantFP *CFP = dyn_cast<ConstantFP>(C)) {
      if (CFP->getType()->isFloatTy()) {
        SmallVector<float, 16> Elts;
        for (unsigned i = 0, e = V.size(); i != e; ++i)
          if (ConstantFP *CFP = dyn_cast<ConstantFP>(V[i]))
            Elts.push_back(CFP->getValueAPF().convertToFloat()); --> Here the copy of float may change the raw value
          else
            break;
        if (Elts.size() == V.size())
          return ConstantDataArray::get(C->getContext(), Elts);
      }

Yes what I have in mind is to provide a constructor taking Constant* as parameter which would store the data in the raw format as it is intended for ConstantDataVector/ConstantDataArray.

Thomas

-----Original Message-----
From: Nick Lewycky [mailto:nicholas at mxc.ca] 
Sent: Tuesday, January 20, 2015 11:01 AM
To: Raoux, Thomas F
Cc: LLVM Developers Mailing List
Subject: Re: [LLVMdev] Bug in InsertElement constant propagation?

Raoux, Thomas F wrote:
> Does anybody else have an opinion on this issue? I'm planning to submit a patch which would add a new get method for ConstantDataVector taking an ArrayRef<Constant*>  and use that in the few places in constant propagation where convertToFloat is used.
>
> Let me know if you think there is a more obvious way to do it.
>
> Right now the only way to create a ConstantDataVector are those method:
>
>    /// get() constructors - Return a constant with vector type with an element
>    /// count and element type matching the ArrayRef passed in.  Note that this
>    /// can return a ConstantAggregateZero object.
>    static Constant *get(LLVMContext&Context, ArrayRef<uint8_t>  Elts);
>    static Constant *get(LLVMContext&Context, ArrayRef<uint16_t>  Elts);
>    static Constant *get(LLVMContext&Context, ArrayRef<uint32_t>  Elts);
>    static Constant *get(LLVMContext&Context, ArrayRef<uint64_t>  Elts);
>    static Constant *get(LLVMContext&Context, ArrayRef<float>  Elts);
>    static Constant *get(LLVMContext&Context, ArrayRef<double>  Elts);
>
>    /// getSplat - Return a ConstantVector with the specified constant in each
>    /// element.  The specified constant has to be a of a compatible type (i8/i16/
>    /// i32/i64/float/double) and must be a ConstantFP or ConstantInt.
>    static Constant *getSplat(unsigned NumElts, Constant *Elt);

I haven't followed the whole thread through, but my first question is whether APFloat::bitcastToAPInt helps? Secondly, what is your constructor doing with this Constant*, the point of ConstantDataFoo is to avoid the construction of the Constant and its internal Use objects
-- especially to avoid the Use objects. It could make sense if your ctor takes the Constant merely as an input to rip the data out of, but not if it tries to hold the Constant* in any way.

Nick

>
> Cheers,
> Thomas
>
>
> -----Original Message-----
> From: Jonathan Roelofs [mailto:jonathan at codesourcery.com]
> Sent: Thursday, January 15, 2015 6:36 AM
> To: Raoux, Thomas F; LLVM Developers Mailing List
> Subject: Re: [LLVMdev] Bug in InsertElement constant propagation?
>
> I'm not sure what the right fix is. I don't see a way to get the float out of APFloat without a copy happening. The usual trick to copy a float somewhere else but preserve the NaN bits is to use memcpy.
>
> Jon
>
> On 1/15/15 2:29 AM, Raoux, Thomas F wrote:
>> I don't see a way to create a ConstantDataVector from Constant or form APFloat though. Did I oversee that?
>> Is the solution to had a new get function in ConstantDataVector to allow that? Any hint on what would be the right fix otherwise?
>>
>> Thomas
>>
>> -----Original Message-----
>> From: Jonathan Roelofs [mailto:jonathan at codesourcery.com]
>> Sent: Wednesday, January 14, 2015 10:30 AM
>> To: Raoux, Thomas F; LLVM Developers Mailing List
>> Subject: Re: [LLVMdev] Bug in InsertElement constant propagation?
>>
>>
>>
>> On 1/14/15 11:12 AM, Raoux, Thomas F wrote:
>>> Ha here is what I was missing. Thanks Jon. It still seems to me that the transformation of LLVM IR is invalid is that right?
>> I don't know if IR is required to preserve NaN bit patterns, but ISTM that it would be better if it did.
>>> I assume we shouldn't be converting APFloat to float in order to avoid such problems?
>> Yes, I think that's the correct fix.
>>
>> Jon
>>
>> --
>> Jon Roelofs
>> jonathan at codesourcery.com
>> CodeSourcery / Mentor Embedded
>>
>
> --
> Jon Roelofs
> jonathan at codesourcery.com
> CodeSourcery / Mentor Embedded
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>