[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