[PATCH] D157146: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

The diff is preferable:)

  --- tools/clang/docs/html/ClangCommandLineReference.html2023-08-12 
22:10:23.876913374 -0700
  +++ tools/clang/docs/html/ClangCommandLineReference.html.old2023-08-12 
22:09:58.900516809 -0700
  @@ -94,8 +94,18 @@
   Static analyzer options
   Fortran compilation options
   Linker 
options
  -clang-cl options
  -clang-dxc options
  +clang-cl options
  +
  +clang-cl compile-only options
  +
  +/M 
group
  +/volatile group
  +
  +
  +clang-cl ignored options
  +
  +
  +clang-dxc options
   
   
   
  @@ -7868,9 +7878,21 @@
   
  
   Set multiple /O flags at once; e.g. ‘/O2y-’ for ‘/O2 /Oy-’
  +
  +clang-cl 
compile-only options¶
  +
  +/M 
group¶
  +
  +
  +/volatile 
group¶
  +
  +
  +
  +clang-cl 
ignored options¶
  +
   
   
  -clang-dxc 
options¶
  +clang-dxc 
options¶
   dxc compatibility options
   
   


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157146

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


[PATCH] D157146: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/utils/TableGen/ClangOptionDocEmitter.cpp:43
 
+bool hasFlag(const Record *Option, StringRef OptionFlag) {
+  for (const Record *Flag : Option->getValueAsListOfDefs("Flags"))

For new functions, consider using `static` for internal linkage functions even 
if they are surrounded by an anonymous namespace, per 
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157146

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


[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Actually, I am not sure changing `%p` to create multiple raw profile files 
should be the default.
Currently, thanks to online profile merging we get just one raw profile file 
(though counters before fork may be doubly incremented).

Using `%p` should not magically change the behavior and I am unsure the result 
is better for PGO. 
Perhaps you can add a new format specifier or API for the proposed behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155290

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


[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D155290#4576643 , @qiongsiwu1 
wrote:

> In D155290#4574797 , @MaskRay wrote:
>
>> In D155290#4572965 , @qiongsiwu1 
>> wrote:
>>
>>> Ping for review.
>>>
>>> If there are no comments on whether we should add `-lpthread` for platforms 
>>> other than AIX, I will land this patch soon, and let the official buildbots 
>>> check for problems. If there are problems, I will let the bots run a bit 
>>> (probably a day or so) before reverting to catch as many platforms I need 
>>> to fix as possible, revert, and fix the patch.
>>>
>>> Thanks!
>>
>> I will take a look as well but please don't land this `-lpthread` change for 
>> non-AIX platforms. I will think about the implication.
>
> Thanks @MaskRay ! I am putting this patch on hold till I hear back. The patch 
> does not alter non-AIX platforms at the moment.

The `pthread_atfork` undefined reference will cause a problem using glibc<2.34 
where `pthread_atfork` lives in libpthread instead of libc.
I think non-AIX-non-Windows systems can live with an extra `-lpthread`. For ELF 
systems (not Apple's among non-AIX-non-Windows systems), `-lpthread` must be 
after `libclang_rt.profile.a`.




Comment at: compiler-rt/test/profile/Posix/instrprof-fork.c:22
+  pid = fork();
+  if (!pid) {
+printf("%ld.profraw\n", (long)getpid());

MaskRay wrote:
> parent and child have the same logic. Use:
> ```
> if (pid == -1)
>   return 1;
> printf("%ld.profraw\n", (long)getpid());
> ```
Please ignore my previous comment. I think we want to have a deterministic 
parent / child output order. Consider adding a wait for the parent process so 
that we always see the child `printf("%ld.profraw\n", (long)getpid());` output 
before the parent one.

We should inspect more information of the two raw profile files. Create a 
function only called by the parent and another function only called by the 
child. Check their counters.

We also need a test if the child process spawns another process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155290

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


[PATCH] D156237: Complete the implementation of P2361 Unevaluated string literals

2023-08-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:431-436
+if (Expr.isInvalid()) {
+  SawError = true;
+  break;
+} else {
+  Exprs.push_back(Expr.get());
+}

cor3ntin wrote:
> aaron.ballman wrote:
> > 
> Oups, sorry @Fznamznon, I'll fix that!
It seems Aaron's request to move the code out of the `else` was not acted on.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2337
+// Emits the list of arguments that should be parsed as unevaluated string
+// literals for each attributes
+static void emitClangAttrUnevaluatedStringLiteralList(RecordKeeper ,

Minor nit: Grammar



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2345
+uint32_t Bits = 0;
+for (uint32_t N = 0; N < Args.size(); ++N) {
+  Bits |= (isStringLiteralArgument(Args[N]) << N);

Do we know that `Args.size()` is less than 32?



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2349-2350
+  if (isVariadicStringLiteralArgument(Args[N])) {
+for (; N < sizeof(uint32_t) * CHAR_BIT; ++N)
+  Bits |= (1 << N);
+break;

```
#include 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156237

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


[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: smeenai.
rnk added inline comments.



Comment at: clang/test/Driver/cl-options.c:567
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck 
-check-prefix=Z7_gdwarf %s

I think Meta folks depend on a mode that emits both codeview and DWARF. 
+@smeenai


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157794

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


[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-08-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D155610#4575579 , @cor3ntin wrote:

> @hubert.reinterpretcast It does not, Unicode characters are only escaped in 
> Diagnostics.cpp, and I think this is what we want.

Thanks @cor3ntin for the insight. I agree that this is a separate concern that 
also applies to static assert messages.


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

https://reviews.llvm.org/D155610

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


[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-08-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899
+  uint32_t CodePoint = 
static_cast(V.getInt().getZExtValue());
+  PrintCharLiteralPrefix(BTy->getKind(), OS);
+  OS << '\'';

cor3ntin wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > cor3ntin wrote:
> > > > Looking at the diagnostics, I don't think it makes sense to print a 
> > > > prefix here. You could just leave that part out.
> > > Why is removing the prefix better? The types can matter (characters 
> > > outside the basic character set are allowed to have negative `char` 
> > > values). Also, moving forward, the value of a character need not be the 
> > > same in the various encodings.
> > Some fun with signedness (imagine a more realistic example with 
> > `ISO-8859-1` ordinary character encoding with a signed `char` type):
> > ```
> > $ clang -Xclang -fwchar-type=short -xc++ -<<<$'static_assert(L"\\uFF10"[0] 
> > == U\'\\uFF10\');'
> > :1:15: error: static assertion failed due to requirement 
> > 'L"\xFF10"[0] == U'\uff10''
> > 1 | static_assert(L"\uFF10"[0] == U'\uFF10');
> >   |   ^
> > :1:28: note: expression evaluates to ''0' (0xFF10) == '0' (0xFF10)'
> > 1 | static_assert(L"\uFF10"[0] == U'\uFF10');
> >   |   ~^~~~
> > 1 error generated.
> > Return:  0x01:1   Fri Aug 11 23:49:02 2023 EDT
> > ```
> Either we care about the actual character - ie `'a'`, or it's value (ie 
> `42`). The motivation for the current patch is to add the value to the 
> diagnostic message.
> I'm also concerned about mixing things that are are are not lexical elements 
> in the diagnostics
Maybe the //motivation// for the current patch is to add the value, but what it 
does (for wide characters as defined in C) is to add the character (and 
obfuscate the value).

Observe the status quo (https://godbolt.org/z/Wc6nKvTMn):
```
note: expression evaluates to '-240 == 65296'
```

From the output higher up (with this patch), we see two "identical" characters 
and values (due to lack of decimal value output). With decimal value output 
added, it will still be potentially confusing why the two identical characters 
have different values (without some sort of type annotation).

I admit that the confusion arises in the status quo treatment of `signed char` 
and `unsigned char`. I hope I am using the word correctly when I say that it is 
ironic that the patch breaks in one context what it seeks to fix in another.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16869
+/// The code point needs to be zero-extended to 32-bits.
+void ConvertCharToString(uint32_t CodePoint, const BuiltinType *BTy,
+ unsigned TyWidth, llvm::raw_ostream ) {

It does not seem that the first parameter expects a `CodePoint` argument in all 
cases. For `Char_S`, `Char_U`, and `Char8`, it seems the function wants to 
treat the input as a UTF-8 code unit.

I suggest changing the argument to be clearly a code unit (and potentially 
treat it as a code point value as appropriate later in the function).

Also: The function should probably be declared as having static linkage.
Additionally: The function does not "convert" in the language semantic sense. 
`WriteCharacterValueDescriptionForDisplay` might be a better name.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16876
+  // other types.
+  if (CodePoint <= UCHAR_MAX) {
+StringRef Escaped = escapeCStyle(CodePoint);

For types other than `Char_S`, `Char_U`, and `Char8`, this fails to treat the 
C1 Controls and Latin-1 Supplement characters as Unicode code points. It looks 
like test coverage for these cases are missing.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16930-16936
+case BuiltinType::Char_S:
+case BuiltinType::Char_U:
+case BuiltinType::Char8:
+case BuiltinType::Char16:
+case BuiltinType::Char32:
+case BuiltinType::WChar_S:
+case BuiltinType::WChar_U: {

cor3ntin wrote:
> hubert.reinterpretcast wrote:
> > Add FIXME for `char` and `wchar_t` cases that this assumes Unicode literal 
> > encodings.
> If we wanted such fixme, it should be L1689.
The `ConvertCharToString` has a first parameter called `CodePoint`. With that 
interface[^1], it is sensible to insert conversion from the applicable literal 
encoding to a Unicode code point value here (thus my request for a FIXME here).

You are probably right that the FIXME belongs elsewhere. If you were thinking 
what I am thinking, then I am guessing you meant L16894? That is where the 
`ConvertCharToString` function seems to assume that a `wchar_t` value is 
directly a "code point value". To generate hex escapes, the function needs to 
be passed the original value (including for `char`s, e.g., to handle stray code 

[PATCH] D154382: [ClangRepl] support code completion at a REPL

2023-08-12 Thread Fred Fu via Phabricator via cfe-commits
capfredf added inline comments.



Comment at: clang/include/clang/Frontend/ASTUnit.h:901
+SmallVectorImpl ,
+std::function 
AfterBeginSourceFile = [](CompilerInstance& CI) -> void {});
 

@v.g.vassilev  Not sure if this is the right solution or idiom to extend this 
method.  

we can make this high order function more general, like 
`std::unique_ptr<()>`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154382

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


[PATCH] D154382: [ClangRepl] support code completion at a REPL

2023-08-12 Thread Fred Fu via Phabricator via cfe-commits
capfredf updated this revision to Diff 549658.
capfredf added a comment.

use ASTUnit::codeComplete


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154382

Files:
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Interpreter/CodeCompletion.h
  clang/include/clang/Interpreter/ExternalSource.h
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/CodeCompletion.cpp
  clang/lib/Interpreter/DeviceOffload.cpp
  clang/lib/Interpreter/ExternalSource.cpp
  clang/lib/Interpreter/ExternalSource.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/incrememal-mode-completion-no-error.cpp
  clang/test/CodeCompletion/incremental-top-level.cpp
  clang/tools/clang-repl/ClangRepl.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/CodeCompletionTest.cpp

Index: clang/unittests/Interpreter/CodeCompletionTest.cpp
===
--- /dev/null
+++ clang/unittests/Interpreter/CodeCompletionTest.cpp
@@ -0,0 +1,100 @@
+#include "clang/Interpreter/CodeCompletion.h"
+#include "clang/Interpreter/Interpreter.h"
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "llvm/LineEditor/LineEditor.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+namespace {
+auto CB = clang::IncrementalCompilerBuilder();
+
+static std::unique_ptr createInterpreter() {
+  auto CI = cantFail(CB.CreateCpp());
+  return cantFail(clang::Interpreter::create(std::move(CI)));
+}
+
+static std::vector runComp(clang::Interpreter ,
+llvm::StringRef Prefix,
+llvm::Error ) {
+  auto CI = CB.CreateCpp();
+  if (auto Err = CI.takeError()) {
+ErrR = std::move(Err);
+return {};
+  }
+
+
+  auto Interp = clang::Interpreter::create(std::move(*CI));
+  if (auto Err = Interp.takeError()) {
+// log the error and returns an empty vector;
+ErrR = std::move(Err);
+
+return {};
+  }
+
+  std::vector Results;
+
+  (*Interp)->codeComplete(Prefix, Prefix.size(), MainInterp.getCompilerInstance(), Results);
+
+  std::vector Comps;
+  for (auto c : ConvertToCodeCompleteStrings(Results)) {
+if (c.startswith(Prefix))
+  Comps.push_back(c.substr(Prefix.size()).str());
+  }
+
+  return Comps;
+}
+
+TEST(CodeCompletionTest, Sanity) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int foo = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "f", Err);
+  EXPECT_EQ((size_t)2, comps.size()); // foo and float
+  EXPECT_EQ(comps[0], std::string("oo"));
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, SanityNoneValid) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int foo = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "babanana", Err);
+  EXPECT_EQ((size_t)0, comps.size()); // foo and float
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, TwoDecls) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int application = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  if (auto R = Interp->ParseAndExecute("int apple = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "app", Err);
+  EXPECT_EQ((size_t)2, comps.size());
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, CompFunDeclsNoError) {
+  auto Interp = createInterpreter();
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "void app(", Err);
+  EXPECT_EQ((bool)Err, false);
+}
+
+} // anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -9,6 +9,7 @@
 add_clang_unittest(ClangReplInterpreterTests
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
+  CodeCompletionTest.cpp
   )
 target_link_libraries(ClangReplInterpreterTests PUBLIC
   clangAST
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- 

[PATCH] D157795: [clang] Add missing field to SectionAttr json AST dump

2023-08-12 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision.
serge-sans-paille added a reviewer: aaron.ballman.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157795

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp


Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -15,6 +15,8 @@
 __attribute__((unavailable)) int unavailable_var0;
 __attribute__((unavailable("reason"))) int unavailable_var1;
 
+__attribute__ ((section ("SECTION_NAME"))) int section_var;
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -384,3 +386,50 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 593,
+// CHECK-NEXT:   "line": 18,
+// CHECK-NEXT:   "col": 48,
+// CHECK-NEXT:   "tokLen": 11
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 546,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 593,
+// CHECK-NEXT:"col": 48,
+// CHECK-NEXT:"tokLen": 11
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "section_var",
+// CHECK-NEXT:  "mangledName": "section_var",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "SectionAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 562,
+// CHECK-NEXT:  "col": 17,
+// CHECK-NEXT:  "tokLen": 7
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 585,
+// CHECK-NEXT:  "col": 40,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"section_name": "SECTION_NAME"
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -547,6 +547,10 @@
   JOS.attribute("message", UA->getMessage());
 }
 
+void JSONNodeDumper::VisitSectionAttr(const SectionAttr *SA) {
+  JOS.attribute("section_name", SA->getName());
+}
+
 void JSONNodeDumper::VisitTypedefType(const TypedefType *TT) {
   JOS.attribute("decl", createBareDeclRef(TT->getDecl()));
   if (!TT->typeMatchesDecl())
Index: clang/include/clang/AST/JSONNodeDumper.h
===
--- clang/include/clang/AST/JSONNodeDumper.h
+++ clang/include/clang/AST/JSONNodeDumper.h
@@ -212,6 +212,7 @@
   void VisitCleanupAttr(const CleanupAttr *CA);
   void VisitDeprecatedAttr(const DeprecatedAttr *DA);
   void VisitUnavailableAttr(const UnavailableAttr *UA);
+  void VisitSectionAttr(const SectionAttr *SA);
 
   void VisitTypedefType(const TypedefType *TT);
   void VisitUsingType(const UsingType *TT);


Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -15,6 +15,8 @@
 __attribute__((unavailable)) int unavailable_var0;
 __attribute__((unavailable("reason"))) int unavailable_var1;
 
+__attribute__ ((section ("SECTION_NAME"))) int section_var;
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -384,3 +386,50 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 593,
+// CHECK-NEXT:   "line": 18,
+// CHECK-NEXT:   "col": 48,
+// CHECK-NEXT:   "tokLen": 11
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 546,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 593,
+// CHECK-NEXT:"col": 48,
+// CHECK-NEXT:"tokLen": 11
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "section_var",
+// CHECK-NEXT:  "mangledName": "section_var",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "SectionAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 562,
+// CHECK-NEXT:  "col": 17,
+// 

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

2023-08-12 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 549645.
serge-sans-paille added a comment.

+ test case


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

https://reviews.llvm.org/D157783

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp

Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -8,6 +8,13 @@
   __attribute__((cleanup(cleanup_function))) int var;
 }
 
+__attribute__((deprecated)) int deprecated_var0;
+__attribute__((deprecated("reason"))) int deprecated_var1;
+__attribute__((deprecated("reason", "replacement"))) int deprecated_var2;
+
+__attribute__((unavailable)) int unavailable_var0;
+__attribute__((unavailable("reason"))) int unavailable_var1;
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -139,3 +146,241 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 282,
+// CHECK-NEXT:   "line": 11,
+// CHECK-NEXT:   "col": 33,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 250,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 282,
+// CHECK-NEXT:"col": 33,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var0",
+// CHECK-NEXT:  "mangledName": "deprecated_var0",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "",
+// CHECK-NEXT:"replacement": ""
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 341,
+// CHECK-NEXT:   "line": 12,
+// CHECK-NEXT:   "col": 43,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 299,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 341,
+// CHECK-NEXT:"col": 43,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var1",
+// CHECK-NEXT:  "mangledName": "deprecated_var1",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 314,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 333,
+// CHECK-NEXT:  "col": 35,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "reason",
+// CHECK-NEXT:"replacement": ""
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 415,
+// CHECK-NEXT:   "line": 13,
+// CHECK-NEXT:   "col": 58,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 358,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 415,
+// CHECK-NEXT:"col": 58,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var2",
+// CHECK-NEXT:  "mangledName": "deprecated_var2",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 373,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 407,
+// CHECK-NEXT:  "col": 50,
+// 

[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

2023-08-12 Thread David Pagan via Phabricator via cfe-commits
ddpagan added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.

ABataev wrote:
> ddpagan wrote:
> > ABataev wrote:
> > > ddpagan wrote:
> > > > ABataev wrote:
> > > > > ddpagan wrote:
> > > > > > ABataev wrote:
> > > > > > > It does not match the spec. 
> > > > > > > ```
> > > > > > > For a combined or composite construct, if no 
> > > > > > > directive-name-modifier is specified then the if clause applies 
> > > > > > > to all constituent constructs to which an if clause can apply.
> > > > > > > ```
> > > > > > > So, if(val) should be applied to both target and parallel 
> > > > > > > regions, no?
> > > > > > > It does not match the spec. 
> > > > > > > ```
> > > > > > > For a combined or composite construct, if no 
> > > > > > > directive-name-modifier is specified then the if clause applies 
> > > > > > > to all constituent constructs to which an if clause can apply.
> > > > > > > ```
> > > > > > > So, if(val) should be applied to both target and parallel 
> > > > > > > regions, no?
> > > > > > 
> > > > > > Hi Alexey - Question for you: does revising the comment above at 
> > > > > > lines 1570-1575 to the following text help explain in a better way 
> > > > > > what's being done, and why?
> > > > > > 
> > > > > >   If we are handling a 'target teams distribute parallel for' 
> > > > > > explicitly written
> > > > > >   in the source, and it has an 'if(val)' clause, the if condition 
> > > > > > is applied to
> > > > > >   both 'target' and 'parallel' regions according to
> > > > > >   OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].
> > > > > > 
> > > > > >   However, if we are mapping an explicit 'target teams loop 
> > > > > > if(val)' onto a
> > > > > >   'target teams distribute parallel for if(val)', to preserve the 
> > > > > > 'if' semantics
> > > > > >   as specified by the user with the 'target teams loop', we apply 
> > > > > > it just to
> > > > > >   the 'target' region.
> > > > > It does not match the spec. Why we shall handle it this way?
> > > > You're right, Alexey ... it doesn't match the spec, but here's why we 
> > > > thought this would be an appropriate way to implement 'target teams 
> > > > loop if(val)'. When a user specifies 'if(val)' with a 'target teams 
> > > > loop', their expectation is that its effect will only apply to the 
> > > > 'target' region. Since a 'loop' construct can be implemented in a 
> > > > number of different ways with the freedom granted by the specs 
> > > > description of 'loop' (part of which describes it as being able to be 
> > > > run concurrently), using a 'target teams distribute parallel for' 
> > > > construct is a simple and effective default choice, which is what 
> > > > happens today. 
> > > > target_teams_loop => target_teams_distribute_parallel_for
> > > > Applying the if clause to the parallel region for this case can 
> > > > potentially limit it to one thread, which would hinder performance 
> > > > gains otherwise possible, and presumably wouldn't be what the user 
> > > > wanted/expected.
> > > > 
> > > > The semantics of the spec (OpenMP 5.2 [3.4, if Clause, Semantics, 
> > > > 15-18]) is definitely what should be applied to an explicit instance of 
> > > > target_teams_distribute_parallel_for, but in this case (when mapping 
> > > > target_teams_loop => target_teams_distribute_parallel_for) it seems 
> > > > reasonable to make the choice described above.
> > > I'm not sure this is true users  expectations.
> > You're right in the sense that there is an assumption being made there. So 
> > would you recommend just staying with the semantics as defined in the spec 
> > regardless?
> Yew, better to follow spec defaults.
Ok. Sounds good. Thanks for review and suggestions, Alexey.


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

https://reviews.llvm.org/D157197

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


[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

2023-08-12 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.

ddpagan wrote:
> ABataev wrote:
> > ddpagan wrote:
> > > ABataev wrote:
> > > > ddpagan wrote:
> > > > > ABataev wrote:
> > > > > > It does not match the spec. 
> > > > > > ```
> > > > > > For a combined or composite construct, if no 
> > > > > > directive-name-modifier is specified then the if clause applies to 
> > > > > > all constituent constructs to which an if clause can apply.
> > > > > > ```
> > > > > > So, if(val) should be applied to both target and parallel regions, 
> > > > > > no?
> > > > > > It does not match the spec. 
> > > > > > ```
> > > > > > For a combined or composite construct, if no 
> > > > > > directive-name-modifier is specified then the if clause applies to 
> > > > > > all constituent constructs to which an if clause can apply.
> > > > > > ```
> > > > > > So, if(val) should be applied to both target and parallel regions, 
> > > > > > no?
> > > > > 
> > > > > Hi Alexey - Question for you: does revising the comment above at 
> > > > > lines 1570-1575 to the following text help explain in a better way 
> > > > > what's being done, and why?
> > > > > 
> > > > >   If we are handling a 'target teams distribute parallel for' 
> > > > > explicitly written
> > > > >   in the source, and it has an 'if(val)' clause, the if condition is 
> > > > > applied to
> > > > >   both 'target' and 'parallel' regions according to
> > > > >   OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].
> > > > > 
> > > > >   However, if we are mapping an explicit 'target teams loop if(val)' 
> > > > > onto a
> > > > >   'target teams distribute parallel for if(val)', to preserve the 
> > > > > 'if' semantics
> > > > >   as specified by the user with the 'target teams loop', we apply it 
> > > > > just to
> > > > >   the 'target' region.
> > > > It does not match the spec. Why we shall handle it this way?
> > > You're right, Alexey ... it doesn't match the spec, but here's why we 
> > > thought this would be an appropriate way to implement 'target teams loop 
> > > if(val)'. When a user specifies 'if(val)' with a 'target teams loop', 
> > > their expectation is that its effect will only apply to the 'target' 
> > > region. Since a 'loop' construct can be implemented in a number of 
> > > different ways with the freedom granted by the specs description of 
> > > 'loop' (part of which describes it as being able to be run concurrently), 
> > > using a 'target teams distribute parallel for' construct is a simple and 
> > > effective default choice, which is what happens today. 
> > > target_teams_loop => target_teams_distribute_parallel_for
> > > Applying the if clause to the parallel region for this case can 
> > > potentially limit it to one thread, which would hinder performance gains 
> > > otherwise possible, and presumably wouldn't be what the user 
> > > wanted/expected.
> > > 
> > > The semantics of the spec (OpenMP 5.2 [3.4, if Clause, Semantics, 15-18]) 
> > > is definitely what should be applied to an explicit instance of 
> > > target_teams_distribute_parallel_for, but in this case (when mapping 
> > > target_teams_loop => target_teams_distribute_parallel_for) it seems 
> > > reasonable to make the choice described above.
> > I'm not sure this is true users  expectations.
> You're right in the sense that there is an assumption being made there. So 
> would you recommend just staying with the semantics as defined in the spec 
> regardless?
Yew, better to follow spec defaults.


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

https://reviews.llvm.org/D157197

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


[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

2023-08-12 Thread David Pagan via Phabricator via cfe-commits
ddpagan added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.

ABataev wrote:
> ddpagan wrote:
> > ABataev wrote:
> > > ddpagan wrote:
> > > > ABataev wrote:
> > > > > It does not match the spec. 
> > > > > ```
> > > > > For a combined or composite construct, if no directive-name-modifier 
> > > > > is specified then the if clause applies to all constituent constructs 
> > > > > to which an if clause can apply.
> > > > > ```
> > > > > So, if(val) should be applied to both target and parallel regions, no?
> > > > > It does not match the spec. 
> > > > > ```
> > > > > For a combined or composite construct, if no directive-name-modifier 
> > > > > is specified then the if clause applies to all constituent constructs 
> > > > > to which an if clause can apply.
> > > > > ```
> > > > > So, if(val) should be applied to both target and parallel regions, no?
> > > > 
> > > > Hi Alexey - Question for you: does revising the comment above at lines 
> > > > 1570-1575 to the following text help explain in a better way what's 
> > > > being done, and why?
> > > > 
> > > >   If we are handling a 'target teams distribute parallel for' 
> > > > explicitly written
> > > >   in the source, and it has an 'if(val)' clause, the if condition is 
> > > > applied to
> > > >   both 'target' and 'parallel' regions according to
> > > >   OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].
> > > > 
> > > >   However, if we are mapping an explicit 'target teams loop if(val)' 
> > > > onto a
> > > >   'target teams distribute parallel for if(val)', to preserve the 'if' 
> > > > semantics
> > > >   as specified by the user with the 'target teams loop', we apply it 
> > > > just to
> > > >   the 'target' region.
> > > It does not match the spec. Why we shall handle it this way?
> > You're right, Alexey ... it doesn't match the spec, but here's why we 
> > thought this would be an appropriate way to implement 'target teams loop 
> > if(val)'. When a user specifies 'if(val)' with a 'target teams loop', their 
> > expectation is that its effect will only apply to the 'target' region. 
> > Since a 'loop' construct can be implemented in a number of different ways 
> > with the freedom granted by the specs description of 'loop' (part of which 
> > describes it as being able to be run concurrently), using a 'target teams 
> > distribute parallel for' construct is a simple and effective default 
> > choice, which is what happens today. 
> > target_teams_loop => target_teams_distribute_parallel_for
> > Applying the if clause to the parallel region for this case can potentially 
> > limit it to one thread, which would hinder performance gains otherwise 
> > possible, and presumably wouldn't be what the user wanted/expected.
> > 
> > The semantics of the spec (OpenMP 5.2 [3.4, if Clause, Semantics, 15-18]) 
> > is definitely what should be applied to an explicit instance of 
> > target_teams_distribute_parallel_for, but in this case (when mapping 
> > target_teams_loop => target_teams_distribute_parallel_for) it seems 
> > reasonable to make the choice described above.
> I'm not sure this is true users  expectations.
You're right in the sense that there is an assumption being made there. So 
would you recommend just staying with the semantics as defined in the spec 
regardless?


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

https://reviews.llvm.org/D157197

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


[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

2023-08-12 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.

ddpagan wrote:
> ABataev wrote:
> > ddpagan wrote:
> > > ABataev wrote:
> > > > It does not match the spec. 
> > > > ```
> > > > For a combined or composite construct, if no directive-name-modifier is 
> > > > specified then the if clause applies to all constituent constructs to 
> > > > which an if clause can apply.
> > > > ```
> > > > So, if(val) should be applied to both target and parallel regions, no?
> > > > It does not match the spec. 
> > > > ```
> > > > For a combined or composite construct, if no directive-name-modifier is 
> > > > specified then the if clause applies to all constituent constructs to 
> > > > which an if clause can apply.
> > > > ```
> > > > So, if(val) should be applied to both target and parallel regions, no?
> > > 
> > > Hi Alexey - Question for you: does revising the comment above at lines 
> > > 1570-1575 to the following text help explain in a better way what's being 
> > > done, and why?
> > > 
> > >   If we are handling a 'target teams distribute parallel for' explicitly 
> > > written
> > >   in the source, and it has an 'if(val)' clause, the if condition is 
> > > applied to
> > >   both 'target' and 'parallel' regions according to
> > >   OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].
> > > 
> > >   However, if we are mapping an explicit 'target teams loop if(val)' onto 
> > > a
> > >   'target teams distribute parallel for if(val)', to preserve the 'if' 
> > > semantics
> > >   as specified by the user with the 'target teams loop', we apply it just 
> > > to
> > >   the 'target' region.
> > It does not match the spec. Why we shall handle it this way?
> You're right, Alexey ... it doesn't match the spec, but here's why we thought 
> this would be an appropriate way to implement 'target teams loop if(val)'. 
> When a user specifies 'if(val)' with a 'target teams loop', their expectation 
> is that its effect will only apply to the 'target' region. Since a 'loop' 
> construct can be implemented in a number of different ways with the freedom 
> granted by the specs description of 'loop' (part of which describes it as 
> being able to be run concurrently), using a 'target teams distribute parallel 
> for' construct is a simple and effective default choice, which is what 
> happens today. 
> target_teams_loop => target_teams_distribute_parallel_for
> Applying the if clause to the parallel region for this case can potentially 
> limit it to one thread, which would hinder performance gains otherwise 
> possible, and presumably wouldn't be what the user wanted/expected.
> 
> The semantics of the spec (OpenMP 5.2 [3.4, if Clause, Semantics, 15-18]) is 
> definitely what should be applied to an explicit instance of 
> target_teams_distribute_parallel_for, but in this case (when mapping 
> target_teams_loop => target_teams_distribute_parallel_for) it seems 
> reasonable to make the choice described above.
I'm not sure this is true users  expectations.


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

https://reviews.llvm.org/D157197

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


[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

2023-08-12 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 549644.
serge-sans-paille retitled this revision from "[clang] Add reason and 
replacement fields to DeprecatedAttr json AST dump" to "[clang] Add rmissing 
fields to DeprecatedAttr and UnavailableAttr json AST dump".
serge-sans-paille added a comment.

+ Unavailable attr


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

https://reviews.llvm.org/D157783

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp

Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -8,6 +8,13 @@
   __attribute__((cleanup(cleanup_function))) int var;
 }
 
+__attribute__((deprecated)) int deprecated_var0;
+__attribute__((deprecated("reason"))) int deprecated_var1;
+__attribute__((deprecated("reason", "replacement"))) int deprecated_var2;
+
+__attribute__((unavailable)) int unavailable_var0;
+__attribute__((unavailable("reason"))) int unavailable_var1;
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -139,3 +146,147 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 282,
+// CHECK-NEXT:   "line": 11,
+// CHECK-NEXT:   "col": 33,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 250,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 282,
+// CHECK-NEXT:"col": 33,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var0",
+// CHECK-NEXT:  "mangledName": "deprecated_var0",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "",
+// CHECK-NEXT:"replacement": ""
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 341,
+// CHECK-NEXT:   "line": 12,
+// CHECK-NEXT:   "col": 43,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 299,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 341,
+// CHECK-NEXT:"col": 43,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var1",
+// CHECK-NEXT:  "mangledName": "deprecated_var1",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 314,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 333,
+// CHECK-NEXT:  "col": 35,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "reason",
+// CHECK-NEXT:"replacement": ""
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 415,
+// CHECK-NEXT:   "line": 13,
+// CHECK-NEXT:   "col": 58,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 358,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 415,
+// CHECK-NEXT:"col": 58,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var2",
+// CHECK-NEXT:  "mangledName": "deprecated_var2",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT: 

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-12 Thread Justin Bogner via Phabricator via cfe-commits
bogner created this revision.
bogner added reviewers: rnk, jansvoboda11, MaskRay.
Herald added subscribers: jeroen.dobbelaere, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The -g flag has been selecting whether to emit dwarf or codeview based
on the target ABI since 2018, so simply aliasing these flags does the
right thing for clang-cl.

This moves some code from Clang::ConstructJob to renderDebugOptions to
make things a little clearer now that we don't need to keep track of
whether we're doing codeview or not in multiple places, and also
combines the duplicate handling of the cl vs clang handling of jmc
flags as a result.

This is mostly NFC, but some -cc1 flags may be rendered in a slightly
different order because of the code that was moved around.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157794

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Clang.h
  clang/test/Driver/cl-options.c
  clang/test/Driver/working-directory.c

Index: clang/test/Driver/working-directory.c
===
--- clang/test/Driver/working-directory.c
+++ clang/test/Driver/working-directory.c
@@ -6,6 +6,6 @@
 
 // CHECK_NO_FILE: no such file or directory: 'no_such_file.cpp'
 
+// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
 // CHECK_WORKS: "-coverage-notes-file" "{{[^"]+}}test{{/|}}Driver{{/|}}Inputs{{/|}}pchfile.gcno"
 // CHECK_WORKS: "-working-directory" "{{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
-// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -564,16 +564,10 @@
 // RUN: %clang_cl /Brepro /Brepro- /c '-###' -- %s 2>&1 | FileCheck -check-prefix=Brepro_ %s
 // Brepro_: "-mincremental-linker-compatible"
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
-// Z7_gdwarf: "-gcodeview"
+// RUN: %clang_cl -gdwarf /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
+// Z7_gdwarf-NOT: "-gcodeview"
 // Z7_gdwarf: "-debug-info-kind=constructor"
 // Z7_gdwarf: "-dwarf-version=
 
Index: clang/lib/Driver/ToolChains/Clang.h
===
--- clang/lib/Driver/ToolChains/Clang.h
+++ clang/lib/Driver/ToolChains/Clang.h
@@ -90,9 +90,7 @@
  RewriteKind rewrite) const;
 
   void AddClangCLArgs(const llvm::opt::ArgList , types::ID InputType,
-  llvm::opt::ArgStringList ,
-  llvm::codegenoptions::DebugInfoKind *DebugInfoKind,
-  bool *EmitCodeView) const;
+  llvm::opt::ArgStringList ) const;
 
   mutable std::unique_ptr CompilationDatabase = nullptr;
   void DumpCompilationDatabase(Compilation , StringRef Filename,
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4204,8 +4204,8 @@
 
 static void
 renderDebugOptions(const ToolChain , const Driver , const llvm::Triple ,
-   const ArgList , bool EmitCodeView, bool IRInput,
-   ArgStringList ,
+   const ArgList , bool IRInput, ArgStringList ,
+   const InputInfo ,
llvm::codegenoptions::DebugInfoKind ,
DwarfFissionKind ) {
   if (Args.hasFlag(options::OPT_fdebug_info_for_profiling,
@@ -4282,6 +4282,7 @@
   if (const Arg *A = getDwarfNArg(Args))
 EmitDwarf = checkDebugInfoOption(A, Args, D, TC);
 
+  bool EmitCodeView = false;
   if (const Arg *A = Args.getLastArg(options::OPT_gcodeview))
 EmitCodeView = checkDebugInfoOption(A, Args, D, TC);
 
@@ -4518,6 +4519,34 @@
 
   renderDwarfFormat(D, T, Args, CmdArgs, EffectiveDWARFVersion);
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
+
+
+  // This controls whether or not we perform JustMyCode 

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

2023-08-12 Thread Justin Bogner via Phabricator via cfe-commits
bogner added a comment.

In D157151#4582109 , @awarzynski 
wrote:

> I think that it would be good to replace `Default` with e.g.
>
> - `Clang`, or
> - `ClangDriver`, or
> - `ClangCompilerDriver`.
>
> Or, at least, to make the meaning of `Default` much clearer (through e.g. 
> comments). In general, I feel that `Default` is skewed towards this notion 
> that `clang` (the compiler driver tool) is the main client of `clangDriver`. 
> That used to be the case, but with Flang's driver also implemented in terms 
> of `clangDriver` , that's no longer the case. Also, the long term goal is to 
> extract the driver library out of Clang. One day :)

This is a little bit complicated by the fact that the Option library is already 
partially extracted out of clang (and used for other drivers, like lld and 
lldb). The "Default" visibility is defined in llvm's Option library, so calling 
it something like "Clang" would be pretty confusing for users that aren't the 
clang Driver library. I suppose one option would be to throw something like 
`using ClangDriver = llvm::Driver::Default;` in Options.h inside the 
`clang::driver::options` namespace, and then using the alias in Options.td. 
Would that help?

> Also, note that `Default` options will be available in both `clang` and 
> `flang-new`. That's the case today (so not something affected by your 
> changes). Ideally, Flang should be able to disable those _Clang options_. 
> That's not possible ATM. Contrary to Flang, Clang can disable _Flang options_ 
> with `FlangOnlyOption`. There is no such flag for Flang ATM, but I think that 
> we could re-use `Default` for that. WDYT?

`Default` options are either options that don't specify `Vis` at all or 
explicitly mention `Default`. They are not visible in `flang-new` after this 
change unless they also specify `FlangOption`. Specifically, instead of having 
to deal with `FlangOnlyOption` we explicitly opt in to default. Some examples:

  def default : Flag<["-"], "a">; //visible only in invocations of `clang`
  def default_explicit : Flag<["-"], "b">, Vis<[Default]>; //visible only in 
invocations of `clang`
  def flang_only : Flag<["-"], "c">, Vis<[FlangOption]>; // only `flang-new`
  def flang_and_clang : Flag<["-"], "d">, Vis<[Default, FlangOption]>; // Both 
`clang` and `flang-new`
  def flang_fc1 : Flag<["-"], "e">, Vis<[FC1Option]>; // `flang-new -fc1`, but 
not the `flang-new` driver or `clang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157151

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


[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

2023-08-12 Thread Ian Anderson via Phabricator via cfe-commits
iana created this revision.
iana added reviewers: zatrazz, jfb, MaskRay, eli.friedman, aaron.ballman, 
rsmith, steplong, efriedma, jyknight, erichkeane.
Herald added a subscriber: ributzka.
Herald added a project: All.
iana requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Apple needs a __need_ macro for va_list. Add one, and also ones for 
va_start/va_arg/va_end, __va_copy, va_copy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157793

Files:
  clang/lib/Headers/stdarg.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c

Index: clang/test/Headers/stdargneeds.c
===
--- /dev/null
+++ clang/test/Headers/stdargneeds.c
@@ -0,0 +1,91 @@
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds0.c
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds1.c
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds2.c
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds3.c
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds4.c
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds5.c
+
+// Split the file so that the "implicitly declaring library function" errors get repeated.
+// Use C89 to verify that __need_ can be used to get types that wouldn't normally be available.
+
+//--- stdargneeds0.c
+static void f(int p, ...) {
+__gnuc_va_list g; // expected-error{{undeclared}}
+va_list v; // expected-error{{undeclared}}
+va_start(v, p); // expected-error{{implicit}} expected-note{{va_start}} expected-error{{undeclared}}
+int i = va_arg(v, int); // expected-error{{implicit}} expected-error{{expression}} expected-error{{undeclared}}
+va_end(v); // expected-error{{implicit}} expected-note{{va_end}} expected-error{{undeclared}}
+__va_copy(g, v); // expected-error{{implicit}} expected-error{{undeclared}} expected-error{{undeclared}}
+va_copy(g, v); // expected-error{{implicit}} expected-note{{va_copy}} expected-error{{undeclared}} expected-error{{undeclared}}
+}
+
+//--- stdargneeds1.c
+#define __need___va_list
+#include 
+static void f(int p, ...) {
+__gnuc_va_list g;
+va_list v; // expected-error{{undeclared}}
+va_start(v, p); // expected-error{{implicit}} expected-note{{va_start}} expected-error{{undeclared}}
+int i = va_arg(v, int); // expected-error{{implicit}} expected-error{{expression}} expected-error{{undeclared}}
+va_end(v); // expected-error{{implicit}} expected-note{{va_end}} expected-error{{undeclared}}
+__va_copy(g, v); // expected-error{{implicit}} expected-error{{undeclared}}
+va_copy(g, v); // expected-error{{implicit}} expected-note{{va_copy}} expected-error{{undeclared}}
+}
+
+//--- stdargneeds2.c
+#define __need_va_list
+#include 
+static void f(int p, ...) {
+__gnuc_va_list g; // expected-error{{undeclared}}
+va_list v;
+va_start(v, p); // expected-error{{implicit}} expected-note{{va_start}}
+int i = va_arg(v, int); // expected-error{{implicit}} expected-error{{expression}}
+va_end(v); // expected-error{{implicit}} expected-note{{va_end}}
+__va_copy(g, v); // expected-error{{implicit}} expected-error{{undeclared}}
+va_copy(g, v); // expected-error{{implicit}} expected-note{{va_copy}} expected-error{{undeclared}}
+}
+
+//--- stdargneeds3.c
+#define __need_va_list
+#define __need_va_arg
+#include 
+static void f(int p, ...) {
+__gnuc_va_list g; // expected-error{{undeclared}}
+va_list v;
+va_start(v, p);
+int i = va_arg(v, int);
+va_end(v);
+__va_copy(g, v); // expected-error{{implicit}} expected-error{{undeclared}}
+va_copy(g, v); // expected-error{{implicit}} expected-note{{va_copy}} expected-error{{undeclared}}
+}
+
+//--- stdargneeds4.c
+#define __need___va_list
+#define __need_va_list
+#define __need___va_copy
+#include 
+static void f(int p, ...) {
+__gnuc_va_list g;
+va_list v;
+va_start(v, p); // expected-error{{implicit}} expected-note{{va_start}}
+int i = va_arg(v, int); // expected-error{{implicit}} expected-error{{expression}}
+va_end(v); // expected-error{{implicit}} expected-note{{va_end}}
+__va_copy(g, v);
+va_copy(g, v); // expected-error{{implicit}} expected-note{{va_copy}}
+}
+
+//--- stdargneeds5.c
+#define __need___va_list
+#define __need_va_list
+#define __need_va_copy
+#include 
+static void f(int p, ...) {
+__gnuc_va_list g;
+va_list v;
+va_start(v, p); // 

[PATCH] D157651: [RISCV] Rewrite CheckInvalidVLENandLMUL to avoid floating point.

2023-08-12 Thread Craig Topper 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 rGee6befe26437: [RISCV] Rewrite CheckInvalidVLENandLMUL to 
avoid floating point. (authored by craig.topper).

Changed prior to commit:
  https://reviews.llvm.org/D157651?vs=549634=549636#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157651

Files:
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4474,14 +4474,24 @@
   assert((EGW == 128 || EGW == 256) && "EGW can only be 128 or 256 bits");
 
   // LMUL * VLEN >= EGW
-  uint64_t ElemSize = Type->isRVVType(32, false) ? 32 : 64;
-  uint64_t ElemCount = Type->isRVVType(1) ? 1 :
-   Type->isRVVType(2) ? 2 :
-   Type->isRVVType(4) ? 4 :
-   Type->isRVVType(8) ? 8 :
-   16;
-  float Lmul = (float)(ElemSize * ElemCount) / llvm::RISCV::RVVBitsPerBlock;
-  uint64_t MinRequiredVLEN = std::max(EGW / Lmul, (float)ElemSize);
+  unsigned ElemSize = Type->isRVVType(32, false) ? 32 : 64;
+  unsigned MinElemCount = Type->isRVVType(1)   ? 1
+  : Type->isRVVType(2) ? 2
+  : Type->isRVVType(4) ? 4
+  : Type->isRVVType(8) ? 8
+   : 16;
+
+  unsigned EGS = EGW / ElemSize;
+  // If EGS is less than or equal to the minimum number of elements, then the
+  // type is valid.
+  if (EGS <= MinElemCount)
+return false;
+
+  // Otherwise, we need vscale to be at least EGS / MinElemCont.
+  assert(EGS % MinElemCount == 0);
+  unsigned VScaleFactor = EGS / MinElemCount;
+  // Vscale is VLEN/RVVBitsPerBlock.
+  unsigned MinRequiredVLEN = VScaleFactor * llvm::RISCV::RVVBitsPerBlock;
   std::string RequiredExt = "zvl" + std::to_string(MinRequiredVLEN) + "b";
   if (!TI.hasFeature(RequiredExt))
 return S.Diag(TheCall->getBeginLoc(),


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4474,14 +4474,24 @@
   assert((EGW == 128 || EGW == 256) && "EGW can only be 128 or 256 bits");
 
   // LMUL * VLEN >= EGW
-  uint64_t ElemSize = Type->isRVVType(32, false) ? 32 : 64;
-  uint64_t ElemCount = Type->isRVVType(1) ? 1 :
-   Type->isRVVType(2) ? 2 :
-   Type->isRVVType(4) ? 4 :
-   Type->isRVVType(8) ? 8 :
-   16;
-  float Lmul = (float)(ElemSize * ElemCount) / llvm::RISCV::RVVBitsPerBlock;
-  uint64_t MinRequiredVLEN = std::max(EGW / Lmul, (float)ElemSize);
+  unsigned ElemSize = Type->isRVVType(32, false) ? 32 : 64;
+  unsigned MinElemCount = Type->isRVVType(1)   ? 1
+  : Type->isRVVType(2) ? 2
+  : Type->isRVVType(4) ? 4
+  : Type->isRVVType(8) ? 8
+   : 16;
+
+  unsigned EGS = EGW / ElemSize;
+  // If EGS is less than or equal to the minimum number of elements, then the
+  // type is valid.
+  if (EGS <= MinElemCount)
+return false;
+
+  // Otherwise, we need vscale to be at least EGS / MinElemCont.
+  assert(EGS % MinElemCount == 0);
+  unsigned VScaleFactor = EGS / MinElemCount;
+  // Vscale is VLEN/RVVBitsPerBlock.
+  unsigned MinRequiredVLEN = VScaleFactor * llvm::RISCV::RVVBitsPerBlock;
   std::string RequiredExt = "zvl" + std::to_string(MinRequiredVLEN) + "b";
   if (!TI.hasFeature(RequiredExt))
 return S.Diag(TheCall->getBeginLoc(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] ee6befe - [RISCV] Rewrite CheckInvalidVLENandLMUL to avoid floating point.

2023-08-12 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2023-08-12T11:14:51-07:00
New Revision: ee6befe26437034be84bf4785127b397afb9dfcb

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

LOG: [RISCV] Rewrite CheckInvalidVLENandLMUL to avoid floating point.

This avoids needing an FP value to represent LMUL.

Reviewed By: 4vtomat

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index dc45e8d61cea73..8c3abc29315e7b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -4474,14 +4474,24 @@ static bool CheckInvalidVLENandLMUL(const TargetInfo 
, CallExpr *TheCall,
   assert((EGW == 128 || EGW == 256) && "EGW can only be 128 or 256 bits");
 
   // LMUL * VLEN >= EGW
-  uint64_t ElemSize = Type->isRVVType(32, false) ? 32 : 64;
-  uint64_t ElemCount = Type->isRVVType(1) ? 1 :
-   Type->isRVVType(2) ? 2 :
-   Type->isRVVType(4) ? 4 :
-   Type->isRVVType(8) ? 8 :
-   16;
-  float Lmul = (float)(ElemSize * ElemCount) / llvm::RISCV::RVVBitsPerBlock;
-  uint64_t MinRequiredVLEN = std::max(EGW / Lmul, (float)ElemSize);
+  unsigned ElemSize = Type->isRVVType(32, false) ? 32 : 64;
+  unsigned MinElemCount = Type->isRVVType(1)   ? 1
+  : Type->isRVVType(2) ? 2
+  : Type->isRVVType(4) ? 4
+  : Type->isRVVType(8) ? 8
+   : 16;
+
+  unsigned EGS = EGW / ElemSize;
+  // If EGS is less than or equal to the minimum number of elements, then the
+  // type is valid.
+  if (EGS <= MinElemCount)
+return false;
+
+  // Otherwise, we need vscale to be at least EGS / MinElemCont.
+  assert(EGS % MinElemCount == 0);
+  unsigned VScaleFactor = EGS / MinElemCount;
+  // Vscale is VLEN/RVVBitsPerBlock.
+  unsigned MinRequiredVLEN = VScaleFactor * llvm::RISCV::RVVBitsPerBlock;
   std::string RequiredExt = "zvl" + std::to_string(MinRequiredVLEN) + "b";
   if (!TI.hasFeature(RequiredExt))
 return S.Diag(TheCall->getBeginLoc(),



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


[PATCH] D157651: [RISCV] Rewrite CheckInvalidVLENandLMUL to avoid floating point.

2023-08-12 Thread Craig Topper via Phabricator via cfe-commits
craig.topper updated this revision to Diff 549634.
craig.topper added a comment.

Revise coment, rename variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157651

Files:
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4474,14 +4474,22 @@
   assert((EGW == 128 || EGW == 256) && "EGW can only be 128 or 256 bits");
 
   // LMUL * VLEN >= EGW
-  uint64_t ElemSize = Type->isRVVType(32, false) ? 32 : 64;
-  uint64_t ElemCount = Type->isRVVType(1) ? 1 :
-   Type->isRVVType(2) ? 2 :
-   Type->isRVVType(4) ? 4 :
-   Type->isRVVType(8) ? 8 :
-   16;
-  float Lmul = (float)(ElemSize * ElemCount) / llvm::RISCV::RVVBitsPerBlock;
-  uint64_t MinRequiredVLEN = std::max(EGW / Lmul, (float)ElemSize);
+  unsigned ElemSize = Type->isRVVType(32, false) ? 32 : 64;
+  unsigned MinElemCount = Type->isRVVType(1)   ? 1
+  : Type->isRVVType(2) ? 2
+  : Type->isRVVType(4) ? 4
+  : Type->isRVVType(8) ? 8
+   : 16;
+
+  unsigned EGS = EGW / ElemSize;
+  // If EGS is less than or equal to the minimum number of elements we're done.
+  if (EGS <= MinElemCount)
+return false;
+
+  // We need vscale to be at least this value.
+  unsigned VScaleFactor = EGS / MinElemCount;
+  // Vscale is VLEN/RVVBitsPerBlock.
+  unsigned MinRequiredVLEN = VScaleFactor * llvm::RISCV::RVVBitsPerBlock;
   std::string RequiredExt = "zvl" + std::to_string(MinRequiredVLEN) + "b";
   if (!TI.hasFeature(RequiredExt))
 return S.Diag(TheCall->getBeginLoc(),


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4474,14 +4474,22 @@
   assert((EGW == 128 || EGW == 256) && "EGW can only be 128 or 256 bits");
 
   // LMUL * VLEN >= EGW
-  uint64_t ElemSize = Type->isRVVType(32, false) ? 32 : 64;
-  uint64_t ElemCount = Type->isRVVType(1) ? 1 :
-   Type->isRVVType(2) ? 2 :
-   Type->isRVVType(4) ? 4 :
-   Type->isRVVType(8) ? 8 :
-   16;
-  float Lmul = (float)(ElemSize * ElemCount) / llvm::RISCV::RVVBitsPerBlock;
-  uint64_t MinRequiredVLEN = std::max(EGW / Lmul, (float)ElemSize);
+  unsigned ElemSize = Type->isRVVType(32, false) ? 32 : 64;
+  unsigned MinElemCount = Type->isRVVType(1)   ? 1
+  : Type->isRVVType(2) ? 2
+  : Type->isRVVType(4) ? 4
+  : Type->isRVVType(8) ? 8
+   : 16;
+
+  unsigned EGS = EGW / ElemSize;
+  // If EGS is less than or equal to the minimum number of elements we're done.
+  if (EGS <= MinElemCount)
+return false;
+
+  // We need vscale to be at least this value.
+  unsigned VScaleFactor = EGS / MinElemCount;
+  // Vscale is VLEN/RVVBitsPerBlock.
+  unsigned MinRequiredVLEN = VScaleFactor * llvm::RISCV::RVVBitsPerBlock;
   std::string RequiredExt = "zvl" + std::to_string(MinRequiredVLEN) + "b";
   if (!TI.hasFeature(RequiredExt))
 return S.Diag(TheCall->getBeginLoc(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157474: [RISCV] Add missing Xsfvcp extension check in clang sema

2023-08-12 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157474

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


[PATCH] D157693: [clang][doc] Mark _Float16 is support natively when Zfh is available

2023-08-12 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:815
   * X86 (if SSE2 is available; natively if AVX512-FP16 is also available)
+  * RISC-V (natively if Zfh is available)
 

Zhinx also


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157693

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


[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

2023-08-12 Thread David Pagan via Phabricator via cfe-commits
ddpagan added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.

ABataev wrote:
> ddpagan wrote:
> > ABataev wrote:
> > > It does not match the spec. 
> > > ```
> > > For a combined or composite construct, if no directive-name-modifier is 
> > > specified then the if clause applies to all constituent constructs to 
> > > which an if clause can apply.
> > > ```
> > > So, if(val) should be applied to both target and parallel regions, no?
> > > It does not match the spec. 
> > > ```
> > > For a combined or composite construct, if no directive-name-modifier is 
> > > specified then the if clause applies to all constituent constructs to 
> > > which an if clause can apply.
> > > ```
> > > So, if(val) should be applied to both target and parallel regions, no?
> > 
> > Hi Alexey - Question for you: does revising the comment above at lines 
> > 1570-1575 to the following text help explain in a better way what's being 
> > done, and why?
> > 
> >   If we are handling a 'target teams distribute parallel for' explicitly 
> > written
> >   in the source, and it has an 'if(val)' clause, the if condition is 
> > applied to
> >   both 'target' and 'parallel' regions according to
> >   OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].
> > 
> >   However, if we are mapping an explicit 'target teams loop if(val)' onto a
> >   'target teams distribute parallel for if(val)', to preserve the 'if' 
> > semantics
> >   as specified by the user with the 'target teams loop', we apply it just to
> >   the 'target' region.
> It does not match the spec. Why we shall handle it this way?
You're right, Alexey ... it doesn't match the spec, but here's why we thought 
this would be an appropriate way to implement 'target teams loop if(val)'. When 
a user specifies 'if(val)' with a 'target teams loop', their expectation is 
that its effect will only apply to the 'target' region. Since a 'loop' 
construct can be implemented in a number of different ways with the freedom 
granted by the specs description of 'loop' (part of which describes it as being 
able to be run concurrently), using a 'target teams distribute parallel for' 
construct is a simple and effective default choice, which is what happens 
today. 
target_teams_loop => target_teams_distribute_parallel_for
Applying the if clause to the parallel region for this case can potentially 
limit it to one thread, which would hinder performance gains otherwise 
possible, and presumably wouldn't be what the user wanted/expected.

The semantics of the spec (OpenMP 5.2 [3.4, if Clause, Semantics, 15-18]) is 
definitely what should be applied to an explicit instance of 
target_teams_distribute_parallel_for, but in this case (when mapping 
target_teams_loop => target_teams_distribute_parallel_for) it seems reasonable 
to make the choice described above.


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

https://reviews.llvm.org/D157197

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


[PATCH] D157773: [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

2023-08-12 Thread Piotr Zegar via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc3da99275a52: [clang-tidy] fix None tmpdir when exporting 
fixes in run-clang-tidy (authored by 5chmidti, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157773

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -389,6 +389,8 @@
 clang_apply_replacements_binary = find_binary(
 args.clang_apply_replacements_binary, "clang-apply-replacements", 
build_path
 )
+
+if args.fix or (yaml and args.export_fixes):
 tmpdir = tempfile.mkdtemp()
 
 try:


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -389,6 +389,8 @@
 clang_apply_replacements_binary = find_binary(
 args.clang_apply_replacements_binary, "clang-apply-replacements", build_path
 )
+
+if args.fix or (yaml and args.export_fixes):
 tmpdir = tempfile.mkdtemp()
 
 try:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] c3da992 - [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

2023-08-12 Thread Piotr Zegar via cfe-commits

Author: Julian Schmidt
Date: 2023-08-12T16:18:33Z
New Revision: c3da99275a520b73235d975017502876e07e3e8e

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

LOG: [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

Differential https://reviews.llvm.org/D145477 removed the check for `(yaml and 
args.export_fixes)` in line 303 to skip looking for the 
`clang-apply-replacements` binary. However, the `tmpdir` variable was set in 
this true branch when exporting fixes and therefore is `None` when invoking 
run-clang-tidy with `run-clang-tidy -p . -export-fixes fixes.yaml`.

Reviewed By: PiotrZSL

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/tool/run-clang-tidy.py

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py 
b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index e0c2d63d2024cf..312d9241cfa57c 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -389,6 +389,8 @@ def main():
 clang_apply_replacements_binary = find_binary(
 args.clang_apply_replacements_binary, "clang-apply-replacements", 
build_path
 )
+
+if args.fix or (yaml and args.export_fixes):
 tmpdir = tempfile.mkdtemp()
 
 try:



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


[PATCH] D157497: feat: Migrate isArch16Bit

2023-08-12 Thread Evgeniy Makarev via Phabricator via cfe-commits
Pivnoy updated this revision to Diff 549621.
Pivnoy added a comment.

Fix some format mistake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/TargetParser/Triple.h
  llvm/include/llvm/TargetParser/TripleUtils.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/TripleUtils.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/unittests/TargetParser/CMakeLists.txt
  llvm/unittests/TargetParser/TripleTest.cpp
  llvm/unittests/TargetParser/TripleUtilsTest.cpp

Index: llvm/unittests/TargetParser/TripleUtilsTest.cpp
===
--- /dev/null
+++ llvm/unittests/TargetParser/TripleUtilsTest.cpp
@@ -0,0 +1,116 @@
+//===--- TripleUtils.cpp - TripleUtils unit tests
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/TargetParser/TripleUtils.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(TripleUtilsTest, CheckArchBitWidth) {
+  Triple T;
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::arm);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::hexagon);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::mips);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::mips64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::msp430);
+  EXPECT_TRUE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::ppc);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::ppc64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::x86);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::x86_64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::amdil);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::amdil64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::hsail);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::hsail64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spir);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spir64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spirv32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spirv64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::sparc);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::sparcel);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+  ;
+
+  T.setArch(Triple::sparcv9);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::wasm32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::wasm64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::avr);
+  EXPECT_TRUE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::lanai);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::riscv32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::riscv64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::csky);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::loongarch32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::loongarch64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::dxil);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::xtensa);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+}
+
+} // namespace
\ No newline at end of file
Index: llvm/unittests/TargetParser/TripleTest.cpp
===
--- llvm/unittests/TargetParser/TripleTest.cpp
+++ llvm/unittests/TargetParser/TripleTest.cpp
@@ -1094,172 +1094,140 @@
 
 TEST(TripleTest, BitWidthPredicates) {
   Triple T;
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::arm);
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::hexagon);
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::mips);
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::mips64);
-  

[PATCH] D157497: feat: Migrate isArch16Bit

2023-08-12 Thread Evgeniy Makarev via Phabricator via cfe-commits
Pivnoy updated this revision to Diff 549620.
Pivnoy added a comment.

Add deprecated attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/TargetParser/Triple.h
  llvm/include/llvm/TargetParser/TripleUtils.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/TripleUtils.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/unittests/TargetParser/CMakeLists.txt
  llvm/unittests/TargetParser/TripleTest.cpp
  llvm/unittests/TargetParser/TripleUtilsTest.cpp

Index: llvm/unittests/TargetParser/TripleUtilsTest.cpp
===
--- /dev/null
+++ llvm/unittests/TargetParser/TripleUtilsTest.cpp
@@ -0,0 +1,116 @@
+//===--- TripleUtils.cpp - TripleUtils unit tests
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/TargetParser/TripleUtils.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(TripleUtilsTest, CheckArchBitWidth) {
+  Triple T;
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::arm);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::hexagon);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::mips);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::mips64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::msp430);
+  EXPECT_TRUE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::ppc);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::ppc64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::x86);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::x86_64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::amdil);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::amdil64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::hsail);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::hsail64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spir);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spir64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spirv32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::spirv64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::sparc);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::sparcel);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+  ;
+
+  T.setArch(Triple::sparcv9);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::wasm32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::wasm64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::avr);
+  EXPECT_TRUE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::lanai);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::riscv32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::riscv64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::csky);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::loongarch32);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::loongarch64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::dxil);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+
+  T.setArch(Triple::xtensa);
+  EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+}
+
+} // namespace
\ No newline at end of file
Index: llvm/unittests/TargetParser/TripleTest.cpp
===
--- llvm/unittests/TargetParser/TripleTest.cpp
+++ llvm/unittests/TargetParser/TripleTest.cpp
@@ -1094,172 +1094,140 @@
 
 TEST(TripleTest, BitWidthPredicates) {
   Triple T;
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::arm);
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::hexagon);
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::mips);
-  EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
   T.setArch(Triple::mips64);
-  

[PATCH] D157786: Add isArch32Bit into TripleUtils

2023-08-12 Thread Evgeniy Makarev via Phabricator via cfe-commits
Pivnoy created this revision.
Herald added subscribers: luke, pmatos, asb, frasercrmck, luismarques, apazos, 
sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, 
simoncook, johnrusso, rbar, fedor.sergeev, kbarton, hiraditya, 
jgravelle-google, arichardson, sbc100, nemanjai, dschuff, emaste.
Herald added a project: All.
Pivnoy requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wangpc, MaskRay, aheejin.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157786

Files:
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/TargetParser/Triple.h
  llvm/include/llvm/TargetParser/TripleUtils.h
  llvm/lib/BinaryFormat/MachO.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ProfileData/InstrProfCorrelator.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
  llvm/lib/Target/LoongArch/LoongArchTargetMachine.cpp
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/TargetParser/Host.cpp
  llvm/lib/TargetParser/Triple.cpp
  llvm/lib/TargetParser/TripleUtils.cpp
  llvm/lib/XRay/InstrumentationMap.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfUtils.cpp
  llvm/unittests/TargetParser/TripleTest.cpp
  llvm/unittests/TargetParser/TripleUtilsTest.cpp

Index: llvm/unittests/TargetParser/TripleUtilsTest.cpp
===
--- llvm/unittests/TargetParser/TripleUtilsTest.cpp
+++ llvm/unittests/TargetParser/TripleUtilsTest.cpp
@@ -17,100 +17,161 @@
 TEST(TripleUtilsTest, CheckArchBitWidth) {
   Triple T;
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
 
   T.setArch(Triple::arm);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::hexagon);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::mips);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::mips64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::msp430);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_TRUE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::ppc);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::ppc64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::x86);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::x86_64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::amdil);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::amdil64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::hsail);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::hsail64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::spir);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::spir64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::spirv32);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::spirv64);
+  EXPECT_FALSE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::sparc);
+  EXPECT_TRUE(llvm::TripleUtils::isArch32Bit(T));
   EXPECT_FALSE(llvm::TripleUtils::isArch16Bit(T));
 
   T.setArch(Triple::sparcel);
+  

[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:94
 static lprofFilename lprofCurFilename = {0,   0, 0, {0}, NULL,
  {0}, 0, 0, 0,   PNS_unknown};
 static int ProfileMergeRequested = 0;

Revert unneeded change.



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:253
+// because the implementation relies on pthread.
+static int ResetNameAtFork = 0;
+

include stdbool.h and use bool



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:1027
 return;
+
   parseAndSetFilename(FilenamePat, PNS_runtime_api, 1);

unneeded change



Comment at: compiler-rt/test/profile/Posix/instrprof-fork.c:4
+// RUN: rm -rf %t.d
+// RUN: mkdir -p %t.d && cd %t.d
+// RUN: %clang_pgogen %s -o %t.exe

I usually use `RUN: rm -rf %t && mkdir %t && cd %t` and then use something like 
`-o t` to place the executable under `%t` as well.



Comment at: compiler-rt/test/profile/Posix/instrprof-fork.c:22
+  pid = fork();
+  if (!pid) {
+printf("%ld.profraw\n", (long)getpid());

parent and child have the same logic. Use:
```
if (pid == -1)
  return 1;
printf("%ld.profraw\n", (long)getpid());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155290

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


[PATCH] D157783: [clang] Add reason and replacement fields to DeprecatedAttr json AST dump

2023-08-12 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision.
serge-sans-paille added a reviewer: aaron.ballman.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157783

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp

Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -8,6 +8,10 @@
   __attribute__((cleanup(cleanup_function))) int var;
 }
 
+__attribute__((deprecated)) int deprecated_var0;
+__attribute__((deprecated("reason"))) int deprecated_var1;
+__attribute__((deprecated("reason", "replacement"))) int deprecated_var2;
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -139,3 +143,147 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 282,
+// CHECK-NEXT:   "line": 11,
+// CHECK-NEXT:   "col": 33,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 250,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 282,
+// CHECK-NEXT:"col": 33,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var0",
+// CHECK-NEXT:  "mangledName": "deprecated_var0",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "",
+// CHECK-NEXT:"replacement": ""
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 341,
+// CHECK-NEXT:   "line": 12,
+// CHECK-NEXT:   "col": 43,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 299,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 341,
+// CHECK-NEXT:"col": 43,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var1",
+// CHECK-NEXT:  "mangledName": "deprecated_var1",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 314,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 333,
+// CHECK-NEXT:  "col": 35,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "reason",
+// CHECK-NEXT:"replacement": ""
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 415,
+// CHECK-NEXT:   "line": 13,
+// CHECK-NEXT:   "col": 58,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 358,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 415,
+// CHECK-NEXT:"col": 58,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var2",
+// CHECK-NEXT:  "mangledName": "deprecated_var2",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 373,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 407,
+// CHECK-NEXT:  "col": 50,
+// 

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

2023-08-12 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

Thanks for trimming this, it's much easier to review! A few more suggestions, 
but nothing major.




Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227-263
+  bool needLocTracking = false;
+
+  if (!opts.OptRecordFile.empty())
+needLocTracking = true;
 
   if (const llvm::opt::Arg *a =
+  args.getLastArg(clang::driver::options::OPT_opt_record_passes)) {

```lang=cpp
  // coment
  if (const llvm::opt::Arg *a =
  args.getLastArg(clang::driver::options::OPT_opt_record_passes))
opts.OptRecordPasses = a->getValue();

  // coment
  if (const llvm::opt::Arg *a =
  args.getLastArg(clang::driver::options::OPT_opt_record_format))
opts.OptRecordFormat = a->getValue();

  // coment
  opts.OptimizationRemark = parseOptimizationRemark(
  diags, args, clang::driver::options::OPT_Rpass_EQ, "pass");

  // coment
  opts.OptimizationRemarkMissed = parseOptimizationRemark(
  diags, args, clang::driver::options::OPT_Rpass_missed_EQ, "pass-missed");

  // coment
  opts.OptimizationRemarkAnalysis = parseOptimizationRemark(
  diags, args, clang::driver::options::OPT_Rpass_analysis_EQ,
  "pass-analysis");

  if (opts.getDebugInfo() == llvm::codegenoptions::NoDebugInfo) {
// If the user requested a flag that requires source locations available in
// the backend, make sure that the backend tracks source location 
information.
bool needLocTracking = !opts.OptRecordFile.empty() || opts.OptRecordPasses 
||
   !opts.OptRecordFormat.empty() ||
   opts.OptimizationRemark.hasValidPattern() ||
   opts.OptimizationRemarkMissed.hasValidPattern() ||
   opts.OptimizationRemarkAnalysis.hasValidPattern();

if (needLocTracking)
  opts.setDebugInfo(llvm::codegenoptions::LocTrackingOnly);
  }
```

I might have made typos when editing this, but hopefully the overall logic 
makes sense. Basically, I am suggesting that "option parsing" is separated from 
the logic for setting up the location tracking in the backend.



Comment at: flang/lib/Frontend/FrontendActions.cpp:978-1005
+if (d.isPassed()) {
+  // Optimization remarks are active only if the -Rpass flag has a regular
+  // expression that matches the name of the pass name in \p D.
+  if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
+emitOptimizationMessage(
+d, clang::diag::remark_fe_backend_optimization_remark);
+

https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code

```lang=cpp
  void
  optimizationRemarkHandler(const llvm::DiagnosticInfoOptimizationBase ) {
if (d.isPassed()) {
  // Optimization remarks are active only if the -Rpass flag has a regular
  // expression that matches the name of the pass name in \p D.
  if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
emitOptimizationMessage(
d, clang::diag::remark_fe_backend_optimization_remark);
  return;
}

if (d.isMissed()) {
  // Missed optimization remarks are active only if the -Rpass-missed
  // flag has a regular expression that matches the name of the pass
  // name in \p D.
  if (codeGenOpts.OptimizationRemarkMissed.patternMatches(d.getPassName()))
emitOptimizationMessage(
d, clang::diag::remark_fe_backend_optimization_remark_missed);
  return;
}

assert(d.isAnalysis() && "Unknown remark type");

bool shouldAlwaysPrint = false;
if (auto *ora = llvm::dyn_cast())
  shouldAlwaysPrint = ora->shouldAlwaysPrint();

if (shouldAlwaysPrint ||
codeGenOpts.OptimizationRemarkAnalysis.patternMatches(
d.getPassName()))
  emitOptimizationMessage(
  d, clang::diag::remark_fe_backend_optimization_remark_analysis);
  }
```



Comment at: flang/lib/Frontend/FrontendActions.cpp:1007-1024
+  bool handleDiagnostics(const llvm::DiagnosticInfo ) override {
+switch (di.getKind()) {
+case llvm::DK_OptimizationRemark:
+  optimizationRemarkHandler(llvm::cast(di));
+  break;
+case llvm::DK_OptimizationRemarkMissed:
+  
optimizationRemarkHandler(llvm::cast(di));

Where is this method used?



Comment at: flang/lib/Frontend/FrontendActions.cpp:1136
 
+  BackendRemarkConsumer m(diags, codeGenOpts);
+

https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

> Variable names should be nouns (as they represent state). The name should be 
> camel case, and start with an upper case letter (e.g. Leader or Boats).



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:169-171
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts());
+

[clang] 415d9e8 - [ASTMatcher] Fix typos in LibASTMatchersReference.html

2023-08-12 Thread via cfe-commits

Author: dingfei
Date: 2023-08-12T21:33:43+08:00
New Revision: 415d9e8ca39c0b42f351cc532ccfb48b6ac97f7f

URL: 
https://github.com/llvm/llvm-project/commit/415d9e8ca39c0b42f351cc532ccfb48b6ac97f7f
DIFF: 
https://github.com/llvm/llvm-project/commit/415d9e8ca39c0b42f351cc532ccfb48b6ac97f7f.diff

LOG: [ASTMatcher] Fix typos in LibASTMatchersReference.html

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index d30e151edae05b..4b87f157a9c2ce 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -2531,12 +2531,12 @@ Node Matchers
   class array {
 T data[Size];
   };
-dependentSizedArrayType
+dependentSizedArrayType()
   matches "T data[Size]"
 
 
 
-Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Type.html;>TypedependentSizedExtVectorTypeMatcherhttps://clang.llvm.org/doxygen/classclang_1_1DependentSizedExtVectorType.html;>DependentSizedExtVectorType...
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Type.html;>TypedependentSizedExtVectorTypeMatcherhttps://clang.llvm.org/doxygen/classclang_1_1DependentSizedExtVectorType.html;>DependentSizedExtVectorType...
 Matches 
C++ extended vector type where either the type or size is dependent.
 
 Given
@@ -2544,7 +2544,7 @@ Node Matchers
   class vector {
 typedef T __attribute__((ext_vector_type(Size))) type;
   };
-dependentSizedExtVectorType
+dependentSizedExtVectorType()
   matches "T __attribute__((ext_vector_type(Size)))"
 
 



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


[PATCH] D157781: [clang] Add cleanup_function field to CleanupAttr json AST dump

2023-08-12 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision.
serge-sans-paille added a reviewer: aaron.ballman.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157781

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp

Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -3,6 +3,11 @@
 int global_decl;
 extern __attribute__((alias("global_decl"))) int global_alias;
 
+void cleanup_function(int*);
+void some() {
+  __attribute__((cleanup(cleanup_function))) int var;
+}
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -78,7 +83,59 @@
 // CHECK-NEXT:  "tokLen": 1
 // CHECK-NEXT: }
 // CHECK-NEXT:},
-// CHECK-NEXT:"decl": "global_decl"
+// CHECK-NEXT:"aliasee": "global_decl"
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 242,
+// CHECK-NEXT:   "col": 50,
+// CHECK-NEXT:   "tokLen": 3
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 195,
+// CHECK-NEXT:"col": 3,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 242,
+// CHECK-NEXT:"col": 50,
+// CHECK-NEXT:"tokLen": 3
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "var",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "CleanupAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 210,
+// CHECK-NEXT:  "col": 18,
+// CHECK-NEXT:  "tokLen": 7
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 234,
+// CHECK-NEXT:  "col": 42,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"cleanup_function": {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "FunctionDecl",
+// CHECK-NEXT: "name": "cleanup_function",
+// CHECK-NEXT: "type": {
+// CHECK-NEXT:  "qualType": "void (int *)"
+// CHECK-NEXT: }
+// CHECK-NEXT:}
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -534,6 +534,10 @@
   JOS.attribute("aliasee", AA->getAliasee());
 }
 
+void JSONNodeDumper::VisitCleanupAttr(const CleanupAttr *CA) {
+  JOS.attribute("cleanup_function", createBareDeclRef(CA->getFunctionDecl()));
+}
+
 void JSONNodeDumper::VisitTypedefType(const TypedefType *TT) {
   JOS.attribute("decl", createBareDeclRef(TT->getDecl()));
   if (!TT->typeMatchesDecl())
Index: clang/include/clang/AST/JSONNodeDumper.h
===
--- clang/include/clang/AST/JSONNodeDumper.h
+++ clang/include/clang/AST/JSONNodeDumper.h
@@ -209,6 +209,7 @@
   void Visit(const APValue , QualType Ty);
 
   void VisitAliasAttr(const AliasAttr *AA);
+  void VisitCleanupAttr(const CleanupAttr *CA);
 
   void VisitTypedefType(const TypedefType *TT);
   void VisitUsingType(const UsingType *TT);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157780: [ASTImporter] Add import of MacroQualifiedType

2023-08-12 Thread Ding Fei via Phabricator via cfe-commits
danix800 updated this revision to Diff 549609.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157780

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8638,6 +8638,24 @@
   EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportMacroQualifiedType) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  Lang_CXX03, "", Lang_CXX03, "X");
+
+  auto *FromTy =
+  FirstDeclMatcher().match(From, macroQualifiedType());
+  auto *ToTy =
+  FirstDeclMatcher().match(To, macroQualifiedType());
+
+  EXPECT_TRUE(isa(FromTy->getUnderlyingType()));
+  EXPECT_TRUE(isa(ToTy->getUnderlyingType()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportCorrectTemplateName) {
   constexpr auto TestCode = R"(
   template 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -419,6 +419,7 @@
 ExpectedType VisitObjCInterfaceType(const ObjCInterfaceType *T);
 ExpectedType VisitObjCObjectType(const ObjCObjectType *T);
 ExpectedType VisitObjCObjectPointerType(const ObjCObjectPointerType *T);
+ExpectedType VisitMacroQualifiedType(const MacroQualifiedType *T);
 
 // Importing declarations
 Error ImportDeclParts(NamedDecl *D, DeclarationName , NamedDecl *,
@@ -1701,6 +1702,17 @@
   return Importer.getToContext().getObjCObjectPointerType(*ToPointeeTypeOrErr);
 }
 
+ExpectedType
+ASTNodeImporter::VisitMacroQualifiedType(const MacroQualifiedType *T) {
+  ExpectedType ToUnderlyingTypeOrErr = import(T->getUnderlyingType());
+  if (!ToUnderlyingTypeOrErr)
+return ToUnderlyingTypeOrErr.takeError();
+
+  IdentifierInfo *ToIdentifier = Importer.Import(T->getMacroIdentifier());
+  return Importer.getToContext().getMacroQualifiedType(*ToUnderlyingTypeOrErr,
+   ToIdentifier);
+}
+
 //
 // Import Declarations
 //


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8638,6 +8638,24 @@
   EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportMacroQualifiedType) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  Lang_CXX03, "", Lang_CXX03, "X");
+
+  auto *FromTy =
+  FirstDeclMatcher().match(From, macroQualifiedType());
+  auto *ToTy =
+  FirstDeclMatcher().match(To, macroQualifiedType());
+
+  EXPECT_TRUE(isa(FromTy->getUnderlyingType()));
+  EXPECT_TRUE(isa(ToTy->getUnderlyingType()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportCorrectTemplateName) {
   constexpr auto TestCode = R"(
   template 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -419,6 +419,7 @@
 ExpectedType VisitObjCInterfaceType(const ObjCInterfaceType *T);
 ExpectedType VisitObjCObjectType(const ObjCObjectType *T);
 ExpectedType VisitObjCObjectPointerType(const ObjCObjectPointerType *T);
+ExpectedType VisitMacroQualifiedType(const MacroQualifiedType *T);
 
 // Importing declarations
 Error ImportDeclParts(NamedDecl *D, DeclarationName , NamedDecl *,
@@ -1701,6 +1702,17 @@
   return Importer.getToContext().getObjCObjectPointerType(*ToPointeeTypeOrErr);
 }
 
+ExpectedType
+ASTNodeImporter::VisitMacroQualifiedType(const MacroQualifiedType *T) {
+  ExpectedType ToUnderlyingTypeOrErr = import(T->getUnderlyingType());
+  if (!ToUnderlyingTypeOrErr)
+return ToUnderlyingTypeOrErr.takeError();
+
+  IdentifierInfo *ToIdentifier = Importer.Import(T->getMacroIdentifier());
+  return Importer.getToContext().getMacroQualifiedType(*ToUnderlyingTypeOrErr,
+   ToIdentifier);
+}
+
 //
 // Import Declarations
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

2023-08-12 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

I've tested this locally and can confirm that the behavior of `clang` and 
`flang-new` has been preserved (as in, these changes won't be visible to the 
end users). Nice!

I think that it would be good to replace `Default` with e.g.

- `Clang`, or
- `ClangDriver`, or
- `ClangCompilerDriver`.

Or, at least, to make the meaning of `Default` much clearer (through e.g. 
comments). In general, I feel that `Default` is skewed towards this notion that 
`clang` (the compiler driver tool) is the main client of `clangDriver`. That 
used to be the case, but with Flang's driver also implemented in terms of 
`clangDriver` , that's no longer the case. Also, the long term goal is to 
extract the driver library out of Clang. One day :)

Also, note that `Default` options will be available in both `clang` and 
`flang-new`. That's the case today (so not something affected by your changes). 
Ideally, Flang should be able to disable those _Clang options_. That's not 
possible ATM. Contrary to Flang, Clang can disable _Flang options_ with 
`FlangOnlyOption`. There is no such flag for Flang ATM, but I think that we 
could re-use `Default` for that. WDYT?




Comment at: clang/docs/InternalsManual.rst:669-670
+  can affect how the option is treated or displayed.
+* ``Vis`` may contain visibility-specific "tags" associated with the option.
+  This lets us filter options for specific tools.
 * ``Alias`` denotes that the option is an alias of another option. This may be

IMHO, the difference between `Flags` and `Vis` is still unclear. Lets take this 
opportunity to refine this. IIUC:

* `vis` should be used to specify the drivers in which a particular option 
would be available. This attribute will impact `tool --help`,
* `flags`  can be used to limit the contexts in which a particular option/flag 
can be used (e.g. `NoXarchOption` or `LinkerOption`).



Comment at: clang/docs/InternalsManual.rst:682-685
+New options are recognized by the Clang driver if ``Vis`` is not specified or
+if it contains ``Default``. Options intended for the ``-cc1`` frontend must be
+explicitly marked with the ``CC1Option`` flag. Flags that specify ``CC1Option``
+but not ``Default`` will only be accessible via ``-cc1``.

"Clang driver" could mean two things:
* [[ 
https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/clang/lib/Driver/CMakeLists.txt#L17-L102
 | clangDriver ]] - the driver library shared between Clang and Flang,
* [[ 
https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/clang/tools/driver/CMakeLists.txt#L26-L36
 | clang ]] - Clang's compiler driver implemented in terms of `clangDriver` 
library.

I think that it's important to distinguish between the two in this document. In 
particular, the meaning of `Default` is confusing. Is the the default for the 
library (`clangDriver`) or the Clang compiler driver binary, `clang`. I believe 
it's the latter, but this wording suggests the former.

I appreciate that this wording pre-dates your patch (and probably Flang), but I 
think that it's worth taking this opportunity to refine this.



Comment at: llvm/docs/ReleaseNotes.rst:160
+* The ``Flags`` field of ``llvm::opt::Option`` has been split into ``Flags``
+  and ``Visibility`` to simplify option sharing between clang's drivers.
+  Overloads of ``llvm::opt::OptTable`` that use ``FlagsToInclude`` have been

[nit] Worth clarifying and advertising a bit more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157151

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


[PATCH] D157780: [ASTImporter] Add import of MacroQualifiedType

2023-08-12 Thread Ding Fei via Phabricator via cfe-commits
danix800 created this revision.
danix800 added a project: clang.
Herald added a subscriber: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
danix800 requested review of this revision.
Herald added a subscriber: cfe-commits.

Add import of MacroQualifiedType.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157780

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8638,6 +8638,24 @@
   EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportMacroQualifiedType) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  Lang_CXX03, "", Lang_CXX03, "X");
+
+  auto *FromTy =
+  FirstDeclMatcher().match(From, macroQualifiedType());
+  auto *ToTy =
+  FirstDeclMatcher().match(To, macroQualifiedType());
+
+  EXPECT_TRUE(isa(FromTy->getUnderlyingType()));
+  EXPECT_TRUE(isa(ToTy->getUnderlyingType()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportCorrectTemplateName) {
   constexpr auto TestCode = R"(
   template 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -419,6 +419,7 @@
 ExpectedType VisitObjCInterfaceType(const ObjCInterfaceType *T);
 ExpectedType VisitObjCObjectType(const ObjCObjectType *T);
 ExpectedType VisitObjCObjectPointerType(const ObjCObjectPointerType *T);
+ExpectedType VisitMacroQualifiedType(const MacroQualifiedType *T);
 
 // Importing declarations
 Error ImportDeclParts(NamedDecl *D, DeclarationName , NamedDecl *,
@@ -1701,6 +1702,17 @@
   return Importer.getToContext().getObjCObjectPointerType(*ToPointeeTypeOrErr);
 }
 
+ExpectedType
+ASTNodeImporter::VisitMacroQualifiedType(const MacroQualifiedType *T) {
+  ExpectedType ToUnderlyingTypeOrErr = import(T->getUnderlyingType());
+  if (!ToUnderlyingTypeOrErr)
+return ToUnderlyingTypeOrErr.takeError();
+
+  IdentifierInfo *ToIdentifier = Importer.Import(T->getMacroIdentifier());
+  return Importer.getToContext().getMacroQualifiedType(*ToUnderlyingTypeOrErr,
+   ToIdentifier);
+}
+
 //
 // Import Declarations
 //


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8638,6 +8638,24 @@
   EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportMacroQualifiedType) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  Lang_CXX03, "", Lang_CXX03, "X");
+
+  auto *FromTy =
+  FirstDeclMatcher().match(From, macroQualifiedType());
+  auto *ToTy =
+  FirstDeclMatcher().match(To, macroQualifiedType());
+
+  EXPECT_TRUE(isa(FromTy->getUnderlyingType()));
+  EXPECT_TRUE(isa(ToTy->getUnderlyingType()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportCorrectTemplateName) {
   constexpr auto TestCode = R"(
   template 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -419,6 +419,7 @@
 ExpectedType VisitObjCInterfaceType(const ObjCInterfaceType *T);
 ExpectedType VisitObjCObjectType(const ObjCObjectType *T);
 ExpectedType VisitObjCObjectPointerType(const ObjCObjectPointerType *T);
+ExpectedType VisitMacroQualifiedType(const MacroQualifiedType *T);
 
 // Importing declarations
 Error ImportDeclParts(NamedDecl *D, DeclarationName , NamedDecl *,
@@ -1701,6 +1702,17 @@
   return Importer.getToContext().getObjCObjectPointerType(*ToPointeeTypeOrErr);
 }
 
+ExpectedType
+ASTNodeImporter::VisitMacroQualifiedType(const MacroQualifiedType *T) {
+  ExpectedType ToUnderlyingTypeOrErr = import(T->getUnderlyingType());
+  if (!ToUnderlyingTypeOrErr)
+return ToUnderlyingTypeOrErr.takeError();
+
+  IdentifierInfo *ToIdentifier = Importer.Import(T->getMacroIdentifier());
+  return Importer.getToContext().getMacroQualifiedType(*ToUnderlyingTypeOrErr,
+   ToIdentifier);
+}
+
 //
 // Import Declarations
 

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

2023-08-12 Thread Ding Fei via Phabricator via cfe-commits
danix800 updated this revision to Diff 549607.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D15

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1838,6 +1838,15 @@
 namesType(typedefType()));
 }
 
+TEST_P(ASTMatchersTest, MacroQualifiedType) {
+  EXPECT_TRUE(matches(R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  typedefDecl(hasName("X"), hasType(pointerType(pointee(
+macroQualifiedType()));
+}
+
 TEST_P(ASTMatchersTest, TemplateSpecializationType) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -485,6 +485,7 @@
   REGISTER_MATCHER(lambdaCapture);
   REGISTER_MATCHER(lambdaExpr);
   REGISTER_MATCHER(linkageSpecDecl);
+  REGISTER_MATCHER(macroQualifiedType);
   REGISTER_MATCHER(materializeTemporaryExpr);
   REGISTER_MATCHER(member);
   REGISTER_MATCHER(memberExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -1058,6 +1058,7 @@
 const AstTypeMatcher functionProtoType;
 const AstTypeMatcher parenType;
 const AstTypeMatcher blockPointerType;
+const AstTypeMatcher macroQualifiedType;
 const AstTypeMatcher memberPointerType;
 const AstTypeMatcher pointerType;
 const AstTypeMatcher objcObjectPointerType;
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7258,6 +7258,17 @@
 ///   matches "typedef int X"
 extern const AstTypeMatcher typedefType;
 
+/// Matches macro qualified types.
+///
+/// Given
+/// \code
+///   #define CDECL __attribute__((cdecl))
+///   typedef void (CDECL *X)();
+/// \endcode
+/// macroQualifiedType()
+///   matches the type of the typedef declaration of \c X.
+extern const AstTypeMatcher macroQualifiedType;
+
 /// Matches enum types.
 ///
 /// Given
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -244,6 +244,7 @@
 
 - Add ``convertVectorExpr``.
 - Add ``dependentSizedExtVectorType``.
+- Add ``macroQualifiedType``.
 
 clang-format
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -2648,6 +2648,17 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Type.html;>TypemacroQualifiedTypeMatcherhttps://clang.llvm.org/doxygen/classclang_1_1MacroQualifiedType.html;>MacroQualifiedType...
+Matches macro 
qualified types.
+
+Given
+  #define CDECL __attribute__((cdecl))
+  typedef void (CDECL *X)();
+macroQualifiedType()
+  matches the type of the typedef declaration of X.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Type.html;>TypememberPointerTypeMatcherhttps://clang.llvm.org/doxygen/classclang_1_1MemberPointerType.html;>MemberPointerType...
 Matches member 
pointer types.
 Given


Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1838,6 +1838,15 @@
 namesType(typedefType()));
 }
 
+TEST_P(ASTMatchersTest, MacroQualifiedType) {
+  EXPECT_TRUE(matches(R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  typedefDecl(hasName("X"), hasType(pointerType(pointee(
+macroQualifiedType()));
+}
+
 TEST_P(ASTMatchersTest, TemplateSpecializationType) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- 

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

2023-08-12 Thread Ding Fei via Phabricator via cfe-commits
danix800 updated this revision to Diff 549605.
danix800 added a comment.
Herald added a subscriber: martong.
Herald added a reviewer: shafik.

Add import of MacroQualifiedType.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D15

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8638,6 +8638,24 @@
   EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportMacroQualifiedType) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  Lang_CXX03, "", Lang_CXX03, "X");
+
+  auto *FromTy =
+  FirstDeclMatcher().match(From, macroQualifiedType());
+  auto *ToTy =
+  FirstDeclMatcher().match(To, macroQualifiedType());
+
+  EXPECT_TRUE(isa(FromTy->getUnderlyingType()));
+  EXPECT_TRUE(isa(ToTy->getUnderlyingType()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportCorrectTemplateName) {
   constexpr auto TestCode = R"(
   template 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -419,6 +419,7 @@
 ExpectedType VisitObjCInterfaceType(const ObjCInterfaceType *T);
 ExpectedType VisitObjCObjectType(const ObjCObjectType *T);
 ExpectedType VisitObjCObjectPointerType(const ObjCObjectPointerType *T);
+ExpectedType VisitMacroQualifiedType(const MacroQualifiedType *T);
 
 // Importing declarations
 Error ImportDeclParts(NamedDecl *D, DeclarationName , NamedDecl *,
@@ -1701,6 +1702,17 @@
   return Importer.getToContext().getObjCObjectPointerType(*ToPointeeTypeOrErr);
 }
 
+ExpectedType
+ASTNodeImporter::VisitMacroQualifiedType(const MacroQualifiedType *T) {
+  ExpectedType ToUnderlyingTypeOrErr = import(T->getUnderlyingType());
+  if (!ToUnderlyingTypeOrErr)
+return ToUnderlyingTypeOrErr.takeError();
+
+  IdentifierInfo *ToIdentifier = Importer.Import(T->getMacroIdentifier());
+  return Importer.getToContext().getMacroQualifiedType(*ToUnderlyingTypeOrErr,
+   ToIdentifier);
+}
+
 //
 // Import Declarations
 //


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8638,6 +8638,24 @@
   EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportMacroQualifiedType) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+  R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  Lang_CXX03, "", Lang_CXX03, "X");
+
+  auto *FromTy =
+  FirstDeclMatcher().match(From, macroQualifiedType());
+  auto *ToTy =
+  FirstDeclMatcher().match(To, macroQualifiedType());
+
+  EXPECT_TRUE(isa(FromTy->getUnderlyingType()));
+  EXPECT_TRUE(isa(ToTy->getUnderlyingType()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportCorrectTemplateName) {
   constexpr auto TestCode = R"(
   template 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -419,6 +419,7 @@
 ExpectedType VisitObjCInterfaceType(const ObjCInterfaceType *T);
 ExpectedType VisitObjCObjectType(const ObjCObjectType *T);
 ExpectedType VisitObjCObjectPointerType(const ObjCObjectPointerType *T);
+ExpectedType VisitMacroQualifiedType(const MacroQualifiedType *T);
 
 // Importing declarations
 Error ImportDeclParts(NamedDecl *D, DeclarationName , NamedDecl *,
@@ -1701,6 +1702,17 @@
   return Importer.getToContext().getObjCObjectPointerType(*ToPointeeTypeOrErr);
 }
 
+ExpectedType
+ASTNodeImporter::VisitMacroQualifiedType(const MacroQualifiedType *T) {
+  ExpectedType ToUnderlyingTypeOrErr = import(T->getUnderlyingType());
+  if (!ToUnderlyingTypeOrErr)
+return ToUnderlyingTypeOrErr.takeError();
+
+  IdentifierInfo *ToIdentifier = Importer.Import(T->getMacroIdentifier());
+  return Importer.getToContext().getMacroQualifiedType(*ToUnderlyingTypeOrErr,
+   ToIdentifier);
+}
+
 //
 // Import Declarations
 //

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

2023-08-12 Thread Ding Fei via Phabricator via cfe-commits
danix800 created this revision.
danix800 added a project: clang.
Herald added a project: All.
danix800 requested review of this revision.
Herald added a subscriber: cfe-commits.

Add matcher for 'MacroQualifiedType'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D15

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1838,6 +1838,15 @@
 namesType(typedefType()));
 }
 
+TEST_P(ASTMatchersTest, MacroQualifiedType) {
+  EXPECT_TRUE(matches(R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  typedefDecl(hasName("X"), hasType(pointerType(pointee(
+macroQualifiedType()));
+}
+
 TEST_P(ASTMatchersTest, TemplateSpecializationType) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -485,6 +485,7 @@
   REGISTER_MATCHER(lambdaCapture);
   REGISTER_MATCHER(lambdaExpr);
   REGISTER_MATCHER(linkageSpecDecl);
+  REGISTER_MATCHER(macroQualifiedType);
   REGISTER_MATCHER(materializeTemporaryExpr);
   REGISTER_MATCHER(member);
   REGISTER_MATCHER(memberExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -1058,6 +1058,7 @@
 const AstTypeMatcher functionProtoType;
 const AstTypeMatcher parenType;
 const AstTypeMatcher blockPointerType;
+const AstTypeMatcher macroQualifiedType;
 const AstTypeMatcher memberPointerType;
 const AstTypeMatcher pointerType;
 const AstTypeMatcher objcObjectPointerType;
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7258,6 +7258,17 @@
 ///   matches "typedef int X"
 extern const AstTypeMatcher typedefType;
 
+/// Matches macro qualified types.
+///
+/// Given
+/// \code
+///   #define CDECL __attribute__((cdecl))
+///   typedef void (CDECL *X)();
+/// \endcode
+/// macroQualifiedType()
+///   matches the type of the typedef declaration of \c X.
+extern const AstTypeMatcher macroQualifiedType;
+
 /// Matches enum types.
 ///
 /// Given
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -244,6 +244,7 @@
 
 - Add ``convertVectorExpr``.
 - Add ``dependentSizedExtVectorType``.
+- Add ``macroQualifiedType``.
 
 clang-format
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -2648,6 +2648,17 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Type.html;>TypemacroQualifiedTypeMatcherhttps://clang.llvm.org/doxygen/classclang_1_1MacroQualifiedType.html;>MacroQualifiedType...
+Matches macro 
qualified types.
+
+Given
+  #define CDECL __attribute__((cdecl))
+  typedef void (CDECL *X)();
+macroQualifiedType()
+  matches the type of the typedef declaration of X.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Type.html;>TypememberPointerTypeMatcherhttps://clang.llvm.org/doxygen/classclang_1_1MemberPointerType.html;>MemberPointerType...
 Matches member 
pointer types.
 Given


Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1838,6 +1838,15 @@
 namesType(typedefType()));
 }
 
+TEST_P(ASTMatchersTest, MacroQualifiedType) {
+  EXPECT_TRUE(matches(R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  typedefDecl(hasName("X"), hasType(pointerType(pointee(
+macroQualifiedType()));
+}
+
 TEST_P(ASTMatchersTest, TemplateSpecializationType) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp

[PATCH] D156910: [clang] Add pragma force_vectorize

2023-08-12 Thread Maksim Kita via Phabricator via cfe-commits
kitaisreal updated this revision to Diff 549600.
kitaisreal added a comment.
Herald added a subscriber: zzheng.

Fixed tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156910

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/Parser/pragma-loop.cpp
  clang/test/Parser/pragma-unroll-and-jam.cpp
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Transforms/Scalar/WarnMissedTransforms.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp

Index: llvm/lib/Transforms/Utils/LoopUtils.cpp
===
--- llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -389,6 +389,11 @@
 }
 
 TransformationMode llvm::hasVectorizeTransformation(const Loop *L) {
+  std::optional EnableForceVectorize =
+  getOptionalBoolLoopAttribute(L, "llvm.loop.vectorize.force_vectorize");
+  if (EnableForceVectorize == true)
+return TM_ForcedByUser;
+
   std::optional Enable =
   getOptionalBoolLoopAttribute(L, "llvm.loop.vectorize.enable");
 
Index: llvm/lib/Transforms/Scalar/WarnMissedTransforms.cpp
===
--- llvm/lib/Transforms/Scalar/WarnMissedTransforms.cpp
+++ llvm/lib/Transforms/Scalar/WarnMissedTransforms.cpp
@@ -19,8 +19,8 @@
 
 #define DEBUG_TYPE "transform-warning"
 
-/// Emit warnings for forced (i.e. user-defined) loop transformations which have
-/// still not been performed.
+/// Emit warnings or errors for forced (i.e. user-defined) loop transformations
+/// which have still not been performed.
 static void warnAboutLeftoverTransformations(Loop *L,
  OptimizationRemarkEmitter *ORE) {
   if (hasUnrollTransformation(L) == TM_ForcedByUser) {
@@ -51,8 +51,19 @@
 getOptionalElementCountLoopAttribute(L);
 std::optional InterleaveCount =
 getOptionalIntLoopAttribute(L, "llvm.loop.interleave.count");
+std::optional ForceVectorize =
+getOptionalBoolLoopAttribute(L, "llvm.loop.vectorize.force_vectorize");
 
-if (!VectorizeWidth || VectorizeWidth->isVector())
+if (ForceVectorize == true)
+  ORE->emit(
+  DiagnosticInfoOptimizationFailure(
+  DEBUG_TYPE, "FailedRequestedForceVectorization", L->getStartLoc(),
+  L->getHeader(), DS_Error)
+  << "loop not force vectorized: the optimizer was unable to perform "
+ "the "
+ "requested transformation; the transformation might be disabled "
+ "or specified as part of an unsupported transformation ordering");
+else if (!VectorizeWidth || VectorizeWidth->isVector())
   ORE->emit(
   DiagnosticInfoOptimizationFailure(DEBUG_TYPE,
 "FailedRequestedVectorization",
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -351,14 +351,14 @@
 
 DiagnosticInfoOptimizationFailure::DiagnosticInfoOptimizationFailure(
 const char *PassName, StringRef RemarkName, const DiagnosticLocation ,
-const Value *CodeRegion)
+const Value *CodeRegion, DiagnosticSeverity Severity)
 : DiagnosticInfoIROptimization(
-  DK_OptimizationFailure, DS_Warning, PassName, RemarkName,
+  DK_OptimizationFailure, Severity, PassName, RemarkName,
   *cast(CodeRegion)->getParent(), Loc, CodeRegion) {}
 
 bool DiagnosticInfoOptimizationFailure::isEnabled() const {
-  // Only print warnings.
-  return getSeverity() == DS_Warning;
+  // Only print warnings and errors.
+  return getSeverity() == DS_Warning || getSeverity() == DS_Error;
 }
 
 void DiagnosticInfoUnsupported::print(DiagnosticPrinter ) const {
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -341,7 +341,7 @@
 
   /// Return the absolute path tot the file.
   std::string getAbsolutePath() const;
-  
+
   const Function () const { return Fn; }
   DiagnosticLocation getLocation() const { return Loc; }
 
@@ -983,9 +983,10 @@
   /// of the diagnostic.
   DiagnosticInfoOptimizationFailure(const Function ,
 const DiagnosticLocation ,
-const Twine )
-  : DiagnosticInfoIROptimization(DK_OptimizationFailure, DS_Warning,
- nullptr, Fn, Loc, Msg) {}
+const Twine 

[PATCH] D157775: [clang] Add aliasee field to AliasAttr json AST dump

2023-08-12 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision.
serge-sans-paille added a reviewer: aaron.ballman.
Herald added a subscriber: jeroen.dobbelaere.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157775

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp

Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -Wdocumentation -ast-dump=json %s | FileCheck %s
+
+int global_decl;
+extern __attribute__((alias("global_decl"))) int global_alias;
+
+// NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
+// using --filters=VarDecl
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 73,
+// CHECK-NEXT:   "file": "{{.*}}",
+// CHECK-NEXT:   "line": 3,
+// CHECK-NEXT:   "col": 5,
+// CHECK-NEXT:   "tokLen": 11
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 69,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 3
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 73,
+// CHECK-NEXT:"col": 5,
+// CHECK-NEXT:"tokLen": 11
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "global_decl",
+// CHECK-NEXT:  "mangledName": "global_decl",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  }
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 135,
+// CHECK-NEXT:   "line": 4,
+// CHECK-NEXT:   "col": 50,
+// CHECK-NEXT:   "tokLen": 12
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 86,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 6
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 135,
+// CHECK-NEXT:"col": 50,
+// CHECK-NEXT:"tokLen": 12
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "global_alias",
+// CHECK-NEXT:  "mangledName": "global_alias",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "storageClass": "extern",
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "AliasAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 108,
+// CHECK-NEXT:  "col": 23,
+// CHECK-NEXT:  "tokLen": 5
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 127,
+// CHECK-NEXT:  "col": 42,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"decl": "global_decl"
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -530,6 +530,10 @@
   return Ret;
 }
 
+void JSONNodeDumper::VisitAliasAttr(const AliasAttr *AA) {
+  JOS.attribute("aliasee", AA->getAliasee());
+}
+
 void JSONNodeDumper::VisitTypedefType(const TypedefType *TT) {
   JOS.attribute("decl", createBareDeclRef(TT->getDecl()));
   if (!TT->typeMatchesDecl())
Index: clang/include/clang/AST/JSONNodeDumper.h
===
--- clang/include/clang/AST/JSONNodeDumper.h
+++ clang/include/clang/AST/JSONNodeDumper.h
@@ -208,6 +208,8 @@
   void Visit(const concepts::Requirement *R);
   void Visit(const APValue , QualType Ty);
 
+  void VisitAliasAttr(const AliasAttr *AA);
+
   void VisitTypedefType(const TypedefType *TT);
   void VisitUsingType(const UsingType *TT);
   void VisitFunctionType(const FunctionType *T);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157773: [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

2023-08-12 Thread Julian Schmidt via Phabricator via cfe-commits
5chmidti added a comment.

Thanks. I don't have commit access, could somebody please commit this for me 
with `Julian Schmidt <44101708+5chmi...@users.noreply.github.com>`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157773

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


[PATCH] D157773: [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

2023-08-12 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157773

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


[PATCH] D157773: [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

2023-08-12 Thread Julian Schmidt via Phabricator via cfe-commits
5chmidti created this revision.
5chmidti added a project: clang-tools-extra.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
5chmidti requested review of this revision.
Herald added a subscriber: cfe-commits.

Differential https://reviews.llvm.org/D145477 removed the check for `(yaml and 
args.export_fixes)` in line 303 to skip looking for the 
`clang-apply-replacements` binary. However, the `tmpdir` variable was set in 
this true branch when exporting fixes and therefore is `None` when invoking 
run-clang-tidy with `run-clang-tidy -p . -export-fixes fixes.yaml`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157773

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -389,6 +389,8 @@
 clang_apply_replacements_binary = find_binary(
 args.clang_apply_replacements_binary, "clang-apply-replacements", 
build_path
 )
+
+if args.fix or (yaml and args.export_fixes):
 tmpdir = tempfile.mkdtemp()
 
 try:


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -389,6 +389,8 @@
 clang_apply_replacements_binary = find_binary(
 args.clang_apply_replacements_binary, "clang-apply-replacements", build_path
 )
+
+if args.fix or (yaml and args.export_fixes):
 tmpdir = tempfile.mkdtemp()
 
 try:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157747: Support Unicode Microsoft predefined macros

2023-08-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

This does not appear to be valid under MSVC and I don't think we should support 
features that MS does not (Nor can i find any documentation that it ought to 
work)
https://godbolt.org/z/7Te3YYeb9

Supporting the same concatenation behavior MSVC does have seem like a cleaner 
approach. We would not have an many new tokens.
Before you do anything though. I'd like the feedback of more folks.

I can't find any documentation for LPREFIX, but it seems in the general case, 
it can be emulated that way https://godbolt.org/z/dr7MjMs99

In terms of breaking change, it's only an issue if L/u/etc are existing macro 
at the point of use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157747

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


[PATCH] D156237: Complete the implementation of P2361 Unevaluated string literals

2023-08-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 549584.
cor3ntin marked 8 inline comments as done.
cor3ntin added a comment.

Address Hubert's feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156237

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Parser/MicrosoftExtensions.cpp
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/cxx-attributes.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/annotate-type.c
  clang/test/Sema/annotate.c
  clang/test/Sema/attr-assume.c
  clang/test/Sema/attr-btf_tag.c
  clang/test/Sema/attr-btf_type_tag.c
  clang/test/Sema/attr-capabilities.c
  clang/test/Sema/attr-enforce-tcb-errors.cpp
  clang/test/Sema/attr-enforce-tcb-errors.m
  clang/test/Sema/attr-error.c
  clang/test/Sema/attr-handles.cpp
  clang/test/Sema/attr-section.c
  clang/test/Sema/attr-tls_model.c
  clang/test/Sema/attr-unavailable-message.c
  clang/test/Sema/attr-warning.c
  clang/test/Sema/diagnose_if.c
  clang/test/Sema/enable_if.c
  clang/test/SemaCXX/attr-deprecated-replacement-error.cpp
  clang/test/SemaCXX/attr-no-sanitize.cpp
  clang/test/SemaCXX/attr-section.cpp
  clang/test/SemaCXX/attr-weakref.cpp
  clang/test/SemaCXX/suppress.cpp
  clang/test/SemaObjC/attr-swift_bridge.m
  clang/test/SemaObjC/objc-asm-attribute-neg-test.m
  clang/test/SemaObjC/validate-attr-swift_attr.m
  clang/test/SemaTemplate/attributes.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -115,12 +115,7 @@
  
   Unevaluated strings
   https://wg21.link/P2361R6;>P2361R6
-  
-
-  Clang 17 (Partial)
-  Attributes arguments don't yet parse as unevaluated string literals.
-
-  
+  Clang 18
  
  
   Add @, $, and ` to the basic character set
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2297,6 +2297,22 @@
  .Default(false);
 }
 
+static bool isStringLiteralArgument(const Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("StringArgument", true)
+ .Default(false);
+}
+
+static bool isVariadicStringLiteralArgument(const Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicStringArgument", true)
+ .Default(false);
+}
+
 static void emitClangAttrVariadicIdentifierArgList(RecordKeeper ,
raw_ostream ) {
   OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
@@ -2317,6 +2333,34 @@
   OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
 }
 
+// Emits the list of arguments that should be parsed as unevaluated string
+// literals for each attributes
+static void emitClangAttrUnevaluatedStringLiteralList(RecordKeeper ,
+  raw_ostream ) {
+  OS << "#if defined(CLANG_ATTR_STRING_LITERAL_ARG_LIST)\n";
+  std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *Attr : Attrs) {
+std::vector Args = Attr->getValueAsListOfDefs("Args");
+uint32_t Bits = 0;
+for (uint32_t N = 0; N < Args.size(); ++N) {
+  Bits |= (isStringLiteralArgument(Args[N]) << N);
+  // If we have a variadic string argument, set all the remaining bits to 1
+  if (isVariadicStringLiteralArgument(Args[N])) {
+for (; N < sizeof(uint32_t) * CHAR_BIT; ++N)
+  Bits |= (1 << N);
+break;
+  }
+}
+if (!Bits)
+  continue;
+// All these spellings have at least one string literal has argument.
+forEachUniqueSpelling(*Attr, [&](const FlattenedSpelling ) {
+  OS << ".Case(\"" << S.name() << "\", " << Bits << ")\n";
+});
+  }
+  OS << "#endif // CLANG_ATTR_STRING_LITERAL_ARG_LIST\n\n";
+}
+
 // Emits the first-argument-is-identifier property for attributes.
 static void emitClangAttrIdentifierArgList(RecordKeeper , raw_ostream ) {
   OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -4611,6 +4655,7 @@
   emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS);
   emitClangAttrArgContextList(Records, OS);
   emitClangAttrIdentifierArgList(Records, OS);
+  emitClangAttrUnevaluatedStringLiteralList(Records, OS);
   

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-08-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899
+  uint32_t CodePoint = 
static_cast(V.getInt().getZExtValue());
+  PrintCharLiteralPrefix(BTy->getKind(), OS);
+  OS << '\'';

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > cor3ntin wrote:
> > > Looking at the diagnostics, I don't think it makes sense to print a 
> > > prefix here. You could just leave that part out.
> > Why is removing the prefix better? The types can matter (characters outside 
> > the basic character set are allowed to have negative `char` values). Also, 
> > moving forward, the value of a character need not be the same in the 
> > various encodings.
> Some fun with signedness (imagine a more realistic example with `ISO-8859-1` 
> ordinary character encoding with a signed `char` type):
> ```
> $ clang -Xclang -fwchar-type=short -xc++ -<<<$'static_assert(L"\\uFF10"[0] == 
> U\'\\uFF10\');'
> :1:15: error: static assertion failed due to requirement 'L"\xFF10"[0] 
> == U'\uff10''
> 1 | static_assert(L"\uFF10"[0] == U'\uFF10');
>   |   ^
> :1:28: note: expression evaluates to ''0' (0xFF10) == '0' (0xFF10)'
> 1 | static_assert(L"\uFF10"[0] == U'\uFF10');
>   |   ~^~~~
> 1 error generated.
> Return:  0x01:1   Fri Aug 11 23:49:02 2023 EDT
> ```
Either we care about the actual character - ie `'a'`, or it's value (ie `42`). 
The motivation for the current patch is to add the value to the diagnostic 
message.
I'm also concerned about mixing things that are are are not lexical elements in 
the diagnostics



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16930-16936
+case BuiltinType::Char_S:
+case BuiltinType::Char_U:
+case BuiltinType::Char8:
+case BuiltinType::Char16:
+case BuiltinType::Char32:
+case BuiltinType::WChar_S:
+case BuiltinType::WChar_U: {

hubert.reinterpretcast wrote:
> Add FIXME for `char` and `wchar_t` cases that this assumes Unicode literal 
> encodings.
If we wanted such fixme, it should be L1689.



Comment at: clang/test/SemaCXX/static-assert.cpp:287
+  static_assert((char16_t)L'ゆ' == L"C̵̭̯̠̎͌ͅť̺"[1], ""); // expected-error 
{{failed}} \
+  // expected-note {{evaluates 
to ''ゆ' (0x3086) == '̵' (0x335)'}}
+  static_assert(L"\/"[1] == u'\xFFFD', ""); // expected-error {{failed}} \

hubert.reinterpretcast wrote:
> The C++23 escaped string formatting facility would not generate a trailing 
> combining character like this. I recommend following suit.
> 
> Info on U+0335: https://util.unicode.org/UnicodeJsps/character.jsp?a=0335
> 
This is way outside the scope of the patch. The diagnostic output facility has 
no understanding of combining characters or graphemes and do not attempt to 
match std::print. It probably would be an improvement but this patch is not 
trying to modify how all diagnostics are printed. (all of that logic is in 
Diagnostic.cpp)


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

https://reviews.llvm.org/D155610

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