[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