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

[Saleem Abdulrasool via llvm-commits]

compnerd added inline comments.

================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:74
@@ +73,3 @@
+        msr CPSR_f, #APSR_C
+        pop {r0, r1, r2, r3, pc}
+END_COMPILERRT_FUNCTION(__aeabi_cdcmple)
----------------
It seems you can factor out the setting of the CPSR, and do a single return.  Something along the lines of:

    __aeabi_cdcmple:
      push {r0-r3}
      blx __aeabi_dcmplt
      cmp r0, #1
      moveq ip, #0
      beq 1f
      ldm sp, {r0-r3}
      blx __aeabi_dcmpeq
      cmp r0, #1
      moveq ip, #(APSR_Z | APSR_C)
      beq 1f
      mov ip, #APSR_C
    1:msr CPSR_f, ip
      pop {r0-r3}
      bx lr


================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:29
@@ +28,3 @@
+        .p2align 2
+DEFINE_COMPILERRT_FUNCTION(__aeabi_cdcmpeq)
+        push {r0, r1, r2, r3, lr}
----------------
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.

================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:33
@@ +32,3 @@
+
+        // __aeabi_cdcmple preserves r0-r3, so we don't need to keep them
+        pop {r0, r1, r2, r3}
----------------
I think that this plus the highlighting in my copy of the RTABI document caused the confusion.  Saving behaviour of cdcmple is irrelevant.

================
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}
+
----------------
You hoisted it one instruction too early.  You just clobbered the result (r0).

================
Comment at: lib/builtins/arm/aeabi_cdcmp.S:44
@@ +43,3 @@
+        pop {lr}
+        b __aeabi_cdcmple
+END_COMPILERRT_FUNCTION(__aeabi_cdcmpeq)
----------------
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


http://reviews.llvm.org/D12089