[PATCH] arm_acle: Implement data processing intrinsics

Renato Golin renato.golin at linaro.org
Wed Aug 20 07:44:54 PDT 2014


Some more comments.

cheers,
--renato

================
Comment at: lib/Headers/arm_acle.h:102
@@ +101,3 @@
+  __ror(uint32_t x, uint32_t y) {
+  return (x >> y) | (x << 32 - y);
+}
----------------
Tim Northover wrote:
> This is undefined behaviour when y >= 32 or == 0.
ACLE says: "y can take any value", so we need some safeguards on all these shift functions.

In this case, something like:

    if (y >= 32) y %= 32;
    if (y == 0) return x;
    return (x >> y) | (x << 32 - y);

If that's the intended, but not explicit, semantics.

================
Comment at: lib/Headers/arm_acle.h:196
@@ +195,3 @@
+static __inline__ uint64_t __attribute__((always_inline, nodebug))
+  __rbitll(uint64_t t) {
+#if __ARM_32BIT_STATE
----------------
You could keep the order and put __rbitll after __rbitl.

================
Comment at: test/CodeGen/arm_acle.c:154
@@ +153,3 @@
+// ARM-NEXT: shl {{[i64,i32]}}
+// ARM-NEXT: or {{[i64,i32]}}
+uint32_t test_cls(uint32_t t) {
----------------
Doesn't this call CLZ as well?

http://reviews.llvm.org/D4983






More information about the llvm-commits mailing list