[llvm-commits] [PATCH] Fix constant propagation through int->float->int bitcasts

[Kristof Beyls]

Thanks for the quick code review.
I've adapted the patch following your feedback, and committed as r163703.

Thanks,

Kristof

> -----Original Message-----
> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> Sent: 11 September 2012 20:57
> To: Kristof Beyls
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] Fix constant propagation through
> int->float->int bitcasts
> 
> On Tue, Sep 11, 2012 at 8:43 AM, Kristof Beyls <kristof.beyls at arm.com>
> wrote:
> > Hi,
> >
> > The attached patch fixes a wrong codegen error that is triggered on
> the
> > following test case:
> >
> > define i32 @f8() nounwind {
> > ; Check that constant propagation through (i32)-1 => (float)Nan =>
> (i32)-1
> > ; gives expected result
> > ; CHECK: mvn r0, #0
> >         %tmp0 = bitcast i32 -1 to float
> >         %tmp1 = bitcast float %tmp0 to i32
> >         ret i32 %tmp1
> > }
> >
> > On x86 hosts, this produces the expected mvn r0,#0 instruction.
> > On ARM hosts, this produces the incorrect following sequence of
> > instructions:
> >         mov     r0, #1069547520
> >         orr     r0, r0, #1073741824
> >
> >
> > The difference in code generation on different host platforms is
> caused
> > because
> > LLVM converts NaN from double to float while compiling this, which I
> believe
> > is
> > undefined behaviour. The attached patch avoids the undefined
> behaviour by
> > calling
> >   SDValue SelectionDAG::getConstantFP(const APFloat& Val, EVT VT,
> bool
> > isTarget);
> > instead of
> >   SDValue SelectionDAG::getConstantFP(double Val, EVT VT, bool
> isTarget);
> >
> > Please review.
> 
> Your test is missing a CHECK line for the start of the function
> ("CHECK: f8:" or something like that.
> 
> "APFloat(Val.bitsToFloat())" is equivalent to "APFloat(Val)" if Val is
> a 32-bit APInt.
> 
> Otherwise looks fine.
> 
> -Eli