Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
On Thu, Apr 14, 2016 at 03:02:31PM +, Taewook Oh via cfe-commits wrote: > Thank you for your comments. I submitted a new patch > (http://reviews.llvm.org/D19062) with a new test that checks attributes > but not types. I think this should cover both cases. What do you think? I think just picking two backends like ARM and X86 is less likely to create volatile IR and covers both aspects at the same time. It feels more robust that way. Joergr ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
Joerg, Thank you for your comments. I submitted a new patch (http://reviews.llvm.org/D19062) with a new test that checks attributes but not types. I think this should cover both cases. What do you think? Thanks, Taewook On 4/14/16, 2:46 AM, "Joerg Sonnenberger" wrote: >On Wed, Apr 13, 2016 at 03:02:09PM +, Taewook Oh wrote: >> Sure, I'll take a look. >> Thanks! > >Please provide two versions of the test, one hard-coded for an >architecture with signed char, one hard-coded for an architecture with >unsigned char. That should cover the difference. > >Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
On Wed, Apr 13, 2016 at 03:02:09PM +, Taewook Oh wrote: > Sure, I'll take a look. > Thanks! Please provide two versions of the test, one hard-coded for an architecture with signed char, one hard-coded for an architecture with unsigned char. That should cover the difference. Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
Sure, I'll take a look. Thanks! -- Taewook On 4/13/16, 8:00 AM, "Aaron Ballman" wrote: >aaron.ballman added a comment. > >I reverted this in r266201; it was causing build bot failures like: > >https://urldefense.proofpoint.com/v2/url?u=http-3A__lab.llvm.org-3A8011_bu >ilders_clang-2Dppc64be-2Dlinux_builds_3517&d=CwIFaQ&c=5VD0RTtNlTh3ycd41b3M >Uw&r=kOsLCgQzH7N8ptZ7diJD9g&m=lY0h_n-Ss10lCixhInwZ0rdHJVh8NThGYMjXc95lcXI& >s=jHcJfUPDOu_1aryqBoPpgyy4YSQAGSBc2Ay-uz13TFg&e= > >Can you please take a look? > >Thanks! > > >https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D1897 >0&d=CwIFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=lY0h_n-Ss10 >lCixhInwZ0rdHJVh8NThGYMjXc95lcXI&s=kmrF5dkYE8pxLuu3VXReWZ6ogoJqmOHz1rTfU4L >nQQ0&e= > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
aaron.ballman added a comment. I reverted this in r266201; it was causing build bot failures like: http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3517 Can you please take a look? Thanks! http://reviews.llvm.org/D18970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
aaron.ballman closed this revision. aaron.ballman added a comment. Commit in r266199, thank you! http://reviews.llvm.org/D18970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
twoh updated this revision to Diff 53395.
twoh added a comment.
Addressing comments from @aaron.ballman. It would be great if someone can
commit this patch for me, as I don't have the privilege yet.
http://reviews.llvm.org/D18970
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
lib/Sema/SemaDecl.cpp
test/FixIt/typo.m
test/Sema/enable_if.c
test/Sema/libbuiltins-ctype.c
Index: test/Sema/libbuiltins-ctype.c
===
--- test/Sema/libbuiltins-ctype.c
+++ test/Sema/libbuiltins-ctype.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+int isalnum(int);
+int isalpha(int);
+int isblank(int);
+int iscntrl(int);
+int isdigit(int);
+int isgraph(int);
+int islower(int);
+int isprint(int);
+int ispunct(int);
+int isspace(int);
+int isupper(int);
+int isxdigit(int);
+int tolower(int);
+int toupper(int);
+
+void test(int x) {
+ // CHECK: call i32 @isalnum(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isalnum(x);
+ // CHECK: call i32 @isalpha(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isalpha(x);
+ // CHECK: call i32 @isblank(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isblank(x);
+ // CHECK: call i32 @iscntrl(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)iscntrl(x);
+ // CHECK: call i32 @isdigit(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isdigit(x);
+ // CHECK: call i32 @isgraph(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isgraph(x);
+ // CHECK: call i32 @islower(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)islower(x);
+ // CHECK: call i32 @isprint(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isprint(x);
+ // CHECK: call i32 @ispunct(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)ispunct(x);
+ // CHECK: call i32 @isspace(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isspace(x);
+ // CHECK: call i32 @isupper(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isupper(x);
+ // CHECK: call i32 @isxdigit(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isxdigit(x);
+ // CHECK: call i32 @tolower(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)tolower(x);
+ // CHECK: call i32 @toupper(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)toupper(x);
+}
+
+// CHECK: declare i32 @isalnum(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isalpha(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isblank(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @iscntrl(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isdigit(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isgraph(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @islower(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isprint(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @ispunct(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isspace(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isupper(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isxdigit(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @tolower(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @toupper(i32) [[NUW_RO:#[0-9]+]]
+
+// CHECK: attributes [[NUW_RO]] = { nounwind readonly{{.*}} }
+// CHECK: attributes [[NUW_RO_CALL]] = { nounwind readonly }
Index: test/Sema/enable_if.c
===
--- test/Sema/enable_if.c
+++ test/Sema/enable_if.c
@@ -72,8 +72,8 @@
__attribute__((unavailable("'c' must have the value of an unsigned char or EOF")));
void test3(int c) {
- isdigit(c);
- isdigit(10);
+ isdigit(c); // expected-warning{{ignoring return value of function declared with pure attribute}}
+ isdigit(10); // expected-warning{{ignoring return value of function declared with pure attribute}}
#ifndef CODEGEN
isdigit(-10); // expected-error{{call to unavailable function 'isdigit': 'c' must have the value of an unsigned char or EOF}}
#endif
Index: test/FixIt/typo.m
===
--- test/FixIt/typo.m
+++ test/FixIt/typo.m
@@ -113,8 +113,6 @@
@end
-double *isupper(int);
-
@interface Sub2 : Super
- (int)method2;
@end
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -273,16 +273,16 @@
// so build a dependent node to describe the type.
if (WantNontrivialTypeSourceInfo)
return ActOnTypenameType(S, SourceLocation(), *SS, II, NameLoc).get();
-
+
NestedNameSpecifierLoc QualifierLoc = SS->getWithLocInContext(Context);
QualType T = CheckTypenameType(ETK_None, SourceLocation(), QualifierLoc,
II, NameLoc);
return ParsedType::make(T);
}
return nullptr;
}
-
+
if (!LookupCtx->isDependentContext() &&
RequireCompleteDeclContext(*SS, LookupCtx))
return nullptr;
@@ -303,7 +303,7 @@
if (ObjectTypePtr && Result.empty()) {
// C++ [basic.lookup.classref]p3:
// If the unqualified
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM, with one small nit.
Comment at: include/clang/Basic/Builtins.h:97
@@ -96,1 +96,3 @@
+ /// \brief Return true if this function has no side effects
+ bool isPure(unsigned ID) const {
Missing period at the end of the comment.
http://reviews.llvm.org/D18970
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
twoh updated this revision to Diff 53369.
twoh added a comment.
Fix attribute annotation from 'const' to 'pure'. Add support for 'pure'
attribute to builtin function database related code. Test case
(test/Sema/libbuiltins-ctype.c) added. Line 116 of test/FixIt/typo.m is removed
because it is not necessary for test but causes a warning that cannot be
addressed by -fixit once isupper is registered to Builtins.def.
http://reviews.llvm.org/D18970
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
lib/Sema/SemaDecl.cpp
test/FixIt/typo.m
test/Sema/enable_if.c
test/Sema/libbuiltins-ctype.c
Index: test/Sema/libbuiltins-ctype.c
===
--- test/Sema/libbuiltins-ctype.c
+++ test/Sema/libbuiltins-ctype.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+int isalnum(int);
+int isalpha(int);
+int isblank(int);
+int iscntrl(int);
+int isdigit(int);
+int isgraph(int);
+int islower(int);
+int isprint(int);
+int ispunct(int);
+int isspace(int);
+int isupper(int);
+int isxdigit(int);
+int tolower(int);
+int toupper(int);
+
+void test(int x) {
+ // CHECK: call i32 @isalnum(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isalnum(x);
+ // CHECK: call i32 @isalpha(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isalpha(x);
+ // CHECK: call i32 @isblank(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isblank(x);
+ // CHECK: call i32 @iscntrl(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)iscntrl(x);
+ // CHECK: call i32 @isdigit(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isdigit(x);
+ // CHECK: call i32 @isgraph(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isgraph(x);
+ // CHECK: call i32 @islower(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)islower(x);
+ // CHECK: call i32 @isprint(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isprint(x);
+ // CHECK: call i32 @ispunct(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)ispunct(x);
+ // CHECK: call i32 @isspace(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isspace(x);
+ // CHECK: call i32 @isupper(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isupper(x);
+ // CHECK: call i32 @isxdigit(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)isxdigit(x);
+ // CHECK: call i32 @tolower(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)tolower(x);
+ // CHECK: call i32 @toupper(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+ (void)toupper(x);
+}
+
+// CHECK: declare i32 @isalnum(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isalpha(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isblank(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @iscntrl(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isdigit(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isgraph(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @islower(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isprint(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @ispunct(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isspace(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isupper(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isxdigit(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @tolower(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @toupper(i32) [[NUW_RO:#[0-9]+]]
+
+// CHECK: attributes [[NUW_RO]] = { nounwind readonly{{.*}} }
+// CHECK: attributes [[NUW_RO_CALL]] = { nounwind readonly }
Index: test/Sema/enable_if.c
===
--- test/Sema/enable_if.c
+++ test/Sema/enable_if.c
@@ -72,8 +72,8 @@
__attribute__((unavailable("'c' must have the value of an unsigned char or EOF")));
void test3(int c) {
- isdigit(c);
- isdigit(10);
+ isdigit(c); // expected-warning{{ignoring return value of function declared with pure attribute}}
+ isdigit(10); // expected-warning{{ignoring return value of function declared with pure attribute}}
#ifndef CODEGEN
isdigit(-10); // expected-error{{call to unavailable function 'isdigit': 'c' must have the value of an unsigned char or EOF}}
#endif
Index: test/FixIt/typo.m
===
--- test/FixIt/typo.m
+++ test/FixIt/typo.m
@@ -113,8 +113,6 @@
@end
-double *isupper(int);
-
@interface Sub2 : Super
- (int)method2;
@end
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -273,16 +273,16 @@
// so build a dependent node to describe the type.
if (WantNontrivialTypeSourceInfo)
return ActOnTypenameType(S, SourceLocation(), *SS, II, NameLoc).get();
-
+
NestedNameSpecifierLoc QualifierLoc = SS->getWithLocInContext(Context);
QualType T = CheckTypenameType(ETK_None, SourceLocation(), QualifierLoc,
II, NameLoc);
return ParsedType::make(T);
}
return nullptr;
}
-
+
if (!LookupCtx->isDependentCont
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
twoh added a comment. @joerg My bad. They surely read from globals. I'll fix the diff. Thanks for the catch! http://reviews.llvm.org/D18970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18970: Add functions in ctype.h to builtin function database
joerg added a subscriber: joerg. joerg added a comment. ctype.h functions are only pure, not const? http://reviews.llvm.org/D18970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18970: Add functions in ctype.h to builtin function database
twoh created this revision. twoh added reviewers: rsmith, aaron.ballman. twoh added a subscriber: cfe-commits. Add functions declared in ctype.h to builtin function database. All functions are annotated with nothrow and const attribute, which enables better optimization. http://reviews.llvm.org/D18970 Files: include/clang/Basic/Builtins.def Index: include/clang/Basic/Builtins.def === --- include/clang/Basic/Builtins.def +++ include/clang/Basic/Builtins.def @@ -773,6 +773,22 @@ LIBBUILTIN(vscanf, "icC*Ra", "fS:0:", "stdio.h", ALL_LANGUAGES) LIBBUILTIN(vfscanf, "iP*RcC*Ra", "fS:1:", "stdio.h", ALL_LANGUAGES) LIBBUILTIN(vsscanf, "icC*RcC*Ra", "fS:1:", "stdio.h", ALL_LANGUAGES) +// C99 ctype.h +LIBBUILTIN(isalnum, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isalpha, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isblank, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(iscntrl, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isdigit, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isgraph, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(islower, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isprint, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(ispunct, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isspace, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isupper, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isxdigit, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(tolower, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(toupper, "ii", "fnc", "ctype.h", ALL_LANGUAGES) + // C99 // In some systems setjmp is a macro that expands to _setjmp. We undefine // it here to avoid having two identical LIBBUILTIN entries. Index: include/clang/Basic/Builtins.def === --- include/clang/Basic/Builtins.def +++ include/clang/Basic/Builtins.def @@ -773,6 +773,22 @@ LIBBUILTIN(vscanf, "icC*Ra", "fS:0:", "stdio.h", ALL_LANGUAGES) LIBBUILTIN(vfscanf, "iP*RcC*Ra", "fS:1:", "stdio.h", ALL_LANGUAGES) LIBBUILTIN(vsscanf, "icC*RcC*Ra", "fS:1:", "stdio.h", ALL_LANGUAGES) +// C99 ctype.h +LIBBUILTIN(isalnum, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isalpha, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isblank, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(iscntrl, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isdigit, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isgraph, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(islower, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isprint, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(ispunct, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isspace, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isupper, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(isxdigit, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(tolower, "ii", "fnc", "ctype.h", ALL_LANGUAGES) +LIBBUILTIN(toupper, "ii", "fnc", "ctype.h", ALL_LANGUAGES) + // C99 // In some systems setjmp is a macro that expands to _setjmp. We undefine // it here to avoid having two identical LIBBUILTIN entries. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
