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

[Ehsan Amiri via cfe-commits]

amehsan added inline comments.


================
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;
----------------
kbarton wrote:
> 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. 
To come up with this code pattern I looked at the following pieces of codes:


```
unsigned long long f (int t) {
  return (unsigned long long)  t;
}
```
When compiled with optimization  produces 

```
define i64 @f(i32 signext %t) local_unnamed_addr #0 {
entry:
  %conv = sext i32 %t to i64
  ret i64 %conv
}

```
Which is incorrect. Also 


```
unsigned long long f (int t) {
  return (unsigned long long)(unsigned)  t;
}
~

```

results in 

```
define i64 @f(i32 signext %t) local_unnamed_addr #0 {
entry:
  %conv = zext i32 %t to i64
  ret i64 %conv
}

```

and 


```
.Lfunc_begin0:
# BB#0:                                 # %entry
        clrldi   3, 3, 32
        blr

```

So I think the code here is optimal and correct and there is no need to change it.


================
Comment at: lib/Headers/altivec.h:10516
+             vector signed __int128 __c) {
+  return __builtin_altivec_vsubecuq(__a, __b, __c);
+}
----------------
kbarton wrote:
> Why do we mask the carries for sign/unsigned ints, but not __128 ints?
for quadword, hardware does the masking (implicitly, by only looking at the rightmost bit)


https://reviews.llvm.org/D26544