[PATCH] D26544: [PPC] support for arithmetic builtins in the FE

Nemanja Ivanovic via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 13 18:49:17 PST 2016


nemanjai added inline comments.


================
Comment at: lib/Headers/altivec.h:314
+  vector signed int __carry = __c & __mask;
+  return vec_add(vec_add(__a, __b), __mask);
+}
----------------
I don't understand why we're adding `__mask` to the sum of `__a` and `__b`. Shouldn't that be `__carry`?


================
Comment at: lib/Headers/altivec.h:322
+  vector unsigned int __carry = __c & __mask;
+  return vec_add(vec_add(__a, __b), __mask);
+}
----------------
Same comment as above.


================
Comment at: lib/Headers/altivec.h:349
+    unsigned int __tempc = (unsigned int) __c[i];
+    __tempc = __tempc & 0x00000001;
+    unsigned long long __longa = (unsigned long long) __tempa;
----------------
Is it not a little cleaner and more readable to just mask out the `__c` parameter before the loop (similarly to the masking you've done with `__mask` above)?


================
Comment at: lib/Headers/altivec.h:350
+    __tempc = __tempc & 0x00000001;
+    unsigned long long __longa = (unsigned long long) __tempa;
+    unsigned long long __longb = (unsigned long long) __tempb;
----------------
I think it's a little more clear and obvious what is happening if you actually have just a single cast and mask - i.e.
``` unsigned long long __longa = ((unsigned long long) __a[i]) & 0xFFFFFFFF;```


https://reviews.llvm.org/D26544





More information about the cfe-commits mailing list