[PATCH] D108469: Improve handling of static assert messages.

2021-09-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 373404.
cor3ntin marked 5 inline comments as done.
cor3ntin added a comment.

- Fix tests, cland tidy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108469

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp
  clang/test/Lexer/null-character-in-literal.c
  clang/test/Misc/diag-special-chars.c
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/int-ptr-cast-SFINAE.cpp
  clang/test/SemaCXX/static-assert.cpp
  llvm/include/llvm/Support/Unicode.h
  llvm/lib/Support/Unicode.cpp

Index: llvm/lib/Support/Unicode.cpp
===
--- llvm/lib/Support/Unicode.cpp
+++ llvm/lib/Support/Unicode.cpp
@@ -20,196 +20,262 @@
 namespace unicode {
 
 bool isPrintable(int UCS) {
-  // Sorted list of non-overlapping intervals of code points that are not
-  // supposed to be printable.
-  static const UnicodeCharRange NonPrintableRanges[] = {
-{ 0x, 0x001F }, { 0x007F, 0x009F }, { 0x034F, 0x034F },
-{ 0x0378, 0x0379 }, { 0x037F, 0x0383 }, { 0x038B, 0x038B },
-{ 0x038D, 0x038D }, { 0x03A2, 0x03A2 }, { 0x0528, 0x0530 },
-{ 0x0557, 0x0558 }, { 0x0560, 0x0560 }, { 0x0588, 0x0588 },
-{ 0x058B, 0x058E }, { 0x0590, 0x0590 }, { 0x05C8, 0x05CF },
-{ 0x05EB, 0x05EF }, { 0x05F5, 0x0605 }, { 0x061C, 0x061D },
-{ 0x06DD, 0x06DD }, { 0x070E, 0x070F }, { 0x074B, 0x074C },
-{ 0x07B2, 0x07BF }, { 0x07FB, 0x07FF }, { 0x082E, 0x082F },
-{ 0x083F, 0x083F }, { 0x085C, 0x085D }, { 0x085F, 0x089F },
-{ 0x08A1, 0x08A1 }, { 0x08AD, 0x08E3 }, { 0x08FF, 0x08FF },
-{ 0x0978, 0x0978 }, { 0x0980, 0x0980 }, { 0x0984, 0x0984 },
-{ 0x098D, 0x098E }, { 0x0991, 0x0992 }, { 0x09A9, 0x09A9 },
-{ 0x09B1, 0x09B1 }, { 0x09B3, 0x09B5 }, { 0x09BA, 0x09BB },
-{ 0x09C5, 0x09C6 }, { 0x09C9, 0x09CA }, { 0x09CF, 0x09D6 },
-{ 0x09D8, 0x09DB }, { 0x09DE, 0x09DE }, { 0x09E4, 0x09E5 },
-{ 0x09FC, 0x0A00 }, { 0x0A04, 0x0A04 }, { 0x0A0B, 0x0A0E },
-{ 0x0A11, 0x0A12 }, { 0x0A29, 0x0A29 }, { 0x0A31, 0x0A31 },
-{ 0x0A34, 0x0A34 }, { 0x0A37, 0x0A37 }, { 0x0A3A, 0x0A3B },
-{ 0x0A3D, 0x0A3D }, { 0x0A43, 0x0A46 }, { 0x0A49, 0x0A4A },
-{ 0x0A4E, 0x0A50 }, { 0x0A52, 0x0A58 }, { 0x0A5D, 0x0A5D },
-{ 0x0A5F, 0x0A65 }, { 0x0A76, 0x0A80 }, { 0x0A84, 0x0A84 },
-{ 0x0A8E, 0x0A8E }, { 0x0A92, 0x0A92 }, { 0x0AA9, 0x0AA9 },
-{ 0x0AB1, 0x0AB1 }, { 0x0AB4, 0x0AB4 }, { 0x0ABA, 0x0ABB },
-{ 0x0AC6, 0x0AC6 }, { 0x0ACA, 0x0ACA }, { 0x0ACE, 0x0ACF },
-{ 0x0AD1, 0x0ADF }, { 0x0AE4, 0x0AE5 }, { 0x0AF2, 0x0B00 },
-{ 0x0B04, 0x0B04 }, { 0x0B0D, 0x0B0E }, { 0x0B11, 0x0B12 },
-{ 0x0B29, 0x0B29 }, { 0x0B31, 0x0B31 }, { 0x0B34, 0x0B34 },
-{ 0x0B3A, 0x0B3B }, { 0x0B45, 0x0B46 }, { 0x0B49, 0x0B4A },
-{ 0x0B4E, 0x0B55 }, { 0x0B58, 0x0B5B }, { 0x0B5E, 0x0B5E },
-{ 0x0B64, 0x0B65 }, { 0x0B78, 0x0B81 }, { 0x0B84, 0x0B84 },
-{ 0x0B8B, 0x0B8D }, { 0x0B91, 0x0B91 }, { 0x0B96, 0x0B98 },
-{ 0x0B9B, 0x0B9B }, { 0x0B9D, 0x0B9D }, { 0x0BA0, 0x0BA2 },
-{ 0x0BA5, 0x0BA7 }, { 0x0BAB, 0x0BAD }, { 0x0BBA, 0x0BBD },
-{ 0x0BC3, 0x0BC5 }, { 0x0BC9, 0x0BC9 }, { 0x0BCE, 0x0BCF },
-{ 0x0BD1, 0x0BD6 }, { 0x0BD8, 0x0BE5 }, { 0x0BFB, 0x0C00 },
-{ 0x0C04, 0x0C04 }, { 0x0C0D, 0x0C0D }, { 0x0C11, 0x0C11 },
-{ 0x0C29, 0x0C29 }, { 0x0C34, 0x0C34 }, { 0x0C3A, 0x0C3C },
-{ 0x0C45, 0x0C45 }, { 0x0C49, 0x0C49 }, { 0x0C4E, 0x0C54 },
-{ 0x0C57, 0x0C57 }, { 0x0C5A, 0x0C5F }, { 0x0C64, 0x0C65 },
-{ 0x0C70, 0x0C77 }, { 0x0C80, 0x0C81 }, { 0x0C84, 0x0C84 },
-{ 0x0C8D, 0x0C8D }, { 0x0C91, 0x0C91 }, { 0x0CA9, 0x0CA9 },
-{ 0x0CB4, 0x0CB4 }, { 0x0CBA, 0x0CBB }, { 0x0CC5, 0x0CC5 },
-{ 0x0CC9, 0x0CC9 }, { 0x0CCE, 0x0CD4 }, { 0x0CD7, 0x0CDD },
-{ 0x0CDF, 0x0CDF }, { 0x0CE4, 0x0CE5 }, { 0x0CF0, 0x0CF0 },
-{ 0x0CF3, 0x0D01 }, { 0x0D04, 0x0D04 }, { 0x0D0D, 0x0D0D },
-{ 0x0D11, 0x0D11 }, { 0x0D3B, 0x0D3C }, { 0x0D45, 0x0D45 },
-{ 0x0D49, 0x0D49 }, { 0x0D4F, 0x0D56 }, { 0x0D58, 0x0D5F },
-{ 0x0D64, 0x0D65 }, { 0x0D76, 0x0D78 }, { 0x0D80, 0x0D81 },
-{ 0x0D84, 0x0D84 }, { 0x0D97, 0x0D99 }, { 0x0DB2, 0x0DB2 },
-{ 0x0DBC, 0x0DBC }, { 0x0DBE, 0x0DBF }, { 0x0DC7, 0x0DC9 },
-{ 0x0DCB, 0x0DCE }, { 0x0DD5, 0x0DD5 }, { 0x0DD7, 0x0DD7 },
-{ 0x0DE0, 0x0DF1 }, { 0x0DF5, 0x0E00 }, { 0x0E3B, 0x0E3E },
-{ 0x0E5C, 0x0E80 }, { 0x0E83, 0x0E83 }, { 0x0E85, 0x0E86 },
-{ 0x0E89, 0x0E89 }, { 0x0E8B, 0x0E8C }, { 0x0E8E, 0x0E93 },
-{ 0x0E98, 0x0E98 }, { 0x0EA0, 0x0EA0 }, { 0x0EA4, 0x0EA4 },
-{ 0x0EA6, 0x0EA6 }, { 0x0EA8, 0x0EA9 }, { 0x0EAC, 0x0EAC },
-{ 0x0EBA, 0x0EBA }, { 0x0EBE, 0x0EBF }, { 0x0EC5, 0x0EC5 },
-{ 0x0EC7, 0x0EC7 }, { 0x0ECE, 0x0ECF }, { 0x0EDA, 0x0

[PATCH] D108469: Improve handling of static assert messages.

2021-09-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 373405.
cor3ntin added a comment.

Formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108469

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp
  clang/test/Lexer/null-character-in-literal.c
  clang/test/Misc/diag-special-chars.c
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/int-ptr-cast-SFINAE.cpp
  clang/test/SemaCXX/static-assert.cpp
  llvm/include/llvm/Support/Unicode.h
  llvm/lib/Support/Unicode.cpp

Index: llvm/lib/Support/Unicode.cpp
===
--- llvm/lib/Support/Unicode.cpp
+++ llvm/lib/Support/Unicode.cpp
@@ -20,196 +20,263 @@
 namespace unicode {
 
 bool isPrintable(int UCS) {
-  // Sorted list of non-overlapping intervals of code points that are not
-  // supposed to be printable.
-  static const UnicodeCharRange NonPrintableRanges[] = {
-{ 0x, 0x001F }, { 0x007F, 0x009F }, { 0x034F, 0x034F },
-{ 0x0378, 0x0379 }, { 0x037F, 0x0383 }, { 0x038B, 0x038B },
-{ 0x038D, 0x038D }, { 0x03A2, 0x03A2 }, { 0x0528, 0x0530 },
-{ 0x0557, 0x0558 }, { 0x0560, 0x0560 }, { 0x0588, 0x0588 },
-{ 0x058B, 0x058E }, { 0x0590, 0x0590 }, { 0x05C8, 0x05CF },
-{ 0x05EB, 0x05EF }, { 0x05F5, 0x0605 }, { 0x061C, 0x061D },
-{ 0x06DD, 0x06DD }, { 0x070E, 0x070F }, { 0x074B, 0x074C },
-{ 0x07B2, 0x07BF }, { 0x07FB, 0x07FF }, { 0x082E, 0x082F },
-{ 0x083F, 0x083F }, { 0x085C, 0x085D }, { 0x085F, 0x089F },
-{ 0x08A1, 0x08A1 }, { 0x08AD, 0x08E3 }, { 0x08FF, 0x08FF },
-{ 0x0978, 0x0978 }, { 0x0980, 0x0980 }, { 0x0984, 0x0984 },
-{ 0x098D, 0x098E }, { 0x0991, 0x0992 }, { 0x09A9, 0x09A9 },
-{ 0x09B1, 0x09B1 }, { 0x09B3, 0x09B5 }, { 0x09BA, 0x09BB },
-{ 0x09C5, 0x09C6 }, { 0x09C9, 0x09CA }, { 0x09CF, 0x09D6 },
-{ 0x09D8, 0x09DB }, { 0x09DE, 0x09DE }, { 0x09E4, 0x09E5 },
-{ 0x09FC, 0x0A00 }, { 0x0A04, 0x0A04 }, { 0x0A0B, 0x0A0E },
-{ 0x0A11, 0x0A12 }, { 0x0A29, 0x0A29 }, { 0x0A31, 0x0A31 },
-{ 0x0A34, 0x0A34 }, { 0x0A37, 0x0A37 }, { 0x0A3A, 0x0A3B },
-{ 0x0A3D, 0x0A3D }, { 0x0A43, 0x0A46 }, { 0x0A49, 0x0A4A },
-{ 0x0A4E, 0x0A50 }, { 0x0A52, 0x0A58 }, { 0x0A5D, 0x0A5D },
-{ 0x0A5F, 0x0A65 }, { 0x0A76, 0x0A80 }, { 0x0A84, 0x0A84 },
-{ 0x0A8E, 0x0A8E }, { 0x0A92, 0x0A92 }, { 0x0AA9, 0x0AA9 },
-{ 0x0AB1, 0x0AB1 }, { 0x0AB4, 0x0AB4 }, { 0x0ABA, 0x0ABB },
-{ 0x0AC6, 0x0AC6 }, { 0x0ACA, 0x0ACA }, { 0x0ACE, 0x0ACF },
-{ 0x0AD1, 0x0ADF }, { 0x0AE4, 0x0AE5 }, { 0x0AF2, 0x0B00 },
-{ 0x0B04, 0x0B04 }, { 0x0B0D, 0x0B0E }, { 0x0B11, 0x0B12 },
-{ 0x0B29, 0x0B29 }, { 0x0B31, 0x0B31 }, { 0x0B34, 0x0B34 },
-{ 0x0B3A, 0x0B3B }, { 0x0B45, 0x0B46 }, { 0x0B49, 0x0B4A },
-{ 0x0B4E, 0x0B55 }, { 0x0B58, 0x0B5B }, { 0x0B5E, 0x0B5E },
-{ 0x0B64, 0x0B65 }, { 0x0B78, 0x0B81 }, { 0x0B84, 0x0B84 },
-{ 0x0B8B, 0x0B8D }, { 0x0B91, 0x0B91 }, { 0x0B96, 0x0B98 },
-{ 0x0B9B, 0x0B9B }, { 0x0B9D, 0x0B9D }, { 0x0BA0, 0x0BA2 },
-{ 0x0BA5, 0x0BA7 }, { 0x0BAB, 0x0BAD }, { 0x0BBA, 0x0BBD },
-{ 0x0BC3, 0x0BC5 }, { 0x0BC9, 0x0BC9 }, { 0x0BCE, 0x0BCF },
-{ 0x0BD1, 0x0BD6 }, { 0x0BD8, 0x0BE5 }, { 0x0BFB, 0x0C00 },
-{ 0x0C04, 0x0C04 }, { 0x0C0D, 0x0C0D }, { 0x0C11, 0x0C11 },
-{ 0x0C29, 0x0C29 }, { 0x0C34, 0x0C34 }, { 0x0C3A, 0x0C3C },
-{ 0x0C45, 0x0C45 }, { 0x0C49, 0x0C49 }, { 0x0C4E, 0x0C54 },
-{ 0x0C57, 0x0C57 }, { 0x0C5A, 0x0C5F }, { 0x0C64, 0x0C65 },
-{ 0x0C70, 0x0C77 }, { 0x0C80, 0x0C81 }, { 0x0C84, 0x0C84 },
-{ 0x0C8D, 0x0C8D }, { 0x0C91, 0x0C91 }, { 0x0CA9, 0x0CA9 },
-{ 0x0CB4, 0x0CB4 }, { 0x0CBA, 0x0CBB }, { 0x0CC5, 0x0CC5 },
-{ 0x0CC9, 0x0CC9 }, { 0x0CCE, 0x0CD4 }, { 0x0CD7, 0x0CDD },
-{ 0x0CDF, 0x0CDF }, { 0x0CE4, 0x0CE5 }, { 0x0CF0, 0x0CF0 },
-{ 0x0CF3, 0x0D01 }, { 0x0D04, 0x0D04 }, { 0x0D0D, 0x0D0D },
-{ 0x0D11, 0x0D11 }, { 0x0D3B, 0x0D3C }, { 0x0D45, 0x0D45 },
-{ 0x0D49, 0x0D49 }, { 0x0D4F, 0x0D56 }, { 0x0D58, 0x0D5F },
-{ 0x0D64, 0x0D65 }, { 0x0D76, 0x0D78 }, { 0x0D80, 0x0D81 },
-{ 0x0D84, 0x0D84 }, { 0x0D97, 0x0D99 }, { 0x0DB2, 0x0DB2 },
-{ 0x0DBC, 0x0DBC }, { 0x0DBE, 0x0DBF }, { 0x0DC7, 0x0DC9 },
-{ 0x0DCB, 0x0DCE }, { 0x0DD5, 0x0DD5 }, { 0x0DD7, 0x0DD7 },
-{ 0x0DE0, 0x0DF1 }, { 0x0DF5, 0x0E00 }, { 0x0E3B, 0x0E3E },
-{ 0x0E5C, 0x0E80 }, { 0x0E83, 0x0E83 }, { 0x0E85, 0x0E86 },
-{ 0x0E89, 0x0E89 }, { 0x0E8B, 0x0E8C }, { 0x0E8E, 0x0E93 },
-{ 0x0E98, 0x0E98 }, { 0x0EA0, 0x0EA0 }, { 0x0EA4, 0x0EA4 },
-{ 0x0EA6, 0x0EA6 }, { 0x0EA8, 0x0EA9 }, { 0x0EAC, 0x0EAC },
-{ 0x0EBA, 0x0EBA }, { 0x0EBE, 0x0EBF }, { 0x0EC5, 0x0EC5 },
-{ 0x0EC7, 0x0EC7 }, { 0x0ECE, 0x0ECF }, { 0x0EDA, 0x0EDB },
-{ 0x0EE0, 0x0EFF }, { 0x0F48, 0x0F48 }, { 0x

[PATCH] D108469: Improve handling of static assert messages.

2021-09-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

In D108469#3002209 , @aaron.ballman 
wrote:

> In D108469#2957652 , @jfb wrote:
>
>> I worry that changing the general `static_assert` printing (adding a colon, 
>> and dropping the quotes) will get @hwright's law to drop on us. We can try 
>> and see if e.g. users of clang have automated checks for `static_assert` in 
>> their CI pipelines or something. I think your new format looks better, but 
>> Hyrum is finicky that way... What do others think?
>
> I think it's fine to change; we've never promised diagnostic formatting 
> compatibility between versions. I'm sure *someone* is relying on this 
> somewhere, but I'm not worried we're going to break a ton of people -- 
> hopefully enough folks are tracking trunk that we can find any major issues 
> before releasing.
>
> Btw, it looks like the CI is currently failing the LLVM unit tests in 
> interesting ways. That should be resolved.
>
> There are changes in `clang/test/Lexer/null-character-in-literal.c` but Phab 
> is unhelpful about showing what those changes are because it thinks the file 
> is a binary file. Can you explain what's been changed there?

A verbatim NULL is replaced by  in the expected-error of a test that 
checks the handling of null in header-name (!)

  -  // expected-error@-1 {{'null\^@character' file not 
found}}
  +  // expected-error@-1 {{'null\character' 
file not found}}




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108469

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


[PATCH] D108469: Improve handling of static assert messages.

2021-09-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 373408.
cor3ntin added a comment.

Properly comment isFormatting/isPrintable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108469

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp
  clang/test/Lexer/null-character-in-literal.c
  clang/test/Misc/diag-special-chars.c
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/int-ptr-cast-SFINAE.cpp
  clang/test/SemaCXX/static-assert.cpp
  llvm/include/llvm/Support/Unicode.h
  llvm/lib/Support/Unicode.cpp

Index: llvm/lib/Support/Unicode.cpp
===
--- llvm/lib/Support/Unicode.cpp
+++ llvm/lib/Support/Unicode.cpp
@@ -19,197 +19,271 @@
 namespace sys {
 namespace unicode {
 
+/// Unicode code points of the categories L, M, N, P, S and Zs are considered
+/// printable.
+/// In addition, U+00AD SOFT HYPHEN is also considered printable, as
+/// it's actually displayed on most terminals. \return true if the character is
+/// considered printable.
 bool isPrintable(int UCS) {
-  // Sorted list of non-overlapping intervals of code points that are not
-  // supposed to be printable.
-  static const UnicodeCharRange NonPrintableRanges[] = {
-{ 0x, 0x001F }, { 0x007F, 0x009F }, { 0x034F, 0x034F },
-{ 0x0378, 0x0379 }, { 0x037F, 0x0383 }, { 0x038B, 0x038B },
-{ 0x038D, 0x038D }, { 0x03A2, 0x03A2 }, { 0x0528, 0x0530 },
-{ 0x0557, 0x0558 }, { 0x0560, 0x0560 }, { 0x0588, 0x0588 },
-{ 0x058B, 0x058E }, { 0x0590, 0x0590 }, { 0x05C8, 0x05CF },
-{ 0x05EB, 0x05EF }, { 0x05F5, 0x0605 }, { 0x061C, 0x061D },
-{ 0x06DD, 0x06DD }, { 0x070E, 0x070F }, { 0x074B, 0x074C },
-{ 0x07B2, 0x07BF }, { 0x07FB, 0x07FF }, { 0x082E, 0x082F },
-{ 0x083F, 0x083F }, { 0x085C, 0x085D }, { 0x085F, 0x089F },
-{ 0x08A1, 0x08A1 }, { 0x08AD, 0x08E3 }, { 0x08FF, 0x08FF },
-{ 0x0978, 0x0978 }, { 0x0980, 0x0980 }, { 0x0984, 0x0984 },
-{ 0x098D, 0x098E }, { 0x0991, 0x0992 }, { 0x09A9, 0x09A9 },
-{ 0x09B1, 0x09B1 }, { 0x09B3, 0x09B5 }, { 0x09BA, 0x09BB },
-{ 0x09C5, 0x09C6 }, { 0x09C9, 0x09CA }, { 0x09CF, 0x09D6 },
-{ 0x09D8, 0x09DB }, { 0x09DE, 0x09DE }, { 0x09E4, 0x09E5 },
-{ 0x09FC, 0x0A00 }, { 0x0A04, 0x0A04 }, { 0x0A0B, 0x0A0E },
-{ 0x0A11, 0x0A12 }, { 0x0A29, 0x0A29 }, { 0x0A31, 0x0A31 },
-{ 0x0A34, 0x0A34 }, { 0x0A37, 0x0A37 }, { 0x0A3A, 0x0A3B },
-{ 0x0A3D, 0x0A3D }, { 0x0A43, 0x0A46 }, { 0x0A49, 0x0A4A },
-{ 0x0A4E, 0x0A50 }, { 0x0A52, 0x0A58 }, { 0x0A5D, 0x0A5D },
-{ 0x0A5F, 0x0A65 }, { 0x0A76, 0x0A80 }, { 0x0A84, 0x0A84 },
-{ 0x0A8E, 0x0A8E }, { 0x0A92, 0x0A92 }, { 0x0AA9, 0x0AA9 },
-{ 0x0AB1, 0x0AB1 }, { 0x0AB4, 0x0AB4 }, { 0x0ABA, 0x0ABB },
-{ 0x0AC6, 0x0AC6 }, { 0x0ACA, 0x0ACA }, { 0x0ACE, 0x0ACF },
-{ 0x0AD1, 0x0ADF }, { 0x0AE4, 0x0AE5 }, { 0x0AF2, 0x0B00 },
-{ 0x0B04, 0x0B04 }, { 0x0B0D, 0x0B0E }, { 0x0B11, 0x0B12 },
-{ 0x0B29, 0x0B29 }, { 0x0B31, 0x0B31 }, { 0x0B34, 0x0B34 },
-{ 0x0B3A, 0x0B3B }, { 0x0B45, 0x0B46 }, { 0x0B49, 0x0B4A },
-{ 0x0B4E, 0x0B55 }, { 0x0B58, 0x0B5B }, { 0x0B5E, 0x0B5E },
-{ 0x0B64, 0x0B65 }, { 0x0B78, 0x0B81 }, { 0x0B84, 0x0B84 },
-{ 0x0B8B, 0x0B8D }, { 0x0B91, 0x0B91 }, { 0x0B96, 0x0B98 },
-{ 0x0B9B, 0x0B9B }, { 0x0B9D, 0x0B9D }, { 0x0BA0, 0x0BA2 },
-{ 0x0BA5, 0x0BA7 }, { 0x0BAB, 0x0BAD }, { 0x0BBA, 0x0BBD },
-{ 0x0BC3, 0x0BC5 }, { 0x0BC9, 0x0BC9 }, { 0x0BCE, 0x0BCF },
-{ 0x0BD1, 0x0BD6 }, { 0x0BD8, 0x0BE5 }, { 0x0BFB, 0x0C00 },
-{ 0x0C04, 0x0C04 }, { 0x0C0D, 0x0C0D }, { 0x0C11, 0x0C11 },
-{ 0x0C29, 0x0C29 }, { 0x0C34, 0x0C34 }, { 0x0C3A, 0x0C3C },
-{ 0x0C45, 0x0C45 }, { 0x0C49, 0x0C49 }, { 0x0C4E, 0x0C54 },
-{ 0x0C57, 0x0C57 }, { 0x0C5A, 0x0C5F }, { 0x0C64, 0x0C65 },
-{ 0x0C70, 0x0C77 }, { 0x0C80, 0x0C81 }, { 0x0C84, 0x0C84 },
-{ 0x0C8D, 0x0C8D }, { 0x0C91, 0x0C91 }, { 0x0CA9, 0x0CA9 },
-{ 0x0CB4, 0x0CB4 }, { 0x0CBA, 0x0CBB }, { 0x0CC5, 0x0CC5 },
-{ 0x0CC9, 0x0CC9 }, { 0x0CCE, 0x0CD4 }, { 0x0CD7, 0x0CDD },
-{ 0x0CDF, 0x0CDF }, { 0x0CE4, 0x0CE5 }, { 0x0CF0, 0x0CF0 },
-{ 0x0CF3, 0x0D01 }, { 0x0D04, 0x0D04 }, { 0x0D0D, 0x0D0D },
-{ 0x0D11, 0x0D11 }, { 0x0D3B, 0x0D3C }, { 0x0D45, 0x0D45 },
-{ 0x0D49, 0x0D49 }, { 0x0D4F, 0x0D56 }, { 0x0D58, 0x0D5F },
-{ 0x0D64, 0x0D65 }, { 0x0D76, 0x0D78 }, { 0x0D80, 0x0D81 },
-{ 0x0D84, 0x0D84 }, { 0x0D97, 0x0D99 }, { 0x0DB2, 0x0DB2 },
-{ 0x0DBC, 0x0DBC }, { 0x0DBE, 0x0DBF }, { 0x0DC7, 0x0DC9 },
-{ 0x0DCB, 0x0DCE }, { 0x0DD5, 0x0DD5 }, { 0x0DD7, 0x0DD7 },
-{ 0x0DE0, 0x0DF1 }, { 0x0DF5, 0x0E00 }, { 0x0E3B, 0x0E3E },
-{ 0x0E5C, 0x0E80 }, { 0x0E83, 0x0E83 }, { 0x0E85, 0x0E86 },
-{ 0x0E89, 0x0E89 }, { 0x0E8B, 0x0E8C }, { 0x0E8

[PATCH] D109607: [X86] Refactor GetSSETypeAtOffset to fix pr51813

2021-09-18 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

This patch seem to have broken `GCC-C-execute-pr44575` from the 
llvm-test-suite. See http://meinersbur.de:8011/#/builders/76/builds/761 (this 
builder compiles with Polly, but it also crashes without Polly)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109607

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


[clang] 9b889f8 - Thread safety analysis: Warn when demoting locks on back edges

2021-09-18 Thread Aaron Puchert via cfe-commits

Author: Aaron Puchert
Date: 2021-09-18T13:46:55+02:00
New Revision: 9b889f826ff587e9758c80823419512d502e457d

URL: 
https://github.com/llvm/llvm-project/commit/9b889f826ff587e9758c80823419512d502e457d
DIFF: 
https://github.com/llvm/llvm-project/commit/9b889f826ff587e9758c80823419512d502e457d.diff

LOG: Thread safety analysis: Warn when demoting locks on back edges

Previously in D104261 we warned about dropping locks from back edges,
this is the corresponding change for exclusive/shared joins. If we're
entering the loop with an exclusive change, which is then relaxed to a
shared lock before we loop back, we have already analyzed the loop body
with the stronger exclusive lock and thus might have false positives.

There is a minor non-observable change: we modify the exit lock set of a
function, but since that isn't used further it doesn't change anything.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D106713

Added: 


Modified: 
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 5b2c882c4235..41a55f9579bd 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1050,7 +1050,7 @@ class ThreadSafetyAnalyzer {
   const CFGBlock* PredBlock,
   const CFGBlock *CurrBlock);
 
-  bool join(const FactEntry &a, const FactEntry &b);
+  bool join(const FactEntry &a, const FactEntry &b, bool CanModify);
 
   void intersectAndWarn(FactSet &EntrySet, const FactSet &ExitSet,
 SourceLocation JoinLoc, LockErrorKind EntryLEK,
@@ -2188,25 +2188,28 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
   }
 }
 
-/// Given two facts merging on a join point, decide whether to warn and which
-/// one to keep.
+/// Given two facts merging on a join point, possibly warn and decide whether 
to
+/// keep or replace.
 ///
-/// \return  false if we should keep \p A, true if we should keep \p B.
-bool ThreadSafetyAnalyzer::join(const FactEntry &A, const FactEntry &B) {
+/// \param CanModify Whether we can replace \p A by \p B.
+/// \return  false if we should keep \p A, true if we should take \p B.
+bool ThreadSafetyAnalyzer::join(const FactEntry &A, const FactEntry &B,
+bool CanModify) {
   if (A.kind() != B.kind()) {
 // For managed capabilities, the destructor should unlock in the right mode
 // anyway. For asserted capabilities no unlocking is needed.
 if ((A.managed() || A.asserted()) && (B.managed() || B.asserted())) {
-  // The shared capability subsumes the exclusive capability.
-  return B.kind() == LK_Shared;
-} else {
-  Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), 
A.loc());
-  // Take the exclusive capability to reduce further warnings.
-  return B.kind() == LK_Exclusive;
+  // The shared capability subsumes the exclusive capability, if possible.
+  bool ShouldTakeB = B.kind() == LK_Shared;
+  if (CanModify || !ShouldTakeB)
+return ShouldTakeB;
 }
+Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc());
+// Take the exclusive capability to reduce further warnings.
+return CanModify && B.kind() == LK_Exclusive;
   } else {
 // The non-asserted capability is the one we want to track.
-return A.asserted() && !B.asserted();
+return CanModify && A.asserted() && !B.asserted();
   }
 }
 
@@ -2237,8 +2240,8 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet 
&EntrySet,
 
 FactSet::iterator EntryIt = EntrySet.findLockIter(FactMan, ExitFact);
 if (EntryIt != EntrySet.end()) {
-  if (join(FactMan[*EntryIt], ExitFact) &&
-  EntryLEK == LEK_LockedSomePredecessors)
+  if (join(FactMan[*EntryIt], ExitFact,
+   EntryLEK != LEK_LockedSomeLoopIterations))
 *EntryIt = Fact;
 } else if (!ExitFact.managed()) {
   ExitFact.handleRemovalFromIntersection(ExitSet, FactMan, JoinLoc,

diff  --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp 
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index e9d41da80517..125a1958c8ba 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2789,6 +2789,25 @@ void loopRelease() {
   }
 }
 
+void loopPromote() {
+  RelockableMutexLock scope(&mu, SharedTraits{});
+  for (unsigned i = 1; i < 10; ++i) {
+x = 1; // expected-warning {{writing variable 'x' requires holding mutex 
'mu' exclusively}}
+if (i == 5)
+  scope.PromoteShared();
+  }
+}
+
+void loopDemote() {
+  RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{the 
other acquisition of mutex 'mu' is here}}
+  // We have to warn on this join point despite the lock being man

[PATCH] D106713: Thread safety analysis: Warn when demoting locks on back edges

2021-09-18 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9b889f826ff5: Thread safety analysis: Warn when demoting 
locks on back edges (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106713

Files:
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2789,6 +2789,25 @@
   }
 }
 
+void loopPromote() {
+  RelockableMutexLock scope(&mu, SharedTraits{});
+  for (unsigned i = 1; i < 10; ++i) {
+x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+if (i == 5)
+  scope.PromoteShared();
+  }
+}
+
+void loopDemote() {
+  RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{the other acquisition of mutex 'mu' is here}}
+  // We have to warn on this join point despite the lock being managed ...
+  for (unsigned i = 1; i < 10; ++i) {
+x = 1; // ... because we might miss that this doesn't always happen under exclusive lock.
+if (i == 5)
+  scope.DemoteExclusive(); // expected-warning {{mutex 'mu' is acquired exclusively and shared in the same scope}}
+  }
+}
+
 void loopAcquireContinue() {
   RelockableMutexLock scope(&mu, DeferTraits{});
   for (unsigned i = 1; i < 10; ++i) {
@@ -2812,6 +2831,29 @@
   }
 }
 
+void loopPromoteContinue() {
+  RelockableMutexLock scope(&mu, SharedTraits{});
+  for (unsigned i = 1; i < 10; ++i) {
+x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+if (i == 5) {
+  scope.PromoteShared();
+  continue;
+}
+  }
+}
+
+void loopDemoteContinue() {
+  RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{the other acquisition of mutex 'mu' is here}}
+  // We have to warn on this join point despite the lock being managed ...
+  for (unsigned i = 1; i < 10; ++i) {
+x = 1; // ... because we might miss that this doesn't always happen under exclusive lock.
+if (i == 5) {
+  scope.DemoteExclusive(); // expected-warning {{mutex 'mu' is acquired exclusively and shared in the same scope}}
+  continue;
+}
+  }
+}
+
 void exclusiveSharedJoin() {
   RelockableMutexLock scope(&mu, DeferTraits{});
   if (b)
Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1050,7 +1050,7 @@
   const CFGBlock* PredBlock,
   const CFGBlock *CurrBlock);
 
-  bool join(const FactEntry &a, const FactEntry &b);
+  bool join(const FactEntry &a, const FactEntry &b, bool CanModify);
 
   void intersectAndWarn(FactSet &EntrySet, const FactSet &ExitSet,
 SourceLocation JoinLoc, LockErrorKind EntryLEK,
@@ -2188,25 +2188,28 @@
   }
 }
 
-/// Given two facts merging on a join point, decide whether to warn and which
-/// one to keep.
+/// Given two facts merging on a join point, possibly warn and decide whether to
+/// keep or replace.
 ///
-/// \return  false if we should keep \p A, true if we should keep \p B.
-bool ThreadSafetyAnalyzer::join(const FactEntry &A, const FactEntry &B) {
+/// \param CanModify Whether we can replace \p A by \p B.
+/// \return  false if we should keep \p A, true if we should take \p B.
+bool ThreadSafetyAnalyzer::join(const FactEntry &A, const FactEntry &B,
+bool CanModify) {
   if (A.kind() != B.kind()) {
 // For managed capabilities, the destructor should unlock in the right mode
 // anyway. For asserted capabilities no unlocking is needed.
 if ((A.managed() || A.asserted()) && (B.managed() || B.asserted())) {
-  // The shared capability subsumes the exclusive capability.
-  return B.kind() == LK_Shared;
-} else {
-  Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc());
-  // Take the exclusive capability to reduce further warnings.
-  return B.kind() == LK_Exclusive;
+  // The shared capability subsumes the exclusive capability, if possible.
+  bool ShouldTakeB = B.kind() == LK_Shared;
+  if (CanModify || !ShouldTakeB)
+return ShouldTakeB;
 }
+Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc());
+// Take the exclusive capability to reduce further warnings.
+return CanModify && B.kind() == LK_Exclusive;
   } else {
 // The non-asserted capability is the one we want to track.
-return A.asserted() && !B.asserted();
+return CanModify && A.asserted() && !B.asserted();
   }
 }
 
@@ -2237,8 +2240,8 @@
 
 

[PATCH] D106715: Thread safety analysis: Drop special block handling

2021-09-18 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106715

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


[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D109951#3007425 , @guitard0g wrote:

> When looking at test case suggestions, I happened upon another problem that 
> occurs when handling preprocessor directives. The following code is properly 
> formatted (following llvm style, with ColumnLimit=0):
>
>   SomeClass::SomeClass()
> : a{a},
>   b{b} {}
>
> However this code:
>
>   Foo::Foo()
>   : x{x},
>   #if DEBUG
> y{y},
>   #endif
> z{z} {}
>
> Is transformed by clang-format (under the same style guidelines) into this:
>
>   Foo::Foo()
>   : x{x},
>   #if DEBUG
> y{y},
>   #endif
> z{z} {
>   }
>
> It seems there is some wonkiness with how it handles preprocessor statements 
> here. I can address this in a another patch.

`ColumnLimit` of 0 seems to work very differently in many occasions.




Comment at: clang/unittests/Format/FormatTest.cpp:19308
+   "#if WINDOWS\n"
+   "#if DEBUG\n"
+   ", b{0}\n"

And now I'm curious, if there is a line between the nested PPs?

```
SomeClass::Constructot()
  : a{}
#if X
, b{}
#if Y
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109951

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


[PATCH] D106713: Thread safety analysis: Warn when demoting locks on back edges

2021-09-18 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

@aaron.ballman, since this is reintroducing some warnings after the relaxation 
in D102026 , should we bring this to Clang 13?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106713

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


[clang] 31cca21 - Revert "OpenMP 5.0 metadirective"

2021-09-18 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2021-09-18T09:10:37-04:00
New Revision: 31cca215654ac5eca01796248696b8d710a5cffc

URL: 
https://github.com/llvm/llvm-project/commit/31cca215654ac5eca01796248696b8d710a5cffc
DIFF: 
https://github.com/llvm/llvm-project/commit/31cca215654ac5eca01796248696b8d710a5cffc.diff

LOG: Revert "OpenMP 5.0 metadirective"

This reverts commit c7d7b98e5263472f05b2f3cb767b5d16e1349e9a.
Breaks tests on macOS, see comment on https://reviews.llvm.org/D91944

Added: 


Modified: 
clang/include/clang-c/Index.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/StmtOpenMP.h
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/StmtNodes.td
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtOpenMP.cpp
clang/lib/AST/StmtPrinter.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaExceptionSpec.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/CXCursor.cpp
flang/lib/Semantics/check-omp-structure.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td

Removed: 
clang/test/OpenMP/metadirective_ast_print.c
clang/test/OpenMP/metadirective_device_kind_codegen.c
clang/test/OpenMP/metadirective_device_kind_codegen.cpp
clang/test/OpenMP/metadirective_empty.cpp
clang/test/OpenMP/metadirective_implementation_codegen.c
clang/test/OpenMP/metadirective_implementation_codegen.cpp
clang/test/OpenMP/metadirective_messages.cpp



diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index b49acf6b58543..8afd4c9ff1d05 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -2592,11 +2592,7 @@ enum CXCursorKind {
*/
   CXCursor_OMPUnrollDirective = 293,
 
-  /** OpenMP metadirective directive.
-   */
-  CXCursor_OMPMetaDirective = 294,
-
-  CXCursor_LastStmt = CXCursor_OMPMetaDirective,
+  CXCursor_LastStmt = CXCursor_OMPUnrollDirective,
 
   /**
* Cursor that represents the translation unit itself.

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 9b261e8540dac..9bfa5b9c23260 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2842,9 +2842,6 @@ 
RecursiveASTVisitor::TraverseOMPLoopDirective(OMPLoopDirective *S) {
   return TraverseOMPExecutableDirective(S);
 }
 
-DEF_TRAVERSE_STMT(OMPMetaDirective,
-  { TRY_TO(TraverseOMPExecutableDirective(S)); })
-
 DEF_TRAVERSE_STMT(OMPParallelDirective,
   { TRY_TO(TraverseOMPExecutableDirective(S)); })
 

diff  --git a/clang/include/clang/AST/StmtOpenMP.h 
b/clang/include/clang/AST/StmtOpenMP.h
index f028c3b323986..cd5fa2b94c317 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -5379,44 +5379,6 @@ class OMPMaskedDirective final : public 
OMPExecutableDirective {
   }
 };
 
-/// This represents '#pragma omp metadirective' directive.
-///
-/// \code
-/// #pragma omp metadirective when(user={condition(N>10)}: parallel for)
-/// \endcode
-/// In this example directive '#pragma omp metadirective' has clauses 'when'
-/// with a dynamic user condition to check if a variable 'N > 10'
-///
-class OMPMetaDirective final : public OMPExecutableDirective {
-  friend class ASTStmtReader;
-  friend class OMPExecutableDirective;
-  Stmt *IfStmt;
-
-  OMPMetaDirective(SourceLocation StartLoc, SourceLocation EndLoc)
-  : OMPExecutableDirective(OMPMetaDirectiveClass,
-   llvm::omp::OMPD_metadirective, StartLoc,
-   EndLoc) {}
-  explicit OMPMetaDirective()
-  : OMPExecutableDirective(OMPMetaDirectiveClass,
-   llvm::omp::OMPD_metadirective, SourceLocation(),
-   SourceLocation()) {}
-
-  void setIfStmt(Stmt *S) { IfStmt = S; }
-
-public:
-  static OMPMetaDirective *Create(const ASTContext &C, SourceLocation StartLoc,
-  SourceLocation EndLoc,
-  ArrayRef Clauses,
-  Stmt *AssociatedStmt, Stmt *IfStmt);
-  static OMPMetaDirective *CreateEmpty(const ASTContext &C, unsigned 
NumClauses,
-   EmptyShell);
-  Stmt *getIfStmt() const { ret

[PATCH] D91944: OpenMP 5.0 metadirective

2021-09-18 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

(Reverted for now since mac bots have been broken for 16 hours now.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944

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


[PATCH] D91944: OpenMP 5.0 metadirective

2021-09-18 Thread David Zarzycki via Phabricator via cfe-commits
davezarzycki added a comment.

Also, when this lands again, please ensure that all the tests don't 
accidentally dump build artifacts into the source directory. Specifically, the 
following test was missing `-o -`: clang/test/OpenMP/metadirective_messages.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944

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


[PATCH] D109607: [X86] Refactor GetSSETypeAtOffset to fix pr51813

2021-09-18 Thread Pengfei Wang via Phabricator via cfe-commits
pengfei added a comment.

In D109607#3007860 , @Meinersbur 
wrote:

> This patch seem to have broken `GCC-C-execute-pr44575` from the 
> llvm-test-suite. See http://meinersbur.de:8011/#/builders/76/builds/761 (this 
> builder compiles with Polly, but it also crashes without Polly)

Thanks @Meinersbur for reporting this. Do you have a small reproducer or the 
crash log? I didn't find any detail about the crash on the bot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109607

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


[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-18 Thread Cameron Mulhern via Phabricator via cfe-commits
csmulhern marked an inline comment as done.
csmulhern added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:22414
+
+  EXPECT_EQ("int a = (int)b;", format("int a = (\n"
+  "int\n"

MyDeveloperDay wrote:
> can't this just be verifyFormat but with 3 arguments?
Isn't that true of all the EXPECT_EQ cases (as long as the expected code is 
stable)? This is my first contribution and I'm basically cargo cutting what I 
see as the prevailing style (verifyFormat for the one param / two param case, 
EXPECT_EQ for the three param case). I can imagine that verifyFormat didn't 
support the three parameter case before and so the EXPECT_EQ usage is legacy 
code. Should I convert everything to use verifyFormat?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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


[PATCH] D108560: [clang-tidy] Add support for NOLINTBEGIN ... NOLINTEND comments to suppress clang-tidy warnings over multiple lines

2021-09-18 Thread Salman Javed via Phabricator via cfe-commits
salman-javed-nz marked 5 inline comments as done.
salman-javed-nz added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp:6
+
+// NOLINTEND
+class B1 { B1(int i); };

aaron.ballman wrote:
> salman-javed-nz wrote:
> > aaron.ballman wrote:
> > > Do you think this should be diagnosed as a sign of user confusion with 
> > > the markings?
> > For a stray `NOLINTEND` like this one, I don't think so. The original 
> > warning is still raised, so I see this as clang-tidy failing safe. The user 
> > is forced to fix their mistake before the warning goes away.
> > 
> > The consequences are of the same severity as misusing the existing `NOLINT` 
> > and `NOLINTNEXTLINE` markers, e.g. putting `NOLINT` on the wrong line, or 
> > adding a blank line after `NOLINTNEXTLINE`.
> Hmm, I'm not yet convinced we don't want to diagnose this situation. I agree 
> that the behavior of *other* diagnostics is good (the user still gets those 
> diagnostics because no range has been suppressed). But it seems like the 
> programmer made a mistake if they don't balance the begin and end markers. I 
> don't think this causes major issues, but I think the code is a bit harder to 
> read because someone who spots the end marker may try looking for the begin 
> marker that doesn't exist.
> 
> I suppose there's a small chance that a stray END may surprise users for more 
> than just code readability -- consider a file with a stray end marker where 
> the user wants to lazily suppress the end file by putting `NOLINTBEGIN` at 
> the head of the file and `NOLINTEND` at the end of the file -- the stray 
> `NOLINTEND` in the middle interrupts the full range.
I see your point. Clang-Tidy should alert the user about any stray 
`NOLINTBEGIN` or `NOLINTEND` markers it sees, not leave it up to the user to 
find them themselves. Not diagnosing this is only going to create more headache 
for the user in the long run.



Comment at: 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp:86
+
+// NOLINTBEGIN
+class H1 { H1(int i); };

aaron.ballman wrote:
> salman-javed-nz wrote:
> > aaron.ballman wrote:
> > > Should this be diagnosed as user confusion?
> > > 
> > > My concern in both of these cases isn't so much that someone writes this 
> > > intentionally, but that one of the begin/end pair gets removed 
> > > accidentally when refactoring. Helping the user to identify *where* the 
> > > unmatched delimiters are seems like it could be user-friendly behavior.
> > The consequences of this one are higher, as there is the potential to 
> > suppress warnings unintentionally and allow clang-tidy rule violations to 
> > go undetected. I agree that something more could be done here.
> > 
> > I can think of two improvements:
> > 
> > 1. In `LineIsMarkedWithNOLINT()`, when a `NOLINTBEGIN` is found, only 
> > return true if the corresponding `NOLINTEND` is found as well. Raise the 
> > original warning if the `NOLINTEND` is omitted.
> > 
> > 2. Raise an additional warning regarding the unmatched pair of delimiters. 
> > Some guidance on how to implement it would be appreciated. In the call 
> > stack of the `LineIsMarkedWithNOLINT()` function, I can't see any exposed 
> > functionality to generate new diagnostics on the fly. Would a new 
> > clang-tidy check be the place to implement this?
> That's a good question -- I don't know that I would expect 
> `LineIsMarkedWithNOLINT()` to generate a diagnostic, but it's the obvious 
> place for checking the validity of the markers. Naively, I would not expect 
> to have to run a linter to check my lint markings, I would expect the linting 
> tool to do that for me.
> 
> Would it make sense for `shouldSuppressDiagnostic()` to take a container of 
> diagnostics generated (so `LineIsMarkedWithNOLINT()` has a place to store 
> diagnostics), and `ClangTidyDiagnosticConsumer::HandleDiagnostic()` then 
> checks whether the container is empty and if not, emits the additional 
> diagnostics from there?
Thank you for the suggestion. That is exactly what I have implemented in my 
latest patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108560

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


[PATCH] D108560: [clang-tidy] Add support for NOLINTBEGIN ... NOLINTEND comments to suppress clang-tidy warnings over multiple lines

2021-09-18 Thread Salman Javed via Phabricator via cfe-commits
salman-javed-nz updated this revision to Diff 373416.
salman-javed-nz marked 2 inline comments as done.
salman-javed-nz added a comment.

`shouldSuppressDiagnostic()`:

- Changed to take a container of diagnostics as an argument. If it finds any 
stray `NOLINTBEGIN`/`NOLINTEND` markers, it creates a diagnostic regarding the 
stray marker and places it in the container.

`HandleDiagnostic()`:

- Emits any diagnostics returned by `shouldSuppressDiagnostic()`.

New unit tests:

- `test\clang-tidy\infrastructure\nolintbeginend-begin-without-end.cpp`
- `test\clang-tidy\infrastructure\nolintbeginend-end-without-begin.cpp`.
- `test\clang-tidy\infrastructure\nolintbeginend-mismatched-delims.cpp`.

`IsNOLINTFound()`:

- Bug fix. `IsNOLINTFound("NOLINT", Str)` returns true when `Str` is 
`"NOLINTNEXTLINE"`. This is because the text search finds `"NOLINT"` as a 
substring of `"NOLINTNEXTLINE"`.
- Added test case in `test\clang-tidy\infrastructure\nolint.cpp`.

`LineIsMarkedWithNOLINT()`:

- Bug fix. `NOLINTNEXTLINE`s on the very first line of a file are ignored. This 
is due to `rsplit('\n\').second` returning a blank string when there are no 
more newline chars to split on.
- Added test case in `test\clang-tidy\infrastructure\nolintnextline.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108560

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/error_in_include.inc
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/nolint_in_include.inc
  clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-without-end.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-end-without-begin.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
@@ -1,49 +1,51 @@
+// NOLINTNEXTLINE
 class A { A(int i); };
+
+class B { B(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 // NOLINTNEXTLINE
-class B { B(int i); };
+class C { C(int i); };
 
 // NOLINTNEXTLINE(for-some-other-check)
-class C { C(int i); };
+class D { D(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 // NOLINTNEXTLINE(*)
-class C1 { C1(int i); };
+class D1 { D1(int i); };
 
 // NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
-class C2 { C2(int i); };
+class D2 { D2(int i); };
 
 // NOLINTNEXTLINE(google-explicit-constructor)
-class C3 { C3(int i); };
+class D3 { D3(int i); };
 
 // NOLINTNEXTLINE(some-check, google-explicit-constructor)
-class C4 { C4(int i); };
+class D4 { D4(int i); };
 
 // NOLINTNEXTLINE without-brackets-skip-all, another-check
-class C5 { C5(int i); };
-
+class D5 { D5(int i); };
 
 // NOLINTNEXTLINE
 
-class D { D(int i); };
+class E { E(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 // NOLINTNEXTLINE
 //
-class E { E(int i); };
+class F { F(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 #define MACRO(X) class X { X(int i); };
-MACRO(F)
+MACRO(G)
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
 // NOLINTNEXTLINE
-MACRO(G)
+MACRO(H)
 
-#define MACRO_NOARG class H { H(int i); };
+#define MACRO_NOARG class I { I(int i); };
 // NOLINTNEXTLINE
 MACRO_NOARG
 
-// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
+// CHECK-MESSAGES: Suppressed 9 warnings (9 NOLINT)
 
 // RUN: %check_clang_tidy %s google-explicit-constructor %t --
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s google-explicit-constructor %t -- --header-filter=.* -system-headers -- -isystem %S/Inputs/nolintbeginend
+
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN
+class B1 { B1(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTEND
+// NOLINTBEGIN
+class B2 { B

[clang] 000875c - OpenMP 5.0 metadirective

2021-09-18 Thread via cfe-commits

Author: alokmishra.besu
Date: 2021-09-18T13:40:44-05:00
New Revision: 000875c127c7f2ae10a954bd592c1658e5912cf1

URL: 
https://github.com/llvm/llvm-project/commit/000875c127c7f2ae10a954bd592c1658e5912cf1
DIFF: 
https://github.com/llvm/llvm-project/commit/000875c127c7f2ae10a954bd592c1658e5912cf1.diff

LOG: OpenMP 5.0 metadirective

This patch supports OpenMP 5.0 metadirective features.
It is implemented keeping the OpenMP 5.1 features like dynamic user condition 
in mind.

A new function, getBestWhenMatchForContext, is defined in 
llvm/Frontend/OpenMP/OMPContext.h

Currently this function return the index of the when clause with the highest 
score from the ones applicable in the Context.
But this function is declared with an array which can be used in OpenMP 5.1 
implementation to select all the valid when clauses which can be resolved in 
runtime. Currently this array is set to null by default and its implementation 
is left for future.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D91944

Added: 
clang/test/OpenMP/metadirective_ast_print.c
clang/test/OpenMP/metadirective_device_kind_codegen.c
clang/test/OpenMP/metadirective_device_kind_codegen.cpp
clang/test/OpenMP/metadirective_empty.cpp
clang/test/OpenMP/metadirective_implementation_codegen.c
clang/test/OpenMP/metadirective_implementation_codegen.cpp
clang/test/OpenMP/metadirective_messages.cpp

Modified: 
clang/include/clang-c/Index.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/StmtOpenMP.h
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/StmtNodes.td
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtOpenMP.cpp
clang/lib/AST/StmtPrinter.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaExceptionSpec.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/CXCursor.cpp
flang/lib/Semantics/check-omp-structure.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td

Removed: 




diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 8afd4c9ff1d05..b49acf6b58543 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -2592,7 +2592,11 @@ enum CXCursorKind {
*/
   CXCursor_OMPUnrollDirective = 293,
 
-  CXCursor_LastStmt = CXCursor_OMPUnrollDirective,
+  /** OpenMP metadirective directive.
+   */
+  CXCursor_OMPMetaDirective = 294,
+
+  CXCursor_LastStmt = CXCursor_OMPMetaDirective,
 
   /**
* Cursor that represents the translation unit itself.

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 9bfa5b9c23260..9b261e8540dac 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2842,6 +2842,9 @@ 
RecursiveASTVisitor::TraverseOMPLoopDirective(OMPLoopDirective *S) {
   return TraverseOMPExecutableDirective(S);
 }
 
+DEF_TRAVERSE_STMT(OMPMetaDirective,
+  { TRY_TO(TraverseOMPExecutableDirective(S)); })
+
 DEF_TRAVERSE_STMT(OMPParallelDirective,
   { TRY_TO(TraverseOMPExecutableDirective(S)); })
 

diff  --git a/clang/include/clang/AST/StmtOpenMP.h 
b/clang/include/clang/AST/StmtOpenMP.h
index cd5fa2b94c317..f028c3b323986 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -5379,6 +5379,44 @@ class OMPMaskedDirective final : public 
OMPExecutableDirective {
   }
 };
 
+/// This represents '#pragma omp metadirective' directive.
+///
+/// \code
+/// #pragma omp metadirective when(user={condition(N>10)}: parallel for)
+/// \endcode
+/// In this example directive '#pragma omp metadirective' has clauses 'when'
+/// with a dynamic user condition to check if a variable 'N > 10'
+///
+class OMPMetaDirective final : public OMPExecutableDirective {
+  friend class ASTStmtReader;
+  friend class OMPExecutableDirective;
+  Stmt *IfStmt;
+
+  OMPMetaDirective(SourceLocation StartLoc, SourceLocation EndLoc)
+  : OMPExecutableDirective(OMPMetaDirectiveClass,
+   llvm::omp::OMPD_metadirective, StartLoc,
+   EndLoc) {}
+  explicit OMPMetaDirective()
+  : OMPExecutableDirective(OMPMetaDirectiveClass,
+   llvm::omp::OMPD_metadirectiv

[PATCH] D109799: [RISCV] add Half-precision test for clang

2021-09-18 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

This patch is really large and touches tests for integer operations. Can you 
just pre-commit the removal of lines with just comment characters. No need to 
review for that. That should reduce the noise in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109799

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


[PATCH] D110029: [OpenMP][Offloading] Use bitset to indicate execution mode instead of value

2021-09-18 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, jhuber6.
Herald added subscribers: ormris, guansong, hiraditya, yaxunl.
tianshilei1992 requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1.
Herald added projects: clang, OpenMP, LLVM.

The execution mode of a kernel is stored in a global variable, whose value 
means:

- 0 - SPMD mode
- 1 - indicates generic mode
- 2 - SPMD mode execution with generic mode semantics

We are going to add support for SIMD execution mode. It will be come with 
another
execution mode, such as SIMD-generic mode. As a result, this value-based 
indicator
is not flexible.

This patch changes to bitset based solution to encode execution mode. Each
position is:
[0] - generic mode
[1] - SPMD mode
[2] - SIMD mode (will be added later)

In this way, `0x1` is generic mode, `0x2` is SPMD mode, and `0x3` is SPMD mode
execution with generic mode semantics. In the future after we add the support 
for
SIMD mode, `0b1xx` will be in SIMD mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110029

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll
  llvm/test/Transforms/OpenMP/is_spmd_exec_mode_fold.ll
  llvm/test/Transforms/OpenMP/spmdization.ll
  llvm/test/Transforms/OpenMP/spmdization_assumes.ll
  llvm/test/Transforms/OpenMP/spmdization_guarding.ll
  openmp/libomptarget/plugins/cuda/src/rtl.cpp

Index: openmp/libomptarget/plugins/cuda/src/rtl.cpp
===
--- openmp/libomptarget/plugins/cuda/src/rtl.cpp
+++ openmp/libomptarget/plugins/cuda/src/rtl.cpp
@@ -71,12 +71,10 @@
   std::vector<__tgt_offload_entry> Entries;
 };
 
-enum ExecutionModeType {
-  SPMD, // constructors, destructors,
-// combined constructs (`teams distribute parallel for [simd]`)
-  GENERIC,  // everything else
-  SPMD_GENERIC, // Generic kernel with SPMD execution
-  NONE
+enum ExecutionModeMaskType {
+  SPMD = 0x2,// constructors, destructors, combined constructs (`teams
+ // distribute parallel for [simd]`)
+  GENERIC = 0x1, // everything else
 };
 
 /// Use a single entity to encode a kernel and a set of flags.
@@ -84,9 +82,9 @@
   CUfunction Func;
 
   // execution mode of kernel
-  // 0 - SPMD mode (without master warp)
-  // 1 - Generic mode (with master warp)
-  // 2 - SPMD mode execution with Generic mode semantics.
+  // ExecutionMode | 0x2 - SPMD mode (without master warp)
+  // ExecutionMode | 0x1 - Generic mode (with master warp)
+  // ExecutionMode | 0x3 - SPMD mode execution with Generic mode semantics.
   int8_t ExecutionMode;
 
   /// Maximal number of threads per block for this kernel.
@@ -866,7 +864,7 @@
  DPxPTR(E - HostBegin), E->name, DPxPTR(Func));
 
   // default value GENERIC (in case symbol is missing from cubin file)
-  int8_t ExecModeVal = ExecutionModeType::GENERIC;
+  int8_t ExecModeVal = 1;
   std::string ExecModeNameStr(E->name);
   ExecModeNameStr += "_exec_mode";
   const char *ExecModeName = ExecModeNameStr.c_str();
@@ -889,12 +887,6 @@
   CUDA_ERR_STRING(Err);
   return nullptr;
 }
-
-if (ExecModeVal < 0 || ExecModeVal > 2) {
-  DP("Error wrong exec_mode value specified in cubin file: %d\n",
- ExecModeVal);
-  return nullptr;
-}
   } else {
 DP("Loading global exec_mode '%s' - symbol missing, using default "
"value GENERIC (1)\n",
@@ -1135,13 +1127,22 @@
 unsigned int CudaBlocksPerGrid;
 if (TeamNum <= 0) {
   if (LoopTripCount > 0 && EnvNumTeams < 0) {
-if (KernelInfo->ExecutionMode == SPMD) {
+if (KernelInfo->ExecutionMode & ExecutionModeMaskType::SPMD &&
+KernelInfo->ExecutionMode & ExecutionModeMaskType::GENERIC) {
+  // If we reach this point, then we are executing a kernel that was
+  // transformed from Generic-mode to SPMD-mode. This kernel has
+  // SPMD-mode execution, but needs its blocks to be scheduled
+  // differently because the current loop trip count only applies to the
+  // `teams distribute` region and will create var too few blocks using
+  // the regular SPMD-mode method.
+  CudaBlocksPerGrid = LoopTripCount;
+} else if (KernelInfo->ExecutionMode & ExecutionModeMaskType::SPMD) {
   // We have a combined construct, i.e. `target teams distribute
   // parallel for [simd]`. We launch so many teams so that each thread
   // will execute one iteration of the l