[PATCH] D12089: Implement __aeabi_c{d,f}{cmpeq,cmple,rcmple}.

Josh Gao via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 18 21:38:04 PDT 2015


jmgao marked 2 inline comments as done.

================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:29
@@ +28,3 @@
+        .p2align 2
+DEFINE_COMPILERRT_FUNCTION(__aeabi_cdcmpeq)
+        push {r0, r1, r2, r3, lr}
----------------
compnerd wrote:
> Yeah, after a second (or third) reading, I agree.  This function is exempt from that restriction.  Im not sure what the benefit of saving lr here is, as you can just as well as bx lr in one case, and just tail call as you do in the other case.
The call to the check_nan helper trashes lr

================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:34
@@ +33,3 @@
+        // __aeabi_cdcmple preserves r0-r3, so we don't need to keep them
+        pop {r0, r1, r2, r3}
+
----------------
compnerd wrote:
> You hoisted it one instruction too early.  You just clobbered the result (r0).
Whoops. I'm not sure how I missed that. I guess the tests passed because it's not changing the behavior if the implementation doesn't trap on NaN and you don't pass in a float with a value of 1 in the first word.

================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:44
@@ +43,3 @@
+        pop {lr}
+        b __aeabi_cdcmple
+END_COMPILERRT_FUNCTION(__aeabi_cdcmpeq)
----------------
compnerd wrote:
> Wouldn't this be shorter as:
> 
>     __aeabi_cdcmpeq:
>         push {r0-r3}
>         blx __aeabi_cdcmpeq_check_nan
>         cmp r0, #1
>         pop {r0-r3}
>         beq __aeabi_cdcmple
>         msr CPSR_f, #APSR_C
>         bx lr
Yes it would, except {r0-r3, lr} for the push/pop since the call to __aeabi_cdcmpeq_check_nan will trash lr.

================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:75
@@ +74,3 @@
+        // Restore the argument registers
+        pop {r0, r1, r2, r3}
+
----------------
How about this?

```
__aeabi_cdcmple:
    push {r0-r4, lr}

    bl __aeabi_dcmplt
    cmp r0, #1
    moveq r4, #0
    beq 1f

    ldm sp, {r0-r3}
    bl __aeabi_dcmpeq
    cmp r0, #1
    moveq r4, #(APSR_C | APSR_Z)
    movne r4, #(APSR_C)
1:
    msr CPSR_f, r4
    pop {r0-r4, pc} 
```


http://reviews.llvm.org/D12089





More information about the llvm-commits mailing list