[PATCH] D28016: [Builtins] [ARM] Adding Thumb1 support for fcmp

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Hi Weiming,

I don't think you can use `MSR/MRS` like that. They're not available anywhere Thumb1 is available (AFAIK, only `M0`), so this will **only** work on `M0`, not on anything else where `__ARM_ARCH_ISA_THUMB == 1` will be true.

Also, `MRS/MSR` is an additional 4 cycles each, and by the number of uses you have here, this function will be several times slower than it should.

cheers,
--renato



================
Comment at: lib/builtins/arm/comparesf2.S:87
+#if __ARM_ARCH_ISA_THUMB == 1
+    bmi     LOCAL_LABEL(eq_identical)
+    subs    r0,     r2, r3
----------------
Why do you use named labels here and not below? I'm fine with either standard, would just be good to be consistent, I think.

My personal preference is to use `1f` for everything, as it's clear what you want from the local context and the notation is slightly less cluttered.


https://reviews.llvm.org/D28016