[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
This revision was automatically updated to reflect the committed changes. Closed by commit rGdca37af061fb: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different (authored by zixuan-wu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 Files: clang/test/CodeGen/c-strings.c clang/test/Sema/builtin-alloca-with-align.c Index: clang/test/Sema/builtin-alloca-with-align.c === --- clang/test/Sema/builtin-alloca-with-align.c +++ clang/test/Sema/builtin-alloca-with-align.c @@ -30,4 +30,8 @@ void test8(void) { __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} +#if defined(__csky__) + // expected-error@-1 {{requested alignment must be 8 or greater}} + // Because the alignment of long long is 4 in CSKY target +#endif } Index: clang/test/CodeGen/c-strings.c === --- clang/test/CodeGen/c-strings.c +++ clang/test/CodeGen/c-strings.c @@ -20,6 +20,11 @@ // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived // from the alignment of a single i8, which is still 1. +// XFAIL: csky +// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which +// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived +// from the alignment of a single i8, which is still 1. + #if defined(__s390x__) unsigned char align = 2; #else @@ -69,4 +74,3 @@ } char x[3] = "ola"; - Index: clang/test/Sema/builtin-alloca-with-align.c === --- clang/test/Sema/builtin-alloca-with-align.c +++ clang/test/Sema/builtin-alloca-with-align.c @@ -30,4 +30,8 @@ void test8(void) { __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} +#if defined(__csky__) + // expected-error@-1 {{requested alignment must be 8 or greater}} + // Because the alignment of long long is 4 in CSKY target +#endif } Index: clang/test/CodeGen/c-strings.c === --- clang/test/CodeGen/c-strings.c +++ clang/test/CodeGen/c-strings.c @@ -20,6 +20,11 @@ // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived // from the alignment of a single i8, which is still 1. +// XFAIL: csky +// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which +// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived +// from the alignment of a single i8, which is still 1. + #if defined(__s390x__) unsigned char align = 2; #else @@ -69,4 +74,3 @@ } char x[3] = "ola"; - ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
zixuan-wu added a comment. Gentle pin.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
zixuan-wu updated this revision to Diff 428585. zixuan-wu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 Files: clang/test/CodeGen/c-strings.c clang/test/Sema/builtin-alloca-with-align.c Index: clang/test/Sema/builtin-alloca-with-align.c === --- clang/test/Sema/builtin-alloca-with-align.c +++ clang/test/Sema/builtin-alloca-with-align.c @@ -30,4 +30,8 @@ void test8(void) { __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} +#if defined(__csky__) + // expected-error@-1 {{requested alignment must be 8 or greater}} + // Because the alignment of long long is 4 in CSKY target +#endif } Index: clang/test/CodeGen/c-strings.c === --- clang/test/CodeGen/c-strings.c +++ clang/test/CodeGen/c-strings.c @@ -20,6 +20,11 @@ // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived // from the alignment of a single i8, which is still 1. +// XFAIL: csky +// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which +// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived +// from the alignment of a single i8, which is still 1. + #if defined(__s390x__) unsigned char align = 2; #else @@ -69,4 +74,3 @@ } char x[3] = "ola"; - Index: clang/test/Sema/builtin-alloca-with-align.c === --- clang/test/Sema/builtin-alloca-with-align.c +++ clang/test/Sema/builtin-alloca-with-align.c @@ -30,4 +30,8 @@ void test8(void) { __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} +#if defined(__csky__) + // expected-error@-1 {{requested alignment must be 8 or greater}} + // Because the alignment of long long is 4 in CSKY target +#endif } Index: clang/test/CodeGen/c-strings.c === --- clang/test/CodeGen/c-strings.c +++ clang/test/CodeGen/c-strings.c @@ -20,6 +20,11 @@ // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived // from the alignment of a single i8, which is still 1. +// XFAIL: csky +// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which +// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived +// from the alignment of a single i8, which is still 1. + #if defined(__s390x__) unsigned char align = 2; #else @@ -69,4 +74,3 @@ } char x[3] = "ola"; - ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
zixuan-wu added inline comments. Comment at: clang/test/Sema/builtin-alloca-with-align.c:32 void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} aaron.ballman wrote: > rengolin wrote: > > zixuan-wu wrote: > > > rengolin wrote: > > > > This test is platform agnostic, perhaps the extra error could be in a > > > > new test, exclusively for csky? > > > Then I prefer to split the file and add UNSUPPORTED for CSKY in the other > > > file which only contains test8 > > But then you wouldn't be testing the extra error you want... hmm. > > > > Maybe it would be fine the way you did it originally. > > > > Would be nice to get a clang person to give their opinion. > // Comment which explains why this is special. > I think I would test this slightly differently: > ``` > void test8(void) { > __builtin_alloca_with_align(sizeof(__INT64_TYPE__), > __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to > __builtin_alloca_with_align is supposed to be in bits}} > #ifdef __csky__ > // expected-error@-2 {{requested alignment must be 8 or greater}} > // Comment which explains why this is special. > #endif // __csky__ > } > ``` Reusing the first warning is good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
aaron.ballman added inline comments. Comment at: clang/test/Sema/builtin-alloca-with-align.c:32 void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} rengolin wrote: > zixuan-wu wrote: > > rengolin wrote: > > > This test is platform agnostic, perhaps the extra error could be in a new > > > test, exclusively for csky? > > Then I prefer to split the file and add UNSUPPORTED for CSKY in the other > > file which only contains test8 > But then you wouldn't be testing the extra error you want... hmm. > > Maybe it would be fine the way you did it originally. > > Would be nice to get a clang person to give their opinion. // Comment which explains why this is special. I think I would test this slightly differently: ``` void test8(void) { __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} #ifdef __csky__ // expected-error@-2 {{requested alignment must be 8 or greater}} // Comment which explains why this is special. #endif // __csky__ } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
zixuan-wu added a comment. Could anybody else have a review or nominate a reviewer? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
rengolin added inline comments. Comment at: clang/test/Sema/builtin-alloca-with-align.c:32 void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} zixuan-wu wrote: > rengolin wrote: > > This test is platform agnostic, perhaps the extra error could be in a new > > test, exclusively for csky? > Then I prefer to split the file and add UNSUPPORTED for CSKY in the other > file which only contains test8 But then you wouldn't be testing the extra error you want... hmm. Maybe it would be fine the way you did it originally. Would be nice to get a clang person to give their opinion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
zixuan-wu added inline comments. Comment at: clang/test/Sema/builtin-alloca-with-align.c:32 void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} rengolin wrote: > This test is platform agnostic, perhaps the extra error could be in a new > test, exclusively for csky? Then I prefer to split the file and add UNSUPPORTED for CSKY in the other file which only contains test8 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
rengolin added inline comments. Comment at: clang/test/Sema/builtin-alloca-with-align.c:32 void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} This test is platform agnostic, perhaps the extra error could be in a new test, exclusively for csky? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124977/new/ https://reviews.llvm.org/D124977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different
zixuan-wu created this revision. zixuan-wu added reviewers: majnemer, kparzysz, ddunbar. Herald added a project: All. zixuan-wu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. CSKY is always in 4-byte align, no matter it's long long type. For aggregate type, it's 4-byte align if its size is bigger than or equal to 4 bytes. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D124977 Files: clang/test/CodeGen/c-strings.c clang/test/Sema/builtin-alloca-with-align.c Index: clang/test/Sema/builtin-alloca-with-align.c === --- clang/test/Sema/builtin-alloca-with-align.c +++ clang/test/Sema/builtin-alloca-with-align.c @@ -29,5 +29,10 @@ } void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} + // expected-error@-1 {{requested alignment must be 8 or greater}} +#else + __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} +#endif } Index: clang/test/CodeGen/c-strings.c === --- clang/test/CodeGen/c-strings.c +++ clang/test/CodeGen/c-strings.c @@ -20,6 +20,11 @@ // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived // from the alignment of a single i8, which is still 1. +// XFAIL: csky +// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which +// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived +// from the alignment of a single i8, which is still 1. + #if defined(__s390x__) unsigned char align = 2; #else @@ -69,4 +74,3 @@ } char x[3] = "ola"; - Index: clang/test/Sema/builtin-alloca-with-align.c === --- clang/test/Sema/builtin-alloca-with-align.c +++ clang/test/Sema/builtin-alloca-with-align.c @@ -29,5 +29,10 @@ } void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} +// expected-error@-1 {{requested alignment must be 8 or greater}} +#else + __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}} +#endif } Index: clang/test/CodeGen/c-strings.c === --- clang/test/CodeGen/c-strings.c +++ clang/test/CodeGen/c-strings.c @@ -20,6 +20,11 @@ // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived // from the alignment of a single i8, which is still 1. +// XFAIL: csky +// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which +// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived +// from the alignment of a single i8, which is still 1. + #if defined(__s390x__) unsigned char align = 2; #else @@ -69,4 +74,3 @@ } char x[3] = "ola"; - ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits