thr3ads.net - llvm dev - [LLVMdev] Bug in InsertElement constant propagation? [Jan 2015]

If this information is useful, please help other people find it:
Share via:

Raoux, Thomas F

2015-Jan-20 15:10 UTC

[LLVMdev] Bug in InsertElement constant propagation?

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);

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

Nick Lewycky

2015-Jan-20 19:01 UTC

head link

[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
>

Raoux, Thomas F

2015-Jan-20 21:41 UTC

head link

[LLVMdev] Bug in InsertElement constant propagation?

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
>

llvm dev - Jan 2015 - [LLVMdev] Bug in InsertElement constant propagation?

[LLVMdev] Bug in InsertElement constant propagation?

[LLVMdev] Bug in InsertElement constant propagation?

[LLVMdev] Bug in InsertElement constant propagation?