[PATCH] D27251: [PPC] some bugs mainly about sign problem fixed in altivec.h
kbarton requested changes to this revision.
kbarton added a comment.
This revision now requires changes to proceed.
Please make explicit the signed for the parameters to the functions you are
changing and remove unnecessary casts. I marked the first few that I found, but
stopped marking them after the first several.
Comment at: lib/Headers/altivec.h:13928
vector signed char __b) {
- return __builtin_altivec_vcmpgtub_p(__CR6_EQ, (vector unsigned char)__b,
- (vector unsigned char)__a);
+ return __builtin_altivec_vcmpgtsb_p(__CR6_EQ, (vector signed char)__b,
+ (vector signed char)__a);
The cast for __b is necessary, since it is already a vector signed char.
I don't know whether this will generate superfluous warnings or not, but it's
probably best to remove it.
Comment at: lib/Headers/altivec.h:13965
static __inline__ int __ATTRS_o_ai vec_all_ge(vector bool short __a,
vector short __b) {
+ return __builtin_altivec_vcmpgtsh_p(__CR6_EQ, (vector signed short)__b,
It's better to make the parameter explicitly vector signed short, and remove
the cast on the next line, for consistency with other builtins.
Comment at: lib/Headers/altivec.h:14002
static __inline__ int __ATTRS_o_ai vec_all_ge(vector bool int __a,
vector int __b) {
+ return __builtin_altivec_vcmpgtsw_p(__CR6_EQ, (vector signed int)__b,
same comment - explicitly vector signed int
Comment at: lib/Headers/altivec.h:14042
vector signed long long __b) {
- return __builtin_altivec_vcmpgtud_p(__CR6_EQ, (vector unsigned long long)__b,
- (vector unsigned long long)__a);
+ return __builtin_altivec_vcmpgtsd_p(__CR6_EQ, (vector signed long long)__b,
+ (vector signed long long)__a);
No cast needed here
Comment at: lib/Headers/altivec.h:14099
vector signed char __b) {
- return __builtin_altivec_vcmpgtub_p(__CR6_LT, (vector unsigned char)__a,
- (vector unsigned char)__b);
+ return __builtin_altivec_vcmpgtsb_p(__CR6_LT, (vector signed char)__a,
+ (vector signed char)__b);
No cast needed here
Comment at: lib/Headers/altivec.h:14136
static __inline__ int __ATTRS_o_ai vec_all_gt(vector bool short __a,
vector short __b) {
+ return __builtin_altivec_vcmpgtsh_p(__CR6_LT, (vector signed short)__a,
Make signed explicit here
https://reviews.llvm.org/D27251
___
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
kbarton added a comment.
Sorry, I don't have time to go through the entire patch in detail right now.
But I did notice several places where the lines are too long, which need to get
fixed.
Comment at: lib/Headers/altivec.h:14206
vector signed long long __b) {
- return __builtin_altivec_vcmpgtud_p(__CR6_LT, (vector unsigned long long)__a,
- (vector unsigned long long)__b);
+ return __builtin_altivec_vcmpgtsd_p(__CR6_LT, (vector signed long long)__a,
__b);
}
line too long
Comment at: lib/Headers/altivec.h:14381
vector signed long long __b) {
- return __builtin_altivec_vcmpgtud_p(__CR6_EQ, (vector unsigned long long)__a,
- (vector unsigned long long)__b);
+ return __builtin_altivec_vcmpgtsd_p(__CR6_EQ, (vector signed long long)__a,
__b);
}
line too long
Comment at: lib/Headers/altivec.h:14549
vector signed long long __b) {
- return __builtin_altivec_vcmpgtud_p(__CR6_LT, (vector unsigned long long)__b,
- (vector unsigned long long)__a);
+ return __builtin_altivec_vcmpgtsd_p(__CR6_LT, __b, (vector signed long
long)__a);
}
line too long
https://reviews.llvm.org/D27251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50989: Remove Darwin support from POWER backend.
kbarton created this revision.
kbarton added reviewers: power-llvm-team, hfinkel, echristo, rsmith.
Herald added a subscriber: nemanjai.
This is the clang counterpart to the patch posted in
https://reviews.llvm.org/D50988.
The patch removes Darwin support from the POWER backend. A similar approach
will be taken for the relevant code in Clang. Once the initial patch lands,
cleanup of the old PPC-specific Darwin paths can be done on demand, using
post-commit reviews whenever possible.
https://reviews.llvm.org/D50989
Files:
clang/test/CodeGen/bool_test.c
clang/test/CodeGen/darwin-ppc-varargs.c
clang/test/CodeGen/darwin-string-literals.c
clang/test/CodeGen/target-data.c
clang/test/CodeGenCXX/mangle-long-double.cpp
clang/test/Coverage/targets.c
Index: clang/test/Coverage/targets.c
===
--- clang/test/Coverage/targets.c
+++ clang/test/Coverage/targets.c
@@ -5,9 +5,7 @@
// RUN: %clang_cc1 -debug-info-kind=limited -triple i686-unknown-dragonfly -emit-llvm -o %t %s
// RUN: %clang_cc1 -debug-info-kind=limited -triple i686-unknown-unknown -emit-llvm -o %t %s
// RUN: %clang_cc1 -debug-info-kind=limited -triple i686-unknown-win32 -emit-llvm -o %t %s
-// RUN: %clang_cc1 -debug-info-kind=limited -triple powerpc-apple-darwin9 -emit-llvm -o %t %s
// RUN: %clang_cc1 -debug-info-kind=limited -triple powerpc-unknown-unknown -emit-llvm -o %t %s
-// RUN: %clang_cc1 -debug-info-kind=limited -triple powerpc64-apple-darwin9 -emit-llvm -o %t %s
// RUN: %clang_cc1 -debug-info-kind=limited -triple powerpc64-unknown-unknown -emit-llvm -o %t %s
// RUN: %clang_cc1 -debug-info-kind=limited -triple sparc-unknown-solaris -emit-llvm -o %t %s
// RUN: %clang_cc1 -debug-info-kind=limited -triple sparc-unknown-unknown -emit-llvm -o %t %s
Index: clang/test/CodeGenCXX/mangle-long-double.cpp
===
--- clang/test/CodeGenCXX/mangle-long-double.cpp
+++ clang/test/CodeGenCXX/mangle-long-double.cpp
@@ -1,12 +1,8 @@
// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu %s -emit-llvm -o - | FileCheck %s --check-prefix=POWER64-LINUX
// RUN: %clang_cc1 -triple powerpc-unknown-linux-gnu %s -emit-llvm -o - | FileCheck %s --check-prefix=POWER-LINUX
-// RUN: %clang_cc1 -triple powerpc64-apple-darwin9 %s -emit-llvm -o - | FileCheck %s --check-prefix=POWER64-DARWIN
-// RUN: %clang_cc1 -triple powerpc-apple-darwin9 %s -emit-llvm -o - | FileCheck %s --check-prefix=POWER-DARWIN
// RUN: %clang_cc1 -triple s390x-unknown-linux-gnu %s -emit-llvm -o - | FileCheck %s --check-prefix=S390X-LINUX
void f(long double) {}
// POWER64-LINUX: _Z1fg
// POWER-LINUX:_Z1fg
-// POWER64-DARWIN: _Z1fe
-// POWER-DARWIN: _Z1fe
// S390X-LINUX:_Z1fg
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -106,14 +106,6 @@
// RUN: FileCheck %s -check-prefix=PPC64LE-LINUX
// PPC64LE-LINUX: target datalayout = "e-m:e-i64:64-n32:64"
-// RUN: %clang_cc1 -triple powerpc-darwin -o - -emit-llvm %s | \
-// RUN: FileCheck %s -check-prefix=PPC32-DARWIN
-// PPC32-DARWIN: target datalayout = "E-m:o-p:32:32-f64:32:64-n32"
-
-// RUN: %clang_cc1 -triple powerpc64-darwin -o - -emit-llvm %s | \
-// RUN: FileCheck %s -check-prefix=PPC64-DARWIN
-// PPC64-DARWIN: target datalayout = "E-m:o-i64:64-n32:64"
-
// RUN: %clang_cc1 -triple nvptx-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=NVPTX
// NVPTX: target datalayout = "e-p:32:32-i64:64-i128:128-v16:16-v32:32-n16:32:64"
Index: clang/test/CodeGen/darwin-string-literals.c
===
--- clang/test/CodeGen/darwin-string-literals.c
+++ clang/test/CodeGen/darwin-string-literals.c
@@ -5,14 +5,6 @@
// CHECK-LSB: @.str.2 = private unnamed_addr constant [18 x i16] [i16 104, i16 101, i16 108, i16 108, i16 111, i16 32, i16 8594, i16 32, i16 9731, i16 32, i16 8592, i16 32, i16 119, i16 111, i16 114, i16 108, i16 100, i16 0], section "__TEXT,__ustring", align 2
// CHECK-LSB: @.str.4 = private unnamed_addr constant [6 x i16] [i16 116, i16 101, i16 115, i16 116, i16 8482, i16 0], section "__TEXT,__ustring", align 2
-
-// RUN: %clang_cc1 -triple powerpc-apple-darwin9 -emit-llvm %s -o - | FileCheck -check-prefix CHECK-MSB %s
-
-// CHECK-MSB: @.str = private unnamed_addr constant [8 x i8] c"string0\00"
-// CHECK-MSB: @.str.1 = private unnamed_addr constant [8 x i8] c"string1\00"
-// CHECK-MSB: @.str.2 = private unnamed_addr constant [18 x i16] [i16 104, i16 101, i16 108, i16 108, i16 111, i16 32, i16 8594, i16 32, i16 9731, i16 32, i16 8592, i16 32, i16 119, i16 111, i16 114, i16 108, i16 100, i16 0], section "__TEXT,__ustring", align 2
-// CHECK-MSB: @.str.4 = private unnamed_addr constant [6 x i16] [i16 116, i16 101, i16 115, i16 116, i16 8482, i16 0], section
[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends
kbarton added a comment.
I think this looks straightforward, as long as people agree to have a separate
CreateConstrained* version of these functions. I'm not qualified to weigh in on
that as I don't know Clang at all and can't comment about the tradeoffs
(although I think they have been well articulated in the discussion). From what
I can see, that is the only thing blocking this from getting approved (unless
there is something else I missed while reading the discussion).
The only other comment I have is there was some very good description about the
intention here from @uweigand, @cameron.mcinally and @andrew.w.kaylor and @kpn.
I think it would be good if that discussion is extracted from this review and
put somewhere (perhaps the language ref) to indicate precisely what the
semantics are we are trying to achieve with FENV_ACCESS ON/OFF.
Comment at: include/llvm/IR/IRBuilder.h:228
+ /// Enable/Disable use of constrained floating point math
+ void setIsConstrainedFP(bool IsCon) { IsFPConstrained = IsCon; }
+
This is a minor quibble, but the method is setIsConstrainedFP, while the member
is IsFPConstrained.
I'm not sure if that is intentionally different, or an oversight.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53157/new/
https://reviews.llvm.org/D53157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50989: Remove Darwin support from POWER backend.
kbarton closed this revision. kbarton added a comment. Herald added a subscriber: jsji. This landed in https://reviews.llvm.org/rL340770. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50989/new/ https://reviews.llvm.org/D50989 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D69088: [Lex] #pragma clang transform
kbarton added a comment. @Meinersbur I missed the RFC and discussion on the cfe-dev mailing list. Could you post a link here so that it's included in the history? I don't have any opposition to this, and it seems that you have addressed all the comments from reviewers. So, unless there was something that came up from the RFC discussion (which I doubt, since you just pinged the patch), I think this is good to land. However, I'm not really in a position to approve the patch since the implementation is well out of my domain of expertise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69088/new/ https://reviews.llvm.org/D69088 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
