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

[James Molloy]

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

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782