kbarton added a comment.
There are several inline comments that need to be addressed.
I also think it's worthwhile putting a comment at the top of the review
indicating the (assumed) semantics for the vec_sube and vec_subec instructions
that are being implemented (i.e., the behaviour mimics the vec_subeuqm
instructions and thus uses one's compliment plus the carry)
================
Comment at: lib/Headers/altivec.h:306
}
+
#endif
----------------
please remove blank line
================
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;
----------------
nemanjai wrote:
> 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;```
Is a mask actually needed? This seems to be what is done in the vec_addec
function below, without the cast. I agree that is cleaner.
The other minor nit is to pick a single value for the mask (1, 0x01,
0x00000001) and use it consistently.
================
Comment at: lib/Headers/altivec.h:10516
+ vector signed __int128 __c) {
+ return __builtin_altivec_vsubecuq(__a, __b, __c);
+}
----------------
Why do we mask the carries for sign/unsigned ints, but not __128 ints?
================
Comment at: lib/Headers/altivec.h:10524
+}
+
+
----------------
Please reorder these to put the __128 below signed and unsigned.
================
Comment at: lib/Headers/altivec.h:10545
+ vector signed int __carry = __c & __mask;
+ return vec_add(vec_add(__a, ~__b), __carry);
+}
----------------
Is it possible to use vec_adde(__a, ~__b, __carry)?
https://reviews.llvm.org/D26544
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
