[PATCH] D39376: [PowerPC] Add implementation for -msave-toc-indirect option - clang portion
jtony added inline comments. Comment at: test/Driver/ppc-features.cpp:140 + // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-htm -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOHTM %s // CHECK-NOHTM: "-target-feature" "-htm" We probably need to add test for mno_save_toc_indirect also once you add it according to Hal's suggestion. https://reviews.llvm.org/D39376 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33053: [PowerPC] Implement vec_xxpermdi builtin.
jtony updated this revision to Diff 99292.
jtony added a comment.
Address all the comments from Nemanja.
https://reviews.llvm.org/D33053
Files:
include/clang/Basic/BuiltinsPPC.def
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/CodeGen/CGBuiltin.cpp
lib/Headers/altivec.h
lib/Sema/SemaChecking.cpp
test/CodeGen/builtins-ppc-error.c
test/CodeGen/builtins-ppc-vsx.c
Index: test/CodeGen/builtins-ppc-vsx.c
===
--- test/CodeGen/builtins-ppc-vsx.c
+++ test/CodeGen/builtins-ppc-vsx.c
@@ -1691,4 +1691,60 @@
res_vd = vec_neg(vd);
// CHECK: fsub <2 x double> , {{%[0-9]+}}
// CHECK-LE: fsub <2 x double> , {{%[0-9]+}}
+
+res_vd = vec_xxpermdi(vd, vd, 0);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vf = vec_xxpermdi(vf, vf, 1);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vsll = vec_xxpermdi(vsll, vsll, 2);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vull = vec_xxpermdi(vull, vull, 3);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vsi = vec_xxpermdi(vsi, vsi, 0);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vui = vec_xxpermdi(vui, vui, 1);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vss = vec_xxpermdi(vss, vss, 2);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vus = vec_xxpermdi(vus, vus, 3);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vsc = vec_xxpermdi(vsc, vsc, 0);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vuc = vec_xxpermdi(vuc, vuc, 1);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+}
+
+// The return type of the call expression may be different from the return type of the shufflevector.
+// Wrong implementation could crash the compiler, add this test case to check that and avoid ICE.
+vector int should_not_assert(vector int a, vector int b) {
+ return vec_xxpermdi(a, b, 0);
+// CHECK-LABEL: should_not_assert
+// CHECK: bitcast <4 x i32> %{{[0-9]+}} to <2 x i64>
+// CHECK-NEXT: bitcast <4 x i32> %{{[0-9]+}} to <2 x i64>
+// CHECK-NEXT: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-NEXT: bitcast <2 x i64> %{{[0-9]+}} to <4 x i32>
+
+// CHECK-LE: bitcast <4 x i32> %{{[0-9]+}} to <2 x i64>
+// CHECK-LE-NEXT: bitcast <4 x i32> %{{[0-9]+}} to <2 x i64>
+// CHECK-LE-NEXT: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE-NEXT: bitcast <2 x i64> %{{[0-9]+}} to <4 x i32>
}
Index: test/CodeGen/builtins-ppc-error.c
===
--- test/CodeGen/builtins-ppc-error.c
+++ test/CodeGen/builtins-ppc-error.c
@@ -13,8 +13,16 @@
extern vector signed int vsi;
extern vector unsigned char vuc;
-void testInsertWord1(void) {
+void testInsertWord(void) {
int index = 5;
vector unsigned char v1 = vec_insert4b(vsi, vuc, index); // expected-error {{argument to '__builtin_vsx_insertword' must be a constant integer}}
vector unsigned long long v2 = vec_extract4b(vuc, index); // expected-error {{argument to '__builtin_vsx_extractuword' must be a constant integer}}
}
+
+void testXXPERMDI(void) {
+ int index = 5;
+ vec_xxpermdi(vsi); //expected-error {{too few arguments to function call, expected at least 3, have 1}}
+ vec_xxpermdi(vsi, vsi, 2, 4); //expected-error {{too many arguments to function call, expected at most 3, have 4}}
+ vec_xxpermdi(vsi, vsi, index); //expected-error {{argument 3 to '__builtin_vsx_xxpermdi' must be a 2-bit unsigned literal (i.e. 0,1,2 or 3)}}
+ vec_xxpermdi(vsi, vuc, 2); //expected-error {{first two arguments to '__builtin_vsx_xxpermdi' must have the same type}}
+}
Index: lib/Sema/SemaChecking.cpp
===
---
[PATCH] D33053: [PowerPC] Implement vec_xxpermdi builtin.
jtony marked 6 inline comments as done.
jtony added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8433
+if (getTarget().isLittleEndian()) {
+ switch (Index) {
+ case 0:
nemanjai wrote:
> The switch is overkill. You should just implement this in an obvious way
> (i.e. the same way as described in the ISA).
> For big endian:
> `ElemIdx0 = (Index & 2;) >> 1`
> `ElemIdx1 = 2 + (Index & 1)`
>
> For little endian:
> `ElemIdx0 = (~Index & 1) + 2;`
> `ElemIdx1 = ~Index & 2 >> 1;`
>
> (of course, please verify the expressions).
Good call, fixed as suggested.
https://reviews.llvm.org/D33053
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33053: [PowerPC] Implement vec_xxpermdi builtin.
jtony created this revision.
The vec_xxpermdi builtin is missing from altivec.h. This has been requested by
developers working on libvpx for VP9 support for Google. Initially, I tried to
define a new intrinsic to map it to the corresponding PowerPC hard instruction
(XXPERMDI) directly. But there was feedback from the community that this can be
done without introducing new intrinsic. This patch re-implement the
vec_xxpermdi builtin by using the existing shuffleVector instruction just in
the FE.
https://reviews.llvm.org/D33053
Files:
include/clang/Basic/BuiltinsPPC.def
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/CodeGen/CGBuiltin.cpp
lib/Headers/altivec.h
lib/Sema/SemaChecking.cpp
test/CodeGen/builtins-ppc-error.c
test/CodeGen/builtins-ppc-vsx.c
Index: test/CodeGen/builtins-ppc-vsx.c
===
--- test/CodeGen/builtins-ppc-vsx.c
+++ test/CodeGen/builtins-ppc-vsx.c
@@ -1691,4 +1691,44 @@
res_vd = vec_neg(vd);
// CHECK: fsub <2 x double> , {{%[0-9]+}}
// CHECK-LE: fsub <2 x double> , {{%[0-9]+}}
+
+res_vd = vec_xxpermdi(vd, vd, 0);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vf = vec_xxpermdi(vf, vf, 1);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vsll = vec_xxpermdi(vsll, vsll, 2);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vull = vec_xxpermdi(vull, vull, 3);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vsi = vec_xxpermdi(vsi, vsi, 0);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vui = vec_xxpermdi(vui, vui, 1);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vss = vec_xxpermdi(vss, vss, 2);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vus = vec_xxpermdi(vus, vus, 3);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vsc = vec_xxpermdi(vsc, vsc, 0);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+
+res_vuc = vec_xxpermdi(vuc, vuc, 1);
+// CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
+// CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32>
}
Index: test/CodeGen/builtins-ppc-error.c
===
--- test/CodeGen/builtins-ppc-error.c
+++ test/CodeGen/builtins-ppc-error.c
@@ -13,8 +13,17 @@
extern vector signed int vsi;
extern vector unsigned char vuc;
-void testInsertWord1(void) {
+void testInsertWord(void) {
int index = 5;
vector unsigned char v1 = vec_insert4b(vsi, vuc, index); // expected-error {{argument to '__builtin_vsx_insertword' must be a constant integer}}
vector unsigned long long v2 = vec_extract4b(vuc, index); // expected-error {{argument to '__builtin_vsx_extractuword' must be a constant integer}}
}
+
+void testXXPERMDI(void) {
+ int index = 5;
+ vec_xxpermdi(vsi); //expected-error {{too few arguments to function call, expected at least 3, have 1}}
+ vec_xxpermdi(vsi, vsi, 2, 4); //expected-error {{too many arguments to function call, expected at most 3, have 4}}
+ vec_xxpermdi(vsi, vsi, index); //expected-error {{third argument to '__builtin_vsx_xxpermdi' must be a constant integer between 0-3}}
+ vec_xxpermdi(vsi, vsi, -1); //expected-error {{third argument to '__builtin_vsx_xxpermdi' must be a constant integer between 0-3}}
+ vec_xxpermdi(vsi, vuc, 2); //expected-error {{first two arguments to '__builtin_vsx_xxpermdi' must be the same type}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1696,6 +1696,8 @@
case PPC::BI__builtin_tabortdci:
return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+ case PPC::BI__builtin_vsx_xxpermdi:
+return SemaBuiltinVSX(TheCall, 3);
}
return SemaBuiltinConstantArgRange(TheCall, i, l,
[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)
jtony added inline comments.
Comment at: llvm/include/llvm/ADT/APFloat.h:800
- void makeLargest(bool Neg) { getIEEE().makeLargest(Neg); }
+ void makeLargest(bool Neg) {
+if (usesLayout(getSemantics())) {
I know it is allowed to return a void function call inside a void function, but
I think this reduces the code readability in general and causes some confusing
to some people. Maybe it is better to avoid using this kind of coding style. I
think we can simply call each function in each branch without the 'return'
keyword, by default, the program will reach the end of function and return.
One possible equivalent code:
void makeNaN(bool SNaN, bool Neg, const APInt *fill) {
if (usesLayout(getSemantics())) {
U.IEEE.makeNaN(SNaN, Neg, fill);
} else if (usesLayout(getSemantics())) {
U.Double.makeNaN(SNaN, Neg, fill);
} else {
llvm_unreachable("Unexpected semantics");
}
}
Comment at: llvm/include/llvm/ADT/APFloat.h:811
+ void makeSmallest(bool Neg) {
+if (usesLayout(getSemantics())) {
+ return U.IEEE.makeSmallest(Neg);
Same here.
Comment at: llvm/include/llvm/ADT/APFloat.h:821
void makeSmallestNormalized(bool Neg) {
-getIEEE().makeSmallestNormalized(Neg);
+if (usesLayout(getSemantics())) {
+ return U.IEEE.makeSmallestNormalized(Neg);
Same here.
Comment at: llvm/include/llvm/ADT/APFloat.h:1100
- void changeSign() { getIEEE().changeSign(); }
- void clearSign() { getIEEE().clearSign(); }
- void copySign(const APFloat ) { getIEEE().copySign(RHS.getIEEE()); }
+ void changeSign() {
+if (usesLayout(getSemantics())) {
Same here.
https://reviews.llvm.org/D27872
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27251: [PPC] some bugs mainly about sign problem fixed in altivec.h
jtony added inline comments.
Comment at: lib/Headers/altivec.h:16456
#ifdef __VSX__
static __inline__ vector signed long long __ATTRS_o_ai
Thanks a lot for your good catch for the macro issue in vec_xst_be, that's a
good catch. BTW, Can you move this up also like vec_xst_be?
Comment at: test/CodeGen/builtins-ppc-vsx.c:1696
+
+signed char param_sc;
+unsigned char param_uc;
I would prefer these definitions occur at the beginning of the file like
before.
Comment at: test/CodeGen/builtins-ppc-vsx.c:1706
+/* - vec_xl_be --
*/
+void test2() {
+ // CHECK-LABEL: define void @test2
These test cases should be grouped together with the test cases from 1663 -
1683. Put the vec_xl_be overloads together, and the vec_xst_be together (maybe
after vec_xl_be). I am OK with either put these test2 and test3 into test 1, or
make them stand-alone, as long as these overloaded test cases for vec_xst_be
and vec_xl_be are put together seperately. Thanks for you good catch, this
problem is not found in our previous code review.
https://reviews.llvm.org/D27251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
