[llvm] r237247 - [AArch64] Codegen VMAX/VMIN for safe math cases

[Silviu Baranga]

Hi Artyom,

The fix looks good to me. If it solves the issues, please recommit!

Cheers,
Silviu

> -----Original Message-----
> From: James Molloy
> Sent: 13 May 2015 19:00
> To: Artyom Skrobov
> Cc: Silviu Baranga; llvm-commits at cs.uiuc.edu; James Molloy
> Subject: Re: [llvm] r237247 - [AArch64] Codegen VMAX/VMIN for safe math
> cases
> 
> Hi Artyom,
> 
> Given this has caused immediate failures, please run at least the LLVM
test
> suite and ideally spec2k6, especially as you have access to our testing
> resources.
> 
> Cheers,
> 
> James
> 
> 
> On 13 May 2015, at 18:52, Artyom Skrobov <Artyom.Skrobov at arm.com>
> wrote:
> 
> >> Unfortunately I had to revert this as it was causing some failures in
> >> spec2000/2006.
> >
> > That's right: the new code was creating integer FMAX nodes, and ISel
> > was unsurprising failing to select them...
> >
> > Suggesting this fix:
> >
> > Index: lib/Target/AArch64/AArch64ISelLowering.cpp
> >
> ==========================================================
> =========
> > --- lib/Target/AArch64/AArch64ISelLowering.cpp    (revision 237247)
> > +++ lib/Target/AArch64/AArch64ISelLowering.cpp    (working copy)
> > @@ -3568,7 +3568,8 @@
> > /// operations would *not* be semantically equivalent.
> > static bool selectCCOpsAreFMaxCompatible(SDValue Cmp, SDValue Result)
> {
> >   if (Cmp == Result)
> > -    return true;
> > +    return (Cmp.getValueType() == MVT::f32 ||
> > +            Cmp.getValueType() == MVT::f64);
> >
> >   ConstantFPSDNode *CCmp = dyn_cast<ConstantFPSDNode>(Cmp);
> >   ConstantFPSDNode *CResult = dyn_cast<ConstantFPSDNode>(Result);
> > Index: test/CodeGen/AArch64/arm64-fmax.ll
> >
> ==========================================================
> =========
> > --- test/CodeGen/AArch64/arm64-fmax.ll    (revision 237247)
> > +++ test/CodeGen/AArch64/arm64-fmax.ll    (working copy)
> > @@ -53,3 +53,10 @@
> > ; CHECK: fcsel s0, s1, s0, ne
> > ; CHECK-SAFE: fcsel s0, s1, s0, ne
> > }
> > +
> > +; Make sure the transformation isn't triggered for integers define
> > +i64 @test_integer(i64  %in) {
> > +  %cmp = icmp slt i64 %in, 0
> > +  %val = select i1 %cmp, i64 0, i64 %in
> > +  ret i64 %val
> > +}
> >
> > OK to re-commit with the fix?
> >
> >
> >