[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision.
amyk added a comment.

This patch also LGTM. Thank you Lei.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.




Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat.c:8
 // RUN:   -D__XL_COMPAT_ALTIVEC__ -target-cpu pwr8 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers -target-feature +altivec 
-target-feature +vsx \
+// RUN: %clang_cc1 -flax-vector-conversions=none -no-opaque-pointers 
-target-feature +altivec -target-feature +vsx \
 // RUN:   -triple powerpc64le-unknown-linux-gnu -emit-llvm %s -o - \

lei wrote:
> nemanjai wrote:
> > I don't know why only one of the functions below has checks for this run 
> > line, but that needs to be fixed. Please add the `NOCOMPAT` checks on the 
> > other functions.
> Can we address this on a followup patch? Since it's not related to the work 
> here.
Absolutely.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Lei Huang via Phabricator via cfe-commits]

lei marked an inline comment as not done.
lei added inline comments.



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat.c:8
 // RUN:   -D__XL_COMPAT_ALTIVEC__ -target-cpu pwr8 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers -target-feature +altivec 
-target-feature +vsx \
+// RUN: %clang_cc1 -flax-vector-conversions=none -no-opaque-pointers 
-target-feature +altivec -target-feature +vsx \
 // RUN:   -triple powerpc64le-unknown-linux-gnu -emit-llvm %s -o - \

nemanjai wrote:
> I don't know why only one of the functions below has checks for this run 
> line, but that needs to be fixed. Please add the `NOCOMPAT` checks on the 
> other functions.
Can we address this on a followup patch? Since it's not related to the work 
here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15587
+
+bool isUnaligned = (BuiltinID == PPC::BI__builtin_altivec_vinsw ||
+BuiltinID == PPC::BI__builtin_altivec_vinsd)

This is strange. You don't need the ternary operator when you are simply 
setting a `bool` variable. This is simply:
```
bool isUnaligned = (BuiltinID == PPC::BI__builtin_altivec_vinsw ||
BuiltinID == PPC::BI__builtin_altivec_vinsd);
```
Same below.

Also, please be careful of the naming convention (i.e. variables start with 
upper case, functions with lowercase, etc.). I will not add further comments to 
this end but please apply this change to the entire patch.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15604
+// builtin called.
+int validMaxValue = isUnaligned ? is32bit ? 12 : 8 : is32bit ? 3 : 1;
+

This is certainly concise, but trying to parse it puts my brain into an 
infinite loop. Please write it as:
```
int ValidMaxValue = 0;
if (IsUnaligned)
  ValidMaxValue = ... ? ... : ...
else
  ValidMaxValue = ...
```



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15608
+std::string rangeErrMsg = isUnaligned ? "byte" : "element";
+rangeErrMsg += " number is outside of the valid range [0, ";
+rangeErrMsg += llvm::to_string(validMaxValue) + "]";

Since you are building a string from pieces here, might as well mention the 
value the user specified so it is something like:
`byte number 44 is outside of the valid range [0, 12]`



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15661
+Value *Op1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *Int128Ty = llvm::IntegerType::get(getLLVMContext(), 128);
+Ops.push_back(

Why not simplify the bitcasts below by just creating a pointer to the vector 
type (call it say, `V1I128Ty`) here? Same in the next `case`.



Comment at: clang/lib/Headers/altivec.h:3211-3219
+   : (vector double)(__builtin_vsx_xvcvuxdsp(  
\
+ (vector unsigned long long)(__a)) *   
\
+ (vector float)(vector unsigned)((0x7f - (__b))
\
+ << 23)),  
\
  vector signed long long   
\
-   : (__builtin_vsx_xvcvsxdsp((vector signed long long)(__a)) *
\
-  (vector float)(vector unsigned)((0x7f - (__b)) << 23)))
+   : (vector double)(__builtin_vsx_xvcvsxdsp(  
\
+ (vector signed long long)(__a)) * 
\

This is wrong. Please see the documentation for what the result type should be 
under XL compat. I believe it needs to be `vector float` regardless of the 
input.



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat.c:8
 // RUN:   -D__XL_COMPAT_ALTIVEC__ -target-cpu pwr8 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers -target-feature +altivec 
-target-feature +vsx \
+// RUN: %clang_cc1 -flax-vector-conversions=none -no-opaque-pointers 
-target-feature +altivec -target-feature +vsx \
 // RUN:   -triple powerpc64le-unknown-linux-gnu -emit-llvm %s -o - \

I don't know why only one of the functions below has checks for this run line, 
but that needs to be fixed. Please add the `NOCOMPAT` checks on the other 
functions.



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat.c:30
 // CHECK-NEXT:[[TMP1:%.*]] = call <4 x float> @llvm.ppc.vsx.xvcvsxdsp(<2 x 
i64> [[TMP0]])
 // CHECK-NEXT:fmul <4 x float> [[TMP1]], 
 

We compute a `vector float` but return a `vector double`. Something is wrong. 
Please see my comment regarding this in `altivec.h`.



Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:746
 [IntrNoMem, ImmArg>]>;
+
   // P10 Vector Extract.

What happened here? Unrelated whitespace change. Please remove it before 
committing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo added a comment.

In D124093#3528639 , @amyk wrote:

> I was wondering where are the test cases in this patch. Did they get missed 
> when updating the revision?

The test case's modification were not needed as part of addressing Nemanja's 
comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

I was wondering where are the test cases in this patch. Did they get missed 
when updating the revision?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo updated this revision to Diff 425086.
maryammo added a comment.

[PowerPC] Address the review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/altivec.h

Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -311,7 +311,7 @@
 
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_add_u128(vector unsigned char __a, vector unsigned char __b) {
-  return __builtin_altivec_vadduqm(__a, __b);
+  return (vector unsigned char)__builtin_altivec_vadduqm(__a, __b);
 }
 #elif defined(__VSX__)
 static __inline__ vector signed long long __ATTRS_o_ai
@@ -325,9 +325,9 @@
   (vector unsigned int)__a + (vector unsigned int)__b;
   vector unsigned int __carry = __builtin_altivec_vaddcuw(
   (vector unsigned int)__a, (vector unsigned int)__b);
-  __carry = __builtin_shufflevector((vector unsigned char)__carry,
-(vector unsigned char)__carry, 0, 0, 0, 7,
-0, 0, 0, 0, 0, 0, 0, 15, 0, 0, 0, 0);
+  __carry = (vector unsigned int)__builtin_shufflevector(
+  (vector unsigned char)__carry, (vector unsigned char)__carry, 0, 0, 0, 7,
+  0, 0, 0, 0, 0, 0, 0, 15, 0, 0, 0, 0);
   return (vector signed long long)(__res + __carry);
 #endif
 }
@@ -358,7 +358,9 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_adde(vector signed __int128 __a, vector signed __int128 __b,
  vector signed __int128 __c) {
-  return __builtin_altivec_vaddeuqm(__a, __b, __c);
+  return (vector signed __int128)__builtin_altivec_vaddeuqm(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b,
+  (vector unsigned __int128)__c);
 }
 
 static __inline__ vector unsigned __int128 __ATTRS_o_ai
@@ -371,7 +373,9 @@
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_adde_u128(vector unsigned char __a, vector unsigned char __b,
   vector unsigned char __c) {
-  return (vector unsigned char)__builtin_altivec_vaddeuqm(__a, __b, __c);
+  return (vector unsigned char)__builtin_altivec_vaddeuqm_c(
+  (vector unsigned char)__a, (vector unsigned char)__b,
+  (vector unsigned char)__c);
 }
 #endif
 
@@ -398,7 +402,9 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_addec(vector signed __int128 __a, vector signed __int128 __b,
   vector signed __int128 __c) {
-  return __builtin_altivec_vaddecuq(__a, __b, __c);
+  return (vector signed __int128)__builtin_altivec_vaddecuq(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b,
+  (vector unsigned __int128)__c);
 }
 
 static __inline__ vector unsigned __int128 __ATTRS_o_ai
@@ -411,7 +417,9 @@
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_addec_u128(vector unsigned char __a, vector unsigned char __b,
vector unsigned char __c) {
-  return (vector unsigned char)__builtin_altivec_vaddecuq(__a, __b, __c);
+  return (vector unsigned char)__builtin_altivec_vaddecuq_c(
+  (vector unsigned char)__a, (vector unsigned char)__b,
+  (vector unsigned char)__c);
 }
 
 #ifdef __powerpc64__
@@ -600,7 +608,8 @@
 
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_addc_u128(vector unsigned char __a, vector unsigned char __b) {
-  return (vector unsigned char)__builtin_altivec_vaddcuq(__a, __b);
+  return (vector unsigned char)__builtin_altivec_vaddcuq_c(
+  (vector unsigned char)__a, (vector unsigned char)__b);
 }
 #endif // defined(__POWER8_VECTOR__) && defined(__powerpc64__)
 
@@ -824,7 +833,9 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_vaddeuqm(vector signed __int128 __a, vector signed __int128 __b,
  vector signed __int128 __c) {
-  return __builtin_altivec_vaddeuqm(__a, __b, __c);
+  return (vector signed __int128)__builtin_altivec_vaddeuqm(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b,
+  (vector unsigned __int128)__c);
 }
 
 static __inline__ vector unsigned __int128 __ATTRS_o_ai
@@ -837,7 +848,8 @@
 
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_vaddcuq(vector signed __int128 __a, vector signed __int128 __b) {
-  return __builtin_altivec_vaddcuq(__a, __b);
+  return (vector signed __int128)__builtin_altivec_vaddcuq(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b);
 }
 
 static __inline__ vector unsigned __int128 __ATTRS_o_ai
@@ -850,7 +862,9 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_vaddecuq(vector signed __int128 __a, vector signed __int128 __b,
  vector signed __int128 __c) {
-  return __builtin_altivec_vaddecuq(__a, __b, __c);
+  return (vector signed 

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.

Also, please run `clang-format` on the changes.




Comment at: clang/lib/Headers/altivec.h:19051
 #ifdef __LITTLE_ENDIAN__
-  return __builtin_altivec_vstribl_p(__CR6_EQ, (vector signed char)__a);
+  return __builtin_altivec_vstribl_p(__CR6_EQ, (vector char)__a);
 #else

We should never cast anything to an integral vector type that doesn't include 
`signed/unsigned/bool`.



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c:1955
   // CHECK: sub <1 x i128>
-  // CHECK-NEXT: lshr <1 x i128>
+  // CHECK-NEXT: ashr <1 x i128>
   // CHECK-NEXT: or <1 x i128>

This is not good. We are changing semantics here - turning a logical shift into 
an arithmetic shift.



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c:10
 // RUN: %clang_cc1 -O2 -target-feature +altivec -target-feature +power8-vector 
\
-// RUN:   -triple powerpc-aix-unknown -emit-llvm %s -o - | FileCheck \
+// RUN:   -triple powerpc-aix-unknown -emit-llvm -fforce-enable-int128 %s -o - 
| FileCheck \
 // RUN:   %s -check-prefix=CHECK-AIX

Why is this addition needed? Seems like something is wrong if we need to force 
enable int128 in order to compile something with `altivec.h`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo updated this revision to Diff 424050.
maryammo added a comment.

[NFC] Fix the formatting


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124093/new/

https://reviews.llvm.org/D124093

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
  clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c

Index: clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c
@@ -7,7 +7,7 @@
 // RUN:   -triple powerpc64-aix-unknown -emit-llvm %s -o - | FileCheck \
 // RUN:   %s -check-prefix=CHECK-AIX
 // RUN: %clang_cc1 -O2 -target-feature +altivec -target-feature +power8-vector \
-// RUN:   -triple powerpc-aix-unknown -emit-llvm %s -o - | FileCheck \
+// RUN:   -triple powerpc-aix-unknown -emit-llvm -fforce-enable-int128 %s -o - | FileCheck \
 // RUN:   %s -check-prefix=CHECK-AIX
 #include 
 // CHECK-LE-LABEL: @test_subc(
Index: clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
@@ -1952,7 +1952,7 @@
 vector signed __int128 test_vec_rl_s128(void) {
   // CHECK-LABEL: @test_vec_rl_s128(
   // CHECK: sub <1 x i128>
-  // CHECK-NEXT: lshr <1 x i128>
+  // CHECK-NEXT: ashr <1 x i128>
   // CHECK-NEXT: or <1 x i128>
   // CHECK-NEXT: ret <1 x i128>
   return vec_rl(vsi128a, vsi128b);
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -311,7 +311,7 @@
 
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_add_u128(vector unsigned char __a, vector unsigned char __b) {
-  return __builtin_altivec_vadduqm(__a, __b);
+  return (vector unsigned char)__builtin_altivec_vadduqm(__a, __b);
 }
 #elif defined(__VSX__)
 static __inline__ vector signed long long __ATTRS_o_ai
@@ -325,9 +325,9 @@
   (vector unsigned int)__a + (vector unsigned int)__b;
   vector unsigned int __carry = __builtin_altivec_vaddcuw(
   (vector unsigned int)__a, (vector unsigned int)__b);
-  __carry = __builtin_shufflevector((vector unsigned char)__carry,
-(vector unsigned char)__carry, 0, 0, 0, 7,
-0, 0, 0, 0, 0, 0, 0, 15, 0, 0, 0, 0);
+  __carry = (vector unsigned int)__builtin_shufflevector(
+  (vector unsigned char)__carry, (vector unsigned char)__carry, 0, 0, 0, 7,
+  0, 0, 0, 0, 0, 0, 0, 15, 0, 0, 0, 0);
   return (vector signed long long)(__res + __carry);
 #endif
 }
@@ -358,7 +358,9 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_adde(vector signed __int128 __a, vector signed __int128 __b,
  vector signed __int128 __c) {
-  return __builtin_altivec_vaddeuqm(__a, __b, __c);
+  return (vector signed __int128)__builtin_altivec_vaddeuqm(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b,
+  (vector unsigned __int128)__c);
 }
 
 static __inline__ vector unsigned __int128 __ATTRS_o_ai
@@ -366,14 +368,16 @@
  vector unsigned __int128 __c) {
   return __builtin_altivec_vaddeuqm(__a, __b, __c);
 }
-#endif
 
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_adde_u128(vector unsigned char __a, vector unsigned char __b,
   vector unsigned char __c) {
-  return (vector unsigned char)__builtin_altivec_vaddeuqm(__a, __b, __c);
+  return (vector unsigned char)__builtin_altivec_vaddeuqm(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b,
+  (vector unsigned __int128)__c);
 }
 #endif
+#endif
 
 static __inline__ vector signed int __ATTRS_o_ai
 vec_adde(vector signed int __a, vector signed int __b,
@@ -398,7 +402,9 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_addec(vector signed __int128 __a, vector signed __int128 __b,
   vector signed __int128 __c) {
-  return __builtin_altivec_vaddecuq(__a, __b, __c);
+  return (vector signed __int128)__builtin_altivec_vaddecuq(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b,
+  (vector unsigned __int128)__c);
 }
 
 static __inline__ vector unsigned __int128 __ATTRS_o_ai
@@ -406,13 +412,15 @@
   vector unsigned __int128 __c) {
   return __builtin_altivec_vaddecuq(__a, __b, __c);
 }
-#endif
 
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_addec_u128(vector unsigned char __a, vector unsigned char __b,
vector unsigned char __c) {
-  return (vector unsigned char)__builtin_altivec_vaddecuq(__a, __b, __c);
+  return (vector unsigned char)__builtin_altivec_vaddecuq(
+  (vector unsigned __int128)__a, (vector unsigned __int128)__b,
+ 

[PATCH] D124093: [PowerPC] Fixing implicit castings in altivec for -fno-lax-vector-conversions

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo created this revision.
Herald added subscribers: shchenz, kbarton, nemanjai.
Herald added a project: All.
maryammo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

XL considers different vector types to be incompatible with each other.
For example assignment between variables of types vector float and vector
long long or even vector signed int and vector unsigned int are diagnosed.
clang, however does not diagnose such cases and does a simple bitcast between
the two types. This could easily result in program errors. This patch is to
fix the implicit casts in altivec.h so that there is no incompatible vector
type errors whit -fno-lax-vector-conversions, this is the prerequisite patch
to switch the default to -fno-lax-vector-conversions later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124093

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
  clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c

Index: clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-quadword-noi128.c
@@ -7,7 +7,7 @@
 // RUN:   -triple powerpc64-aix-unknown -emit-llvm %s -o - | FileCheck \
 // RUN:   %s -check-prefix=CHECK-AIX
 // RUN: %clang_cc1 -O2 -target-feature +altivec -target-feature +power8-vector \
-// RUN:   -triple powerpc-aix-unknown -emit-llvm %s -o - | FileCheck \
+// RUN:   -triple powerpc-aix-unknown -emit-llvm -fforce-enable-int128 %s -o - | FileCheck \
 // RUN:   %s -check-prefix=CHECK-AIX
 #include 
 // CHECK-LE-LABEL: @test_subc(
Index: clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
@@ -1952,7 +1952,7 @@
 vector signed __int128 test_vec_rl_s128(void) {
   // CHECK-LABEL: @test_vec_rl_s128(
   // CHECK: sub <1 x i128>
-  // CHECK-NEXT: lshr <1 x i128>
+  // CHECK-NEXT: ashr <1 x i128>
   // CHECK-NEXT: or <1 x i128>
   // CHECK-NEXT: ret <1 x i128>
   return vec_rl(vsi128a, vsi128b);
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -311,7 +311,7 @@
 
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_add_u128(vector unsigned char __a, vector unsigned char __b) {
-  return __builtin_altivec_vadduqm(__a, __b);
+  return (vector unsigned char)__builtin_altivec_vadduqm(__a, __b);
 }
 #elif defined(__VSX__)
 static __inline__ vector signed long long __ATTRS_o_ai
@@ -325,9 +325,10 @@
   (vector unsigned int)__a + (vector unsigned int)__b;
   vector unsigned int __carry = __builtin_altivec_vaddcuw(
   (vector unsigned int)__a, (vector unsigned int)__b);
-  __carry = __builtin_shufflevector((vector unsigned char)__carry,
-(vector unsigned char)__carry, 0, 0, 0, 7,
-0, 0, 0, 0, 0, 0, 0, 15, 0, 0, 0, 0);
+  __carry = (vector unsigned int)__builtin_shufflevector(
+		 (vector unsigned char)__carry,
+ (vector unsigned char)__carry, 0, 0, 0, 7,
+ 0, 0, 0, 0, 0, 0, 0, 15, 0, 0, 0, 0);
   return (vector signed long long)(__res + __carry);
 #endif
 }
@@ -358,7 +359,10 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_adde(vector signed __int128 __a, vector signed __int128 __b,
  vector signed __int128 __c) {
-  return __builtin_altivec_vaddeuqm(__a, __b, __c);
+  return (vector signed __int128)__builtin_altivec_vaddeuqm(
+(vector unsigned __int128)__a,
+		(vector unsigned __int128)__b,
+			(vector unsigned __int128)__c);
 }
 
 static __inline__ vector unsigned __int128 __ATTRS_o_ai
@@ -366,14 +370,17 @@
  vector unsigned __int128 __c) {
   return __builtin_altivec_vaddeuqm(__a, __b, __c);
 }
-#endif
 
 static __inline__ vector unsigned char __attribute__((__always_inline__))
 vec_adde_u128(vector unsigned char __a, vector unsigned char __b,
   vector unsigned char __c) {
-  return (vector unsigned char)__builtin_altivec_vaddeuqm(__a, __b, __c);
+  return (vector unsigned char)__builtin_altivec_vaddeuqm(
+		   (vector unsigned __int128)__a,
+		   (vector unsigned __int128)__b,
+	   (vector unsigned __int128)__c);
 }
 #endif
+#endif
 
 static __inline__ vector signed int __ATTRS_o_ai
 vec_adde(vector signed int __a, vector signed int __b,
@@ -398,7 +405,10 @@
 static __inline__ vector signed __int128 __ATTRS_o_ai
 vec_addec(vector signed __int128 __a, vector signed __int128 __b,