[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 393048.
MyDeveloperDay added a comment.

Add more unit tests


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

https://reviews.llvm.org/D115050

Files:
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1924,6 +1924,22 @@
   verifyFormat("int *a = f1();", Style);
   verifyFormat("int  = f2();", Style);
   verifyFormat("int & = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto  : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const int  : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const Foo  : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const auto  : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const int  : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const Foo  : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const auto  : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const int  : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const Foo  : {1, 2, 3})", Style);
+  verifyFormat("for (auto x = 0; auto  : {1, 2, 3})", Style);
+  verifyFormat("for (auto x = 0; int  : {1, 2, 3})", Style);
+  verifyFormat("for (int x = 0; auto  : {1, 2, 3})", Style);
+  verifyFormat("for (int x = 0; int  : {1, 2, 3})", Style);
+  verifyFormat("for (f(); auto  : {1, 2, 3})", Style);
+  verifyFormat("for (f(); int  : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -1943,6 +1959,24 @@
   verifyFormat("int* a = f1();", Style);
   verifyFormat("int& b = f2();", Style);
   verifyFormat("int&& c = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto& c : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const int& c : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const Foo& c : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const Foo* c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const auto& c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const int& c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const Foo& c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const Foo* c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const auto& c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const int& c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const Foo& c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const Foo* c : {1, 2, 3})", Style);
+  verifyFormat("for (auto x = 0; auto& c : {1, 2, 3})", Style);
+  verifyFormat("for (auto x = 0; int& c : {1, 2, 3})", Style);
+  verifyFormat("for (int x = 0; auto& c : {1, 2, 3})", Style);
+  verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
+  verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
+  verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -1961,6 +1995,9 @@
   verifyFormat("int *a = f1();", Style);
   verifyFormat("int& b = f2();", Style);
   verifyFormat("int&& c = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const Foo *c : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -1979,6 +2016,20 @@
   verifyFormat("int* a = f1();", Style);
   verifyFormat("int & b = f2();", Style);
   verifyFormat("int && c = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto & c : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const int & c : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const Foo & c : {1, 2, 3})", Style);
+  verifyFormat("for (auto a = 0, b = 0; const Foo* c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const auto & c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const int & c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const Foo & c : {1, 2, 3})", Style);
+  verifyFormat("for (int a = 0, b++; const Foo* c : {1, 2, 3})", Style);
+  verifyFormat("for (auto x = 0; auto & c : {1, 2, 3})", Style);
+  verifyFormat("for (auto x = 0; int & c : {1, 2, 3})", Style);
+  verifyFormat("for (int x = 0; auto & c : {1, 2, 3})", Style);
+  verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
+  

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

[Chuanqi Xu 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 rG320e4efe99d3: [C++20] [Coroutines] Mark coroutine done if 
unhandled_exception throws (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115219

Files:
  llvm/docs/Coroutines.rst
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
  llvm/test/Transforms/Coroutines/coro-split-eh-01.ll

Index: llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
@@ -61,6 +61,9 @@
 ; CHECK:  lpad:
 ; CHECK-NEXT:   %tok = cleanuppad within none []
 ; CHECK-NEXT:   call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding path.
+; CHECK-NEXT:   %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:   store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:   cleanupret from %tok unwind to caller
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
@@ -67,6 +67,9 @@
 ; CHECK-NEXT:  %lpval = landingpad { i8*, i32 }
 ; CHECK-NEXT: cleanup
 ; CHECK-NEXT:  call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding path.
+; CHECK-NEXT:  %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:  store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:  resume { i8*, i32 } %lpval
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -280,6 +280,27 @@
   BB->getTerminator()->eraseFromParent();
 }
 
+// Mark a coroutine as done, which implies that the coroutine is finished and
+// never get resumed.
+//
+// In resume-switched ABI, the done state is represented by storing zero in
+// ResumeFnAddr.
+//
+// NOTE: We couldn't omit the argument `FramePtr`. It is necessary because the
+// pointer to the frame in splitted function is not stored in `Shape`.
+static void markCoroutineAsDone(IRBuilder<> , const coro::Shape ,
+Value *FramePtr) {
+  assert(
+  Shape.ABI == coro::ABI::Switch &&
+  "markCoroutineAsDone is only supported for Switch-Resumed ABI for now.");
+  auto *GepIndex = Builder.CreateStructGEP(
+  Shape.FrameTy, FramePtr, coro::Shape::SwitchFieldIndex::Resume,
+  "ResumeFn.addr");
+  auto *NullPtr = ConstantPointerNull::get(cast(
+  Shape.FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume)));
+  Builder.CreateStore(NullPtr, GepIndex);
+}
+
 /// Replace an unwind call to llvm.coro.end.
 static void replaceUnwindCoroEnd(AnyCoroEndInst *End, const coro::Shape ,
  Value *FramePtr, bool InResume,
@@ -288,10 +309,18 @@
 
   switch (Shape.ABI) {
   // In switch-lowering, this does nothing in the main function.
-  case coro::ABI::Switch:
+  case coro::ABI::Switch: {
+// In C++'s specification, the coroutine should be marked as done
+// if promise.unhandled_exception() throws.  The frontend will
+// call coro.end(true) along this path.
+//
+// FIXME: We should refactor this once there is other language
+// which uses Switch-Resumed style other than C++.
+markCoroutineAsDone(Builder, Shape, FramePtr);
 if (!InResume)
   return;
 break;
+  }
   // In async lowering this does nothing.
   case coro::ABI::Async:
 break;
@@ -364,13 +393,9 @@
 auto *Save = S->getCoroSave();
 Builder.SetInsertPoint(Save);
 if (S->isFinal()) {
-  // Final suspend point is represented by storing zero in ResumeFnAddr.
-  auto *GepIndex = Builder.CreateStructGEP(FrameTy, FramePtr,
- coro::Shape::SwitchFieldIndex::Resume,
-  "ResumeFn.addr");
-  auto *NullPtr = ConstantPointerNull::get(cast(
-  FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume)));
-  Builder.CreateStore(NullPtr, GepIndex);
+  // The coroutine should be marked done if it reaches the final suspend
+  // point.
+  markCoroutineAsDone(Builder, Shape, FramePtr);
 } else {
   auto *GepIndex = Builder.CreateStructGEP(

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

Thanks for reviewing this!


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

https://reviews.llvm.org/D115219

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

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 393039.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D115219

Files:
  llvm/docs/Coroutines.rst
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
  llvm/test/Transforms/Coroutines/coro-split-eh-01.ll

Index: llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
@@ -61,6 +61,9 @@
 ; CHECK:  lpad:
 ; CHECK-NEXT:   %tok = cleanuppad within none []
 ; CHECK-NEXT:   call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding path.
+; CHECK-NEXT:   %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:   store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:   cleanupret from %tok unwind to caller
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
@@ -67,6 +67,9 @@
 ; CHECK-NEXT:  %lpval = landingpad { i8*, i32 }
 ; CHECK-NEXT: cleanup
 ; CHECK-NEXT:  call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding path.
+; CHECK-NEXT:  %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:  store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:  resume { i8*, i32 } %lpval
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -280,6 +280,27 @@
   BB->getTerminator()->eraseFromParent();
 }
 
+// Mark a coroutine as done, which implies that the coroutine is finished and
+// never get resumed.
+//
+// In resume-switched ABI, the done state is represented by storing zero in
+// ResumeFnAddr.
+//
+// NOTE: We couldn't omit the argument `FramePtr`. It is necessary because the
+// pointer to the frame in splitted function is not stored in `Shape`.
+static void markCoroutineAsDone(IRBuilder<> , const coro::Shape ,
+Value *FramePtr) {
+  assert(
+  Shape.ABI == coro::ABI::Switch &&
+  "markCoroutineAsDone is only supported for Switch-Resumed ABI for now.");
+  auto *GepIndex = Builder.CreateStructGEP(
+  Shape.FrameTy, FramePtr, coro::Shape::SwitchFieldIndex::Resume,
+  "ResumeFn.addr");
+  auto *NullPtr = ConstantPointerNull::get(cast(
+  Shape.FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume)));
+  Builder.CreateStore(NullPtr, GepIndex);
+}
+
 /// Replace an unwind call to llvm.coro.end.
 static void replaceUnwindCoroEnd(AnyCoroEndInst *End, const coro::Shape ,
  Value *FramePtr, bool InResume,
@@ -288,10 +309,18 @@
 
   switch (Shape.ABI) {
   // In switch-lowering, this does nothing in the main function.
-  case coro::ABI::Switch:
+  case coro::ABI::Switch: {
+// In C++'s specification, the coroutine should be marked as done
+// if promise.unhandled_exception() throws.  The frontend will
+// call coro.end(true) along this path.
+//
+// FIXME: We should refactor this once there is other language
+// which uses Switch-Resumed style other than C++.
+markCoroutineAsDone(Builder, Shape, FramePtr);
 if (!InResume)
   return;
 break;
+  }
   // In async lowering this does nothing.
   case coro::ABI::Async:
 break;
@@ -364,13 +393,9 @@
 auto *Save = S->getCoroSave();
 Builder.SetInsertPoint(Save);
 if (S->isFinal()) {
-  // Final suspend point is represented by storing zero in ResumeFnAddr.
-  auto *GepIndex = Builder.CreateStructGEP(FrameTy, FramePtr,
- coro::Shape::SwitchFieldIndex::Resume,
-  "ResumeFn.addr");
-  auto *NullPtr = ConstantPointerNull::get(cast(
-  FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume)));
-  Builder.CreateStore(NullPtr, GepIndex);
+  // The coroutine should be marked done if it reaches the final suspend
+  // point.
+  markCoroutineAsDone(Builder, Shape, FramePtr);
 } else {
   auto *GepIndex = Builder.CreateStructGEP(
   FrameTy, FramePtr, Shape.getSwitchIndexField(), "index.addr");
Index: llvm/docs/Coroutines.rst
===
--- llvm/docs/Coroutines.rst

[PATCH] D114505: [clang][unittests] Fix a clang unittest linking issue

[Lu Weining via Phabricator via cfe-commits]

SixWeining added a comment.

In D114505#3181637 , @dexonsmith 
wrote:

> LGTM. Sorry for missing this before. Looks to me like the debian build 
> failure above is unrelated.
>
> If you need someone to commit for you, please include the info for 
> GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL.

Thanks @dexonsmith . I don't have commit access, could you help to land this 
patch for me? Please use below info to commit the change.

email = luwein...@loongson.cn
name = Weining Lu

BTW, according to the policy 
, a "Differential 
Revision: " line should be added to the commit message. But it seems that 
I'm not allowed to add such line without commit access. Could you help to add 
it?


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

https://reviews.llvm.org/D114505

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

[PATCH] D109818: [HIPSPV] Convert HIP kernels to SPIR-V kernels

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki added a comment.

Thanks, @bader.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109818

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

[PATCH] D115222: [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)

[Chuanqi Xu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG352e36e10d2c: [Coroutines] Remove unused coroutine 
builtin/intrinsics llvm.coro.param (NFC… (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115222

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenCoroutines/coro-builtins.c
  llvm/docs/Coroutines.rst
  llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Coroutines/Coroutines.cpp

Index: llvm/lib/Transforms/Coroutines/Coroutines.cpp
===
--- llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -141,7 +141,6 @@
   "llvm.coro.id.retcon",
   "llvm.coro.id.retcon.once",
   "llvm.coro.noop",
-  "llvm.coro.param",
   "llvm.coro.prepare.async",
   "llvm.coro.prepare.retcon",
   "llvm.coro.promise",
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -1282,10 +1282,6 @@
 def int_coro_alloca_get : Intrinsic<[llvm_ptr_ty], [llvm_token_ty], []>;
 def int_coro_alloca_free : Intrinsic<[], [llvm_token_ty], []>;
 
-def int_coro_param : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty, llvm_ptr_ty],
-   [IntrNoMem, ReadNone>,
-ReadNone>]>;
-
 // Coroutine Manipulation Intrinsics.
 
 def int_coro_resume : Intrinsic<[], [llvm_ptr_ty], [Throws]>;
Index: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
===
--- llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -625,7 +625,6 @@
 case Intrinsic::coro_frame:
 case Intrinsic::coro_size:
 case Intrinsic::coro_suspend:
-case Intrinsic::coro_param:
 case Intrinsic::coro_subfn_addr:
   // These intrinsics don't actually represent code after lowering.
   return 0;
Index: llvm/docs/Coroutines.rst
===
--- llvm/docs/Coroutines.rst
+++ llvm/docs/Coroutines.rst
@@ -1644,89 +1644,6 @@
 In a yield-once coroutine, it is undefined behavior if the coroutine
 executes a call to ``llvm.coro.suspend.retcon`` after resuming in any way.
 
-.. _coro.param:
-
-'llvm.coro.param' Intrinsic
-^^^
-::
-
-  declare i1 @llvm.coro.param(i8* , i8* )
-
-Overview:
-"
-
-The '``llvm.coro.param``' is used by a frontend to mark up the code used to
-construct and destruct copies of the parameters. If the optimizer discovers that
-a particular parameter copy is not used after any suspends, it can remove the
-construction and destruction of the copy by replacing corresponding coro.param
-with `i1 false` and replacing any use of the `copy` with the `original`.
-
-Arguments:
-""
-
-The first argument points to an `alloca` storing the value of a parameter to a
-coroutine.
-
-The second argument points to an `alloca` storing the value of the copy of that
-parameter.
-
-Semantics:
-""
-
-The optimizer is free to always replace this intrinsic with `i1 true`.
-
-The optimizer is also allowed to replace it with `i1 false` provided that the
-parameter copy is only used prior to control flow reaching any of the suspend
-points. The code that would be DCE'd if the `coro.param` is replaced with
-`i1 false` is not considered to be a use of the parameter copy.
-
-The frontend can emit this intrinsic if its language rules allow for this
-optimization.
-
-Example:
-
-Consider the following example. A coroutine takes two parameters `a` and `b`
-that has a destructor and a move constructor.
-
-.. code-block:: c++
-
-  struct A { ~A(); A(A&&); bool foo(); void bar(); };
-
-  task f(A a, A b) {
-if (a.foo())
-  return 42;
-
-a.bar();
-co_await read_async(); // introduces suspend point
-b.bar();
-  }
-
-Note that, uses of `b` is used after a suspend point and thus must be copied
-into a coroutine frame, whereas `a` does not have to, since it never used
-after suspend.
-
-A frontend can create parameter copies for `a` and `b` as follows:
-
-.. code-block:: text
-
-  task f(A a', A b') {
-a = alloca A;
-b = alloca A;
-// move parameters to its copies
-if (coro.param(a', a)) A::A(a, A&& a');
-if (coro.param(b', b)) A::A(b, A&& b');
-...
-// destroy parameters copies
-if (coro.param(a', a)) A::~A(a);
-if (coro.param(b', b)) A::~A(b);
-  }
-
-The optimizer can replace coro.param(a',a) with `i1 false` and replace all uses
-of `a` with `a'`, since it is not used after suspend.
-
-The 

[clang] 352e36e - [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2021-12-09T14:40:25+08:00
New Revision: 352e36e10d2cff310cacfc98aab39d508682e61d

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

LOG: [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param 
(NFC-ish)

I found that the coroutine intrinsic llvm.coro.param in documentation
(https://llvm.org/docs/Coroutines.html#id101) didn't get used actually
since there isn't lowering codes in LLVM. I also checked the
implementation of libstdc++ and libc++. Both of them didn't use
llvm.coro.param. So I am pretty sure that the llvm.coro.param intrinsic
is unused. I think it would be better t to remove it to avoid possible
misleading understandings.

Note: according to [class.copy.elision]/p1.3, this optimization is
allowed by the C++ language specification. Let's make it someday.

Reviewed By: rjmccall

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/Builtins.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGenCoroutines/coro-builtins.c
llvm/docs/Coroutines.rst
llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
llvm/include/llvm/IR/Intrinsics.td
llvm/lib/Transforms/Coroutines/Coroutines.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index e2c724e91d7d4..24e85d336ef13 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3007,7 +3007,6 @@ an appropriate value during the emission.
   void  *__builtin_coro_begin(void *memory)
   void   __builtin_coro_end(void *coro_frame, bool unwind)
   char   __builtin_coro_suspend(bool final)
-  bool   __builtin_coro_param(void *original, void *copy)
 
 Note that there is no builtin matching the `llvm.coro.save` intrinsic. LLVM
 automatically will insert one if the first argument to `llvm.coro.suspend` is

diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 162660518f8bb..7ef550e5d81af 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -1612,7 +1612,6 @@ LANGBUILTIN(__builtin_coro_alloc, "b", "n", COR_LANG)
 LANGBUILTIN(__builtin_coro_begin, "v*v*", "n", COR_LANG)
 LANGBUILTIN(__builtin_coro_end, "bv*Ib", "n", COR_LANG)
 LANGBUILTIN(__builtin_coro_suspend, "cIb", "n", COR_LANG)
-LANGBUILTIN(__builtin_coro_param, "bv*v*", "n", COR_LANG)
 
 // OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
 // We need the generic prototype, since the packet type could be anything.

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index efe41f60fb520..5a12231541562 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -4702,8 +4702,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_end);
   case Builtin::BI__builtin_coro_suspend:
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_suspend);
-  case Builtin::BI__builtin_coro_param:
-return EmitCoroutineIntrinsic(E, Intrinsic::coro_param);
 
   // OpenCL v2.0 s6.13.16.2, Built-in pipe read and write functions
   case Builtin::BIread_pipe:

diff  --git a/clang/test/CodeGenCoroutines/coro-builtins.c 
b/clang/test/CodeGenCoroutines/coro-builtins.c
index 9800eef0ed819..67a97b691d59e 100644
--- a/clang/test/CodeGenCoroutines/coro-builtins.c
+++ b/clang/test/CodeGenCoroutines/coro-builtins.c
@@ -5,9 +5,7 @@ void *myAlloc(long long);
 // CHECK-LABEL: f(
 void f(int n) {
   // CHECK: %n.addr = alloca i32
-  // CHECK: %n_copy = alloca i32
   // CHECK: %promise = alloca i32
-  int n_copy;
   int promise;
 
   // CHECK: %[[PROM_ADDR:.+]] = bitcast i32* %promise to i8*
@@ -45,9 +43,4 @@ void f(int n) {
 
   // CHECK-NEXT: call i8 @llvm.coro.suspend(token none, i1 true)
   __builtin_coro_suspend(1);
-
-  // CHECK-NEXT: %[[N_ADDR:.+]] = bitcast i32* %n.addr to i8*
-  // CHECK-NEXT: %[[N_COPY_ADDR:.+]] = bitcast i32* %n_copy to i8*
-  // CHECK-NEXT: call i1 @llvm.coro.param(i8* %[[N_ADDR]], i8* 
%[[N_COPY_ADDR]])
-  __builtin_coro_param(, _copy);
 }

diff  --git a/llvm/docs/Coroutines.rst b/llvm/docs/Coroutines.rst
index 8ea40563546ab..09d44e6be16bf 100644
--- a/llvm/docs/Coroutines.rst
+++ b/llvm/docs/Coroutines.rst
@@ -1644,89 +1644,6 @@ a call to ``llvm.coro.suspend.retcon`` after resuming 
abnormally.
 In a yield-once coroutine, it is undefined behavior if the coroutine
 executes a call to ``llvm.coro.suspend.retcon`` after resuming in any way.
 
-.. _coro.param:
-
-'llvm.coro.param' Intrinsic
-^^^
-::
-
-  declare i1 @llvm.coro.param(i8* , i8* )
-
-Overview:
-"
-
-The '``llvm.coro.param``' 

[PATCH] D112903: [C++20] [Module] Fix bug47116 and implement [module.interface]/p6

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 393030.
ChuanqiXu added a comment.

Rebased


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

https://reviews.llvm.org/D112903

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/module/module.interface/p2-2.cpp
  clang/test/CXX/module/module.interface/p6.cpp

Index: clang/test/CXX/module/module.interface/p6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.interface/p6.cpp
@@ -0,0 +1,93 @@
+// The test is check we couldn't export a redeclaration which isn't exported previously and
+// check it is OK to redeclare no matter exported nor not if is the previous declaration is exported.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+export module X;
+
+struct S { // expected-note {{previous declaration is here}}
+  int n;
+};
+typedef S S;
+export typedef S S; // OK, does not redeclare an entity
+export struct S;// expected-error {{cannot export redeclaration 'S' here since the previous declaration is not exported.}}
+
+namespace A {
+struct X; // expected-note {{previous declaration is here}}
+export struct Y;
+} // namespace A
+
+namespace A {
+export struct X; // expected-error {{cannot export redeclaration 'X' here since the previous declaration is not exported.}}
+export struct Y; // OK
+struct Z;// expected-note {{previous declaration is here}}
+export struct Z; // expected-error {{cannot export redeclaration 'Z' here since the previous declaration is not exported.}}
+} // namespace A
+
+namespace A {
+struct B;// expected-note {{previous declaration is here}}
+struct C {}; // expected-note {{previous declaration is here}}
+} // namespace A
+
+namespace A {
+export struct B {}; // expected-error {{cannot export redeclaration 'B' here since the previous declaration is not exported.}}
+export struct C;// expected-error {{cannot export redeclaration 'C' here since the previous declaration is not exported.}}
+} // namespace A
+
+template 
+struct TemplS; // expected-note {{previous declaration is here}}
+
+export template 
+struct TemplS {}; // expected-error {{cannot export redeclaration 'TemplS' here since the previous declaration is not exported.}}
+
+template 
+struct TemplS2; // expected-note {{previous declaration is here}}
+
+export template 
+struct TemplS2 {}; // expected-error {{cannot export redeclaration 'TemplS2' here since the previous declaration is not exported.}}
+
+void baz();// expected-note {{previous declaration is here}}
+export void baz(); // expected-error {{cannot export redeclaration 'baz' here since the previous declaration is not exported.}}
+
+namespace A {
+export void foo();
+void bar();// expected-note {{previous declaration is here}}
+export void bar(); // expected-error {{cannot export redeclaration 'bar' here since the previous declaration is not exported.}}
+void f1(); // expected-note {{previous declaration is here}}
+} // namespace A
+
+// OK
+//
+// [module.interface]/p6
+// A redeclaration of an entity X is implicitly exported if X was introduced by an exported declaration
+void A::foo();
+
+// The compiler couldn't export A::f1() here since A::f1() is declared above without exported.
+// See [module.interface]/p6 for details.
+export void A::f1(); // expected-error {{cannot export redeclaration 'f1' here since the previous declaration is not exported.}}
+
+template 
+void TemplFunc(); // expected-note {{previous declaration is here}}
+
+export template 
+void TemplFunc() { // expected-error {{cannot export redeclaration 'TemplFunc' here since the previous declaration is not exported.}}
+}
+
+namespace A {
+template 
+void TemplFunc2(); // expected-note {{previous declaration is here}}
+export template 
+void TemplFunc2() {} // expected-error {{cannot export redeclaration 'TemplFunc2' here since the previous declaration is not exported.}}
+template 
+void TemplFunc3(); // expected-note {{previous declaration is here}}
+} // namespace A
+
+export template 
+void A::TemplFunc3() {} // expected-error {{cannot export redeclaration 'TemplFunc3' here since the previous declaration is not exported.}}
+
+int var;// expected-note {{previous declaration is here}}
+export int var; // expected-error {{cannot export redeclaration 'var' here since the previous declaration is not exported.}}
+
+template 
+T TemplVar; // expected-note {{previous declaration is here}}
+export template 
+T TemplVar; // expected-error {{cannot export redeclaration 'TemplVar' here since the previous declaration is not exported.}}
Index: clang/test/CXX/module/module.interface/p2-2.cpp
===
--- /dev/null
+++ 

[PATCH] D115425: [clangd] Generate ConfigFragment/YAML/docs from one tablegen source

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman, mgorny.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

The idea is to reduce the number of places that need to be updated as we add
more config settings, encourage regular patterns, and make cross-cutting changes
easier (e.g. reformat docs).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115425

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Config.td
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/ClangdConfigEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -132,6 +132,10 @@
 void EmitTestPragmaAttributeSupportedAttributes(llvm::RecordKeeper ,
 llvm::raw_ostream );
 
+void EmitClangdConfigHeader(llvm::RecordKeeper , llvm::raw_ostream );
+void EmitClangdConfigYAML(llvm::RecordKeeper , llvm::raw_ostream );
+void EmitClangdConfigDocs(llvm::RecordKeeper , llvm::raw_ostream );
+
 } // end namespace clang
 
 #endif
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -92,7 +92,10 @@
   GenDiagDocs,
   GenOptDocs,
   GenDataCollectors,
-  GenTestPragmaAttributeSupportedAttributes
+  GenTestPragmaAttributeSupportedAttributes,
+  GenClangdConfigHeader,
+  GenClangdConfigYAML,
+  GenClangdConfigDocs,
 };
 
 namespace {
@@ -253,7 +256,13 @@
 clEnumValN(GenTestPragmaAttributeSupportedAttributes,
"gen-clang-test-pragma-attribute-supported-attributes",
"Generate a list of attributes supported by #pragma clang "
-   "attribute for testing purposes")));
+   "attribute for testing purposes"),
+clEnumValN(GenClangdConfigHeader, "gen-clangd-config-header",
+   "Generate clangd Config.h struct"),
+clEnumValN(GenClangdConfigYAML, "gen-clangd-config-yaml",
+   "Generate clangd config YAML parser"),
+clEnumValN(GenClangdConfigDocs, "gen-clangd-config-docs",
+   "Generate clangd config documentation")));
 
 cl::opt
 ClangComponent("clang-component",
@@ -473,6 +482,15 @@
   case GenTestPragmaAttributeSupportedAttributes:
 EmitTestPragmaAttributeSupportedAttributes(Records, OS);
 break;
+  case GenClangdConfigHeader:
+EmitClangdConfigHeader(Records, OS);
+break;
+  case GenClangdConfigYAML:
+EmitClangdConfigYAML(Records, OS);
+break;
+  case GenClangdConfigDocs:
+EmitClangdConfigDocs(Records, OS);
+break;
   }
 
   return false;
Index: clang/utils/TableGen/ClangdConfigEmitter.cpp
===
--- /dev/null
+++ clang/utils/TableGen/ClangdConfigEmitter.cpp
@@ -0,0 +1,214 @@
+//=- ClangDiagnosticsEmitter.cpp - Generate clangd tables --*- C++ -*-//
+//
+// 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
+//
+//===--===//
+//
+// These tablegen backends emit clangd config tables.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/TableGenBackend.h"
+
+namespace {
+class Emitter {
+  auto indent() {
+Indent += 2;
+return llvm::make_scope_exit([this] { Indent -= 2; });
+  }
+  llvm::raw_ostream () {
+OS << "\n";
+OS.indent(Indent);
+return OS;
+  }
+  auto sep() {
+return [Sep(""), this]() mutable {
+  OS << Sep;
+  Sep = "\n";
+};
+  }
+
+  static std::string quote(llvm::StringRef S) {
+std::string Buf;
+llvm::raw_string_ostream OS(Buf);
+OS << '"';
+llvm::printEscapedString(S, OS);
+OS << '"';
+OS.flush();
+return Buf;
+  }
+
+  // Returns lines from a docstring:
+  //  - with leading and trailing whitespace-only lines removed
+  //  - with indentation stripped based on the first (non-empty) line
+  static llvm::SmallVector docLines(llvm::StringRef Doc) {
+llvm::Optional Indent; // from first nonempty line
+

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 393028.
ChuanqiXu added a comment.

Update comments.


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

https://reviews.llvm.org/D113545

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
  clang/test/CXX/module/module.interface/p2.cpp
  clang/test/CXX/module/module.reach/Inputs/p5-A.cppm
  clang/test/CXX/module/module.reach/p5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/Modules/Inputs/Reachability-Private/Private.cppm
  clang/test/Modules/Inputs/Reachability-func-default-arg/func_default_arg.cppm
  clang/test/Modules/Inputs/Reachability-func-ret/func_ret.cppm
  
clang/test/Modules/Inputs/Reachability-template-default-arg/template_default_arg.cppm
  clang/test/Modules/Inputs/Reachability-template-instantiation/Templ.h
  clang/test/Modules/Inputs/Reachability-template-instantiation/Use.cppm
  clang/test/Modules/Inputs/Reachability-using-templates/mod-templates.cppm
  clang/test/Modules/Inputs/Reachability-using/mod.cppm
  clang/test/Modules/Reachability-Private.cpp
  clang/test/Modules/Reachability-func-default-arg.cpp
  clang/test/Modules/Reachability-func-ret.cpp
  clang/test/Modules/Reachability-template-default-arg.cpp
  clang/test/Modules/Reachability-template-instantiation.cpp
  clang/test/Modules/Reachability-using-templates.cpp
  clang/test/Modules/Reachability-using.cpp

Index: clang/test/Modules/Reachability-using.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-using.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-using/mod.cppm --precompile -o %t/mod.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import mod;
+void foo() {
+  u v{};
+}
Index: clang/test/Modules/Reachability-using-templates.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-using-templates.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-using-templates/mod-templates.cppm --precompile -o %t/mod.templates.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import mod.templates;
+void foo() {
+  u v{};
+}
Index: clang/test/Modules/Reachability-template-instantiation.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-template-instantiation.cpp
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-template-instantiation/Use.cppm --precompile -o %t/Use.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t -I%S/Inputs/Reachability-template-instantiation %s -c -Xclang -verify
+// expected-no-diagnostics
+module;
+#include "Templ.h"
+export module Use;
+export template 
+class Use {
+public:
+  Wrapper value;
+};
Index: clang/test/Modules/Reachability-template-default-arg.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-template-default-arg.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-template-default-arg/template_default_arg.cppm --precompile -o %t/template_default_arg.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import template_default_arg;
+void bar() {
+  A<> a0;
+  A a1; // expected-error {{declaration of 't' must be imported from module 'template_default_arg' before it is required}}
+   // expected-note@Inputs/Reachability-template-default-arg/template_default_arg.cppm:2 {{declaration here is not visible}}
+}
Index: clang/test/Modules/Reachability-func-ret.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-func-ret.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-func-ret/func_ret.cppm --precompile -o %t/func_ret.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import func_ret;
+void bar() {
+  auto ret = foo();
+}
Index: clang/test/Modules/Reachability-func-default-arg.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-func-default-arg.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 393027.
ChuanqiXu added a comment.

Rebased


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

https://reviews.llvm.org/D113545

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
  clang/test/CXX/module/module.interface/p2.cpp
  clang/test/CXX/module/module.reach/Inputs/p5-A.cppm
  clang/test/CXX/module/module.reach/p5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/Modules/Inputs/Reachability-Private/Private.cppm
  clang/test/Modules/Inputs/Reachability-func-default-arg/func_default_arg.cppm
  clang/test/Modules/Inputs/Reachability-func-ret/func_ret.cppm
  
clang/test/Modules/Inputs/Reachability-template-default-arg/template_default_arg.cppm
  clang/test/Modules/Inputs/Reachability-template-instantiation/Templ.h
  clang/test/Modules/Inputs/Reachability-template-instantiation/Use.cppm
  clang/test/Modules/Inputs/Reachability-using-templates/mod-templates.cppm
  clang/test/Modules/Inputs/Reachability-using/mod.cppm
  clang/test/Modules/Reachability-Private.cpp
  clang/test/Modules/Reachability-func-default-arg.cpp
  clang/test/Modules/Reachability-func-ret.cpp
  clang/test/Modules/Reachability-template-default-arg.cpp
  clang/test/Modules/Reachability-template-instantiation.cpp
  clang/test/Modules/Reachability-using-templates.cpp
  clang/test/Modules/Reachability-using.cpp

Index: clang/test/Modules/Reachability-using.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-using.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-using/mod.cppm --precompile -o %t/mod.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import mod;
+void foo() {
+  u v{};
+}
Index: clang/test/Modules/Reachability-using-templates.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-using-templates.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-using-templates/mod-templates.cppm --precompile -o %t/mod.templates.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import mod.templates;
+void foo() {
+  u v{};
+}
Index: clang/test/Modules/Reachability-template-instantiation.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-template-instantiation.cpp
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-template-instantiation/Use.cppm --precompile -o %t/Use.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t -I%S/Inputs/Reachability-template-instantiation %s -c -Xclang -verify
+// expected-no-diagnostics
+module;
+#include "Templ.h"
+export module Use;
+export template 
+class Use {
+public:
+  Wrapper value;
+};
Index: clang/test/Modules/Reachability-template-default-arg.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-template-default-arg.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-template-default-arg/template_default_arg.cppm --precompile -o %t/template_default_arg.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import template_default_arg;
+void bar() {
+  A<> a0;
+  A a1; // expected-error {{declaration of 't' must be imported from module 'template_default_arg' before it is required}}
+   // expected-note@Inputs/Reachability-template-default-arg/template_default_arg.cppm:2 {{declaration here is not visible}}
+}
Index: clang/test/Modules/Reachability-func-ret.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-func-ret.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-func-ret/func_ret.cppm --precompile -o %t/func_ret.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import func_ret;
+void bar() {
+  auto ret = foo();
+}
Index: clang/test/Modules/Reachability-func-default-arg.cpp
===
--- /dev/null
+++ clang/test/Modules/Reachability-func-default-arg.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 

[clang] 9791b58 - [C++20 Modules] Don't create global module fragment for extern linkage declaration in GMF already

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2021-12-09T13:55:15+08:00
New Revision: 9791b589516b644a6273607b46a9c6661993d667

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

LOG: [C++20 Modules] Don't create global module fragment for extern linkage 
declaration in GMF already

Previously we would create global module fragment for extern linkage
declaration which is alreday in global module fragment. However, it is
clearly redundant to do so. This patch would check if the extern linkage
declaration are already in GMF before we create a GMF for it.

Added: 
clang/test/CXX/module/module.unit/p7/Inputs/h7.h
clang/test/CXX/module/module.unit/p7/t7.cpp

Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a12dd4db66c13..73c5ad1dd7acf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -,6 +,12 @@ class Sema final {
 return ModuleScopes.empty() ? nullptr : ModuleScopes.back().Module;
   }
 
+  /// Helper function to judge if we are in module purview.
+  /// Return false if we are not in a module.
+  bool isCurrentModulePurview() const {
+return getCurrentModule() ? getCurrentModule()->isModulePurview() : false;
+  }
+
   /// Enter the scope of the global module.
   Module *PushGlobalModuleFragment(SourceLocation BeginLoc, bool IsImplicit);
   /// Leave the scope of the global module.

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 4a0eda2a700fe..3f6bde1b9ed6a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -16153,7 +16153,10 @@ Decl *Sema::ActOnStartLinkageSpecification(Scope *S, 
SourceLocation ExternLoc,
   ///   - ...
   ///   - appears within a linkage-specification,
   ///   it is attached to the global module.
-  if (getLangOpts().CPlusPlusModules && getCurrentModule()) {
+  ///
+  /// If the declaration is already in global module fragment, we don't
+  /// need to attach it again.
+  if (getLangOpts().CPlusPlusModules && isCurrentModulePurview()) {
 Module *GlobalModule =
 PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);
 D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
@@ -16177,7 +16180,11 @@ Decl *Sema::ActOnFinishLinkageSpecification(Scope *S,
 LSDecl->setRBraceLoc(RBraceLoc);
   }
 
-  if (getLangOpts().CPlusPlusModules && getCurrentModule())
+  // If the current module doesn't has Parent, it implies that the
+  // LinkageSpec isn't in the module created by itself. So we don't
+  // need to pop it.
+  if (getLangOpts().CPlusPlusModules && getCurrentModule() &&
+  getCurrentModule()->isGlobalModule() && getCurrentModule()->Parent)
 PopGlobalModuleFragment();
 
   PopDeclContext();

diff  --git a/clang/test/CXX/module/module.unit/p7/Inputs/h7.h 
b/clang/test/CXX/module/module.unit/p7/Inputs/h7.h
new file mode 100644
index 0..cd3b6706d561c
--- /dev/null
+++ b/clang/test/CXX/module/module.unit/p7/Inputs/h7.h
@@ -0,0 +1,10 @@
+#ifndef H7_H
+#define H7_H
+
+extern "C++" {
+class A {};
+}
+
+class B : public A {};
+
+#endif

diff  --git a/clang/test/CXX/module/module.unit/p7/t7.cpp 
b/clang/test/CXX/module/module.unit/p7/t7.cpp
new file mode 100644
index 0..7ce0cbb964131
--- /dev/null
+++ b/clang/test/CXX/module/module.unit/p7/t7.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -I%S/Inputs/ %s -verify
+// expected-no-diagnostics
+module;
+#include "h7.h"
+export module X;



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

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: llvm/include/llvm/ProfileData/InstrProfWriter.h:48
 public:
-  InstrProfWriter(bool Sparse = false, bool InstrEntryBBEnabled = false);
   ~InstrProfWriter();

Was the InstrEntryBBEnabled parameter just never used? I didn't see any changes 
to callsites.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115393

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

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/test/Verifier/callbr.ll:42
+  callbr void asm sideeffect "${0:l} ${1:l} ${2:l}", "i,X,i"(i8* 
blockaddress(@test3, %4), i8* blockaddress(@test3, %2), i8* 
blockaddress(@test3, %3))
   to label %1 [label %3, label %4]
 1:

Does it mean we are able to pass blockaddress out of current function? For that 
case, we can't put it in the indirect labels list, right?



Comment at: llvm/test/tools/llvm-diff/callbr.ll:28-29
 entry:
-  callbr void asm sideeffect "", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* 
blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
+  callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* 
blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
   to label %asm.fallthrough [label %return, label %t_no]
 

If my above assumption is true, I think we can't replace the `X` with `i` here.
Besides, I'm confused on the indirect labels list. Are they the labels of `bar` 
or `foo`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410

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

[PATCH] D115199: [WIP][X86][AMX] Support amxpreserve attribute in clang.

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke updated this revision to Diff 393012.
LuoYuanke added a comment.
Herald added a subscriber: martong.

Updating D115199 : [WIP][X86][AMX] Support 
amxpreserve attribute in clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115199

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Sema/attr-target-mv.c
  clang/test/SemaCXX/attr-non-x86-amx-preserve.cpp
  clang/test/SemaCXX/attr-x86-amx-preserve.cpp
  clang/unittests/AST/StructuralEquivalenceTest.cpp

Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -476,6 +476,16 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest,
+   FunctionsWithDifferentAMXSavedRegsAttr) {
+  if (llvm::Triple(llvm::sys::getDefaultTargetTriple()).getArch() !=
+  llvm::Triple::x86_64)
+return;
+  auto t = makeNamedDecls("__attribute__((amxpreserve)) void foo();",
+  " void foo();", Lang_C99);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 struct StructuralEquivalenceCXXMethodTest : StructuralEquivalenceTest {
 };
 
Index: clang/test/SemaCXX/attr-x86-amx-preserve.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-x86-amx-preserve.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
+
+struct a {
+  int b __attribute__((amxpreserve)); // expected-warning {{'amxpreserve' only applies to function types; type here is 'int'}}
+  static void foo(int *a) __attribute__((amxpreserve)) {}
+};
+
+struct a test __attribute__((amxpreserve)); // expected-warning {{'amxpreserve' only applies to function types; type here is 'struct a'}}
+
+__attribute__((amxpreserve(999))) void bar(int *) {} // expected-error {{'amxpreserve' attribute takes no arguments}}
+
+void __attribute__((amxpreserve)) foo(int *){}
+
+__attribute__((amxpreserve)) void foo2(int *) {}
+
+typedef __attribute__((amxpreserve)) void (*foo3)(int *);
+
+int (*foo4)(double a, __attribute__((amxpreserve)) float b); // expected-warning {{'amxpreserve' only applies to function types; type here is 'float'}}
+
+typedef void (*foo5)(int *);
+
+void foo6(){} // expected-note {{previous declaration is here}}
+
+void __attribute__((amxpreserve)) foo6(); // expected-error {{function declared with 'amxpreserve' attribute was previously declared without the 'amxpreserve' attribute}} 
+
+int main(int argc, char **argv) {
+  void (*fp)(int *) = foo; // expected-error {{cannot initialize a variable of type 'void (*)(int *)' with an lvalue of type 'void (int *) __attribute__((amxpreserve))'}} 
+  a::foo();
+  foo3 func = foo2;
+  func();
+  foo5 __attribute__((amxpreserve)) func2 = foo2;
+  return 0;
+}
Index: clang/test/SemaCXX/attr-non-x86-amx-preserve.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-non-x86-amx-preserve.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++11 -triple armv7-unknown-linux-gnueabi -fsyntax-only -verify %s
+
+struct a {
+  int __attribute__((amxpreserve)) b; // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+  static void foo(int *a) __attribute__((amxpreserve)) {} // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+};
+
+struct a test __attribute__((amxpreserve)); // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+
+__attribute__((amxpreserve(999))) void bar(int *) {} // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+
+__attribute__((amxpreserve)) void foo(int *){} // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+
+[[clang::amxpreserve]] void foo2(int *) {} // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+
+typedef __attribute__((amxpreserve)) void (*foo3)(int *); // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+
+typedef void (*foo5)(int *);
+
+int (*foo4)(double a, __attribute__((amxpreserve)) float b); // expected-warning {{unknown attribute 'amxpreserve' ignored}}
+
+int main(int argc, char **argv) {
+  void (*fp)(int *) = foo;
+  a::foo();
+  foo3 func = foo2;
+  func();
+  foo5 __attribute__((amxpreserve)) func2 = foo2; // expected-warning {{unknown attribute 

[PATCH] D115415: [clang][macho] add clang frontend support for emitting macho files with two build version load commands

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

This LGTM, but I came across it pretty quickly; might be worth leaving for a 
couple of days in case someone else has comments.


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

https://reviews.llvm.org/D115415

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

[PATCH] D105765: Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake:509
 set(DARWIN_macho_embedded_LIBRARY_OUTPUT_DIR
-  ${COMPILER_RT_OUTPUT_DIR}/lib/macho_embedded)
+  ${COMPILER_RT_OUTPUT_LIBRARY_DIR}/macho_embedded)
 set(DARWIN_macho_embedded_LIBRARY_INSTALL_DIR

arphaman wrote:
> Ericson2314 wrote:
> > arphaman wrote:
> > > It looks like this change broke the `macho_embedded` layout for Darwin's 
> > > compiler-rt build, so now the clang driver isn unable to find these 
> > > libraries.
> > > 
> > > I will commit a change that uses `COMPILER_RT_OUTPUT_DIR` again for the 
> > > `macho_embedded` libraries.
> > Can you help me understand this better? `COMPILER_RT_OUTPUT_LIBRARY_DIR` 
> > should be defined to be the same thing unless target-specific directories 
> > are used, Is the problem in the latter case?
> The problem is that Darwin was emitting `macho_embedded` libraries under 
> `usr/lib/clang//lib/darwin/macho_embedded`, but this change moved 
> them to `usr/lib/clang//lib/macho_embedded`, so now the clang driver 
> isn't able to find them.
> 
> I think using `COMPILER_RT_OUTPUT_LIBRARY_DIR` is the right thing, but we 
> still want to be compatible with the existing layout unless we change the 
> driver. I think changing the driver might be a better approach though, so I 
> will try that.
Sorry, I meant to say that Darwin was emitting macho_embedded libraries under 
`usr/lib/clang//lib/macho_embedded`, but this change moved them to 
`usr/lib/clang//lib/darwin/macho_embedded`, so now the clang driver 
isn't able to find them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105765

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

[PATCH] D105765: Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake:509
 set(DARWIN_macho_embedded_LIBRARY_OUTPUT_DIR
-  ${COMPILER_RT_OUTPUT_DIR}/lib/macho_embedded)
+  ${COMPILER_RT_OUTPUT_LIBRARY_DIR}/macho_embedded)
 set(DARWIN_macho_embedded_LIBRARY_INSTALL_DIR

Ericson2314 wrote:
> arphaman wrote:
> > It looks like this change broke the `macho_embedded` layout for Darwin's 
> > compiler-rt build, so now the clang driver isn unable to find these 
> > libraries.
> > 
> > I will commit a change that uses `COMPILER_RT_OUTPUT_DIR` again for the 
> > `macho_embedded` libraries.
> Can you help me understand this better? `COMPILER_RT_OUTPUT_LIBRARY_DIR` 
> should be defined to be the same thing unless target-specific directories are 
> used, Is the problem in the latter case?
The problem is that Darwin was emitting `macho_embedded` libraries under 
`usr/lib/clang//lib/darwin/macho_embedded`, but this change moved them 
to `usr/lib/clang//lib/macho_embedded`, so now the clang driver isn't 
able to find them.

I think using `COMPILER_RT_OUTPUT_LIBRARY_DIR` is the right thing, but we still 
want to be compatible with the existing layout unless we change the driver. I 
think changing the driver might be a better approach though, so I will try that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105765

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

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D115374#3181670 , @logan-5 wrote:

> To be clear, it sounds like we should //either// add `.take()` for moving the 
> string out of `raw_string_ostream`'s string reference, //or// make 
> `raw_string_ostream` not need to be flushed (after which there won't be as 
> clear a use for `.take()`, since you can just use the underlying string 
> directly).
>
> I vote for the second option, making `raw_string_ostream` not need to be 
> flushed, since it allows for simpler code at the call site (`return Str;`), 
> permits NRVO at the call site, and avoids some possibly weird questions 
> `.take()` would bring with it (like whether it would ever be surprising that 
> it moves out of a //reference// that someone else might also have).
>
> If that's the direction that sounds best, I can submit an updated patch 
> sometime tomorrow.

Yeah, probably one or the other. I'm leaning toward the no-flush approach as 
well, but I'd suggest making that a separate prep patch to reduce 
churn/dependencies in case there's some reason it needs to be reverted (e.g., 
compile time regression). Removing the no-longer-needed `.str()`s in a 
follow-up a few days later (rebasing this without the flushes) should be 
trivial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

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

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Logan Smith via Phabricator via cfe-commits]

logan-5 added a comment.

To be clear, it sounds like we should //either// add `.take()` for moving the 
string out of `raw_string_ostream`'s string reference, //or// make 
`raw_string_ostream` not need to be flushed (after which there won't be as 
clear a use for `.take()`, since you can just use the underlying string 
directly).

I vote for the second option, making `raw_string_ostream` not need to be 
flushed, since it allows for simpler code at the call site (`return Str;`), 
permits NRVO at the call site, and avoids some possibly weird questions 
`.take()` would bring with it (like whether it would ever be surprising that it 
moves out of a //reference// that someone else might also have).

If that's the direction that sounds best, I can submit an updated patch 
sometime tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

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

[PATCH] D115415: [clang][macho] add clang frontend support for emitting macho files with two build version load commands

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: ravikandhadai, dexonsmith, steven_wu.
Herald added subscribers: dang, ributzka, hiraditya.
arphaman requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This patch extends clang frontend to add metadata that can be used to emit 
macho files with two build version load commands.
It utilizes "darwin.target_variant.triple" and "darwin.target_variant.SDK 
Version" metadata names for that.

MachO uses two build version load commands to represent an object file / binary 
that is targeting both the macOS target,
and the Mac Catalyst target. At runtime, a dynamic library that supports both 
targets can be loaded from either a native
macOS or a Mac Catalyst app on a macOS system. We want to add support to this 
to upstream to LLVM to be able to build
compiler-rt for both targets, to finish the complete support for the Mac 
Catalyst platform, which is right now targetable
by upstream clang, but the compiler-rt bits aren't supported because of the 
lack of this multiple build version support.

  

Follow-up to https://reviews.llvm.org/D112189, I'm working on final two 
patches, to add clang driver support, and change compiler-rt to build with 
target variant.


https://reviews.llvm.org/D115415

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets.cpp
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/darwin-target-variant.c
  llvm/include/llvm/IR/Module.h
  llvm/lib/IR/Module.cpp

Index: llvm/lib/IR/Module.cpp
===
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -736,7 +736,7 @@
   addModuleFlag(ModFlagBehavior::Error, "override-stack-alignment", Align);
 }
 
-void Module::setSDKVersion(const VersionTuple ) {
+static void addSDKVersionMD(const VersionTuple , Module , StringRef Name) {
   SmallVector Entries;
   Entries.push_back(V.getMajor());
   if (auto Minor = V.getMinor()) {
@@ -746,8 +746,12 @@
 // Ignore the 'build' component as it can't be represented in the object
 // file.
   }
-  addModuleFlag(ModFlagBehavior::Warning, "SDK Version",
-ConstantDataArray::get(Context, Entries));
+  M.addModuleFlag(Module::ModFlagBehavior::Warning, Name,
+  ConstantDataArray::get(M.getContext(), Entries));
+}
+
+void Module::setSDKVersion(const VersionTuple ) {
+  addSDKVersionMD(V, *this, "SDK Version");
 }
 
 static VersionTuple getSDKVersionMD(Metadata *MD) {
@@ -820,6 +824,15 @@
   return "";
 }
 
+void Module::setDarwinTargetVariantTriple(StringRef T) {
+  addModuleFlag(ModFlagBehavior::Override, "darwin.target_variant.triple",
+MDString::get(getContext(), T));
+}
+
 VersionTuple Module::getDarwinTargetVariantSDKVersion() const {
   return getSDKVersionMD(getModuleFlag("darwin.target_variant.SDK Version"));
 }
+
+void Module::setDarwinTargetVariantSDKVersion(VersionTuple Version) {
+  addSDKVersionMD(Version, *this, "darwin.target_variant.SDK Version");
+}
Index: llvm/include/llvm/IR/Module.h
===
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -941,10 +941,17 @@
   /// @returns a string containing the target variant triple.
   StringRef getDarwinTargetVariantTriple() const;
 
+  /// Set the target variant triple which is a string describing a variant of
+  /// the target host platform.
+  void setDarwinTargetVariantTriple(StringRef T);
+
   /// Get the target variant version build SDK version metadata.
   ///
   /// An empty version is returned if no such metadata is attached.
   VersionTuple getDarwinTargetVariantSDKVersion() const;
+
+  /// Set the target variant version build SDK version metadata.
+  void setDarwinTargetVariantSDKVersion(VersionTuple Version);
 };
 
 /// Given "llvm.used" or "llvm.compiler.used" as a global name, collect the
Index: clang/test/CodeGen/darwin-target-variant.c
===
--- /dev/null
+++ clang/test/CodeGen/darwin-target-variant.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos11 -darwin-target-variant-triple x86_64-apple-ios14-macabi -target-sdk-version=11.1 -darwin-target-variant-sdk-version=14.1 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: !llvm.module.flags = !{!0, !1, !2
+// CHECK: !0 = !{i32 2, !"SDK Version", [2 x i32] [i32 11, i32 1]}
+// CHECK: !1 = !{i32 4, !"darwin.target_variant.triple", !"x86_64-apple-ios14-macabi"}
+// CHECK: !2 = !{i32 2, !"darwin.target_variant.SDK Version", [2 x i32] [i32 14, i32 1]}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4397,6 

[PATCH] D114505: [clang][unittests] Fix a clang unittest linking issue

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM. Sorry for missing this before. Looks to me like the debian build failure 
above is unrelated.

If you need someone to commit for you, please include the info for 
GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL.


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

https://reviews.llvm.org/D114505

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

[PATCH] D115409: [SelectionDAGBuilder] drop special handling for CallBr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll:23
+; CHECK-NEXT: t30: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t32: ch = br t30, BasicBlock:ch
 

@craig.topper can you triple check this change to this whole file carefully, 
please?

I'm not sure TBH why my change to SelectionDAGBuilder changed this.  I'm also 
not sure of the original intent of the test. It's running `-debug-only=isel` 
which prints A LOT of different phases; I'm not sure which it was originally 
testing.

I'm not sure why we get an `BlockAddress` now; I don't really understand the 
output from selectionDAG here. (`t16` looks unused to me, IIUC).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115409

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

[PATCH] D115311: [clang][CGStmt] emit i constraint rather than X for asm goto indirect dests

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 393001.
nickdesaulniers added a comment.

- try to update this correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115311

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/CodeGen/asm.c
  clang/test/Modules/asm-goto.c


Index: clang/test/Modules/asm-goto.c
===
--- clang/test/Modules/asm-goto.c
+++ clang/test/Modules/asm-goto.c
@@ -4,7 +4,7 @@
 #include "a.h"
 
 // CHECK-LABEL: define {{.*}} @foo(
-// CHECK: callbr {{.*}} "=r,X{{.*}} blockaddress(@foo, %indirect))
+// CHECK: callbr {{.*}} "=r,i{{.*}} blockaddress(@foo, %indirect))
 // CHECK-NEXT: to label %asm.fallthrough [label %indirect]
 
 int bar(void) {
Index: clang/test/CodeGen/asm.c
===
--- clang/test/CodeGen/asm.c
+++ clang/test/CodeGen/asm.c
@@ -267,7 +267,7 @@
 int t32(int cond)
 {
   asm goto("testl %0, %0; jne %l1;" :: "r"(cond)::label_true, loop);
-  // CHECK: callbr void asm sideeffect "testl $0, $0; jne ${1:l};", 
"r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@t32, 
%label_true), i8* blockaddress(@t32, %loop)) #1
+  // CHECK: callbr void asm sideeffect "testl $0, $0; jne ${1:l};", 
"r,i,i,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@t32, 
%label_true), i8* blockaddress(@t32, %loop)) #1
   return 0;
 loop:
   return 0;
Index: clang/test/CodeGen/asm-goto.c
===
--- clang/test/CodeGen/asm-goto.c
+++ clang/test/CodeGen/asm-goto.c
@@ -55,14 +55,14 @@
 
 int test4(int out1, int out2) {
   // CHECK-LABEL: define{{.*}} i32 @test4(
-  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${3:l}", 
"={si},={di},r,X,X,0,1
+  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${3:l}", 
"={si},={di},r,i,i,0,1
   // CHECK: to label %asm.fallthrough [label %label_true, label %loop]
   // CHECK-LABEL: asm.fallthrough:
   if (out1 < out2)
 asm volatile goto("jne %l3" : "+S"(out1), "+D"(out2) : "r"(out1) :: 
label_true, loop);
   else
 asm volatile goto("jne %l5" : "+S"(out1), "+D"(out2) : "r"(out1), 
"r"(out2) :: label_true, loop);
-  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", 
"={si},={di},r,r,X,X,0,1
+  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", 
"={si},={di},r,r,i,i,0,1
   // CHECK: to label %asm.fallthrough2 [label %label_true, label %loop]
   // CHECK-LABEL: asm.fallthrough2:
   return out1 + out2;
@@ -74,7 +74,7 @@
 
 int test5(int addr, int size, int limit) {
   // CHECK-LABEL: define{{.*}} i32 @test5(
-  // CHECK: callbr i32 asm "add $1,$0 ; jc ${3:l} ; cmp $2,$0 ; ja ${3:l} ; ", 
"=r,imr,imr,X,0
+  // CHECK: callbr i32 asm "add $1,$0 ; jc ${3:l} ; cmp $2,$0 ; ja ${3:l} ; ", 
"=r,imr,imr,i,0
   // CHECK: to label %asm.fallthrough [label %t_err]
   // CHECK-LABEL: asm.fallthrough:
   asm goto(
@@ -92,7 +92,7 @@
 
 int test6(int out1) {
   // CHECK-LABEL: define{{.*}} i32 @test6(
-  // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne 
${2:l}", "={si},r,X,X,0,{{.*}} i8* blockaddress(@test6, %label_true), i8* 
blockaddress(@test6, %landing)
+  // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne 
${2:l}", "={si},r,i,i,0,{{.*}} i8* blockaddress(@test6, %label_true), i8* 
blockaddress(@test6, %landing)
   // CHECK: to label %asm.fallthrough [label %label_true, label %landing]
   // CHECK-LABEL: asm.fallthrough:
   // CHECK-LABEL: landing:
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2548,7 +2548,7 @@
 ArgTypes.push_back(BA->getType());
 if (!Constraints.empty())
   Constraints += ',';
-Constraints += 'X';
+Constraints += 'i';
   }
   Fallthrough = createBasicBlock("asm.fallthrough");
 }


Index: clang/test/Modules/asm-goto.c
===
--- clang/test/Modules/asm-goto.c
+++ clang/test/Modules/asm-goto.c
@@ -4,7 +4,7 @@
 #include "a.h"
 
 // CHECK-LABEL: define {{.*}} @foo(
-// CHECK: callbr {{.*}} "=r,X{{.*}} blockaddress(@foo, %indirect))
+// CHECK: callbr {{.*}} "=r,i{{.*}} blockaddress(@foo, %indirect))
 // CHECK-NEXT: to label %asm.fallthrough [label %indirect]
 
 int bar(void) {
Index: clang/test/CodeGen/asm.c
===
--- clang/test/CodeGen/asm.c
+++ clang/test/CodeGen/asm.c
@@ -267,7 +267,7 @@
 int t32(int cond)
 {
   asm goto("testl %0, %0; jne %l1;" :: "r"(cond)::label_true, loop);
-  // CHECK: callbr void asm sideeffect "testl $0, $0; jne ${1:l};", "r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@t32, %label_true), i8* blockaddress(@t32, %loop)) #1
+  // CHECK: callbr void asm sideeffect "testl 

[PATCH] D115311: [clang][CGStmt] emit i constraint rather than X for asm goto indirect dests

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers planned changes to this revision.
nickdesaulniers added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8556
   unsigned ResNo = 0;   // ResNo - The result number of the next output.
-  unsigned NumMatchingOps = 0;
   for (auto  : TargetConstraints) {

ugh, I didn't mean to post this hunk. I've been struggling with `arc` today. 
let me try to fix this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115311

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

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/test/Verifier/callbr.ll:41
   ; the asm goto is in the arg list to the asm).
-  callbr void asm sideeffect "${0:l} ${1:l} ${2:l}", "X,X,X"(i8* 
blockaddress(@test3, %4), i8* blockaddress(@test3, %2), i8* 
blockaddress(@test3, %3))
+  callbr void asm sideeffect "${0:l} ${1:l} ${2:l}", "i,X,i"(i8* 
blockaddress(@test3, %4), i8* blockaddress(@test3, %2), i8* 
blockaddress(@test3, %3))
   to label %1 [label %3, label %4]

Note to reviewers, this is the only case that was interesting IMO, because the 
middle input operand is not an indirect destination. Ie. the label referred to 
in the `blockaddress` parameter is not duplicated in the `[]` on the next line.

Not that it makes a difference, just preemptively answering the question: "why 
is this case different from everything else in this change?"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410

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

[PATCH] D113254: [clang] Fix a misadjusted path style comparison in a unittest

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM; sorry for missing this previously! (I'd have been happy for you to commit 
this without review, BTW, since it seems a bit obvious, but no excuse of course 
for me being slow...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113254

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

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D115374#3181491 , @logan-5 wrote:

> In D115374#3181383 , @dexonsmith 
> wrote:
>
>> I don't feel strongly, but IMO the code might be a bit harder to 
>> read/maintain with the explicit flush. I worry that it'd be easy to move the 
>> `flush()` away from the `return`. Not sure I'm right; could just be 
>> familiarity with `str()`.
>
> I definitely hear you. I don't really mind it personally, and I did it this 
> way because I saw precedent in a couple spots (there's one on 
> CompilerInvocation.cpp:653, several in clangd, etc.). I definitely see how it 
> could be a little bit spooky though.

I haven't done the git-blame, but I somewhat suspect `str()` was added 
specifically as a convenience to make the flush+access/return a one-liner.

> I suppose the question then becomes whether to name it `take()` or `str() &&` 
> for symmetry with C++20's `std::ostringstream`. (Also for the record, I agree 
> that NRVOing some std::strings isn't going to make a giant difference; my 
> opinion is simply that if we can get it, we might as well.)

I think `take()` would be clearer for use in LLVM code, despite the symmetry. 
Especially since the two stream libraries have significant differences. (Also, 
maybe I'm mis-remembering, but I think I've seen places where `OS.str()` is 
passed to some function when it's partially written; seems dangerous to silent 
start selecting a different overload. Could be I'm wrong though.)




Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36
+OS.flush();
+return Str;
   }

logan-5 wrote:
> dexonsmith wrote:
> > logan-5 wrote:
> > > Quuxplusone wrote:
> > > > FWIW, it appears to me that in most (all?) of these cases, what's 
> > > > really wanted is not "a string //and// a stream" but rather "a stream 
> > > > that owns a string" (`std::ostringstream` or the LLVM-codebase 
> > > > equivalent thereof). Then the return can be `return 
> > > > std::move(OS).str();` — for `std::ostringstream`, this Does The Right 
> > > > Thing since C++20, and if LLVM had its own stringstream it could make 
> > > > it Do The Right Thing today.
> > > > https://en.cppreference.com/w/cpp/io/basic_ostringstream/str
> > > > 
> > > True enough. Although `return std::move(OS).str();` is still much harder 
> > > to type than the less efficient `return OS.str();`, and it requires at 
> > > minimum a move of the underlying string--whereas `return Str;` is the 
> > > easiest of all to type, and it opens things up for NRVO. If (as I said in 
> > > the patch summary) `raw_string_ostream` were changed to be guaranteed to 
> > > not need flushing, `return Str;` would IMHO be cemented as the clear 
> > > winner.
> > > 
> > > That said, you're clearly right that all these cases are semantically 
> > > trying to do "a stream that owns a string", and it's clunky to execute 
> > > with the existing APIs.
> > >  If (as I said in the patch summary) raw_string_ostream were changed to 
> > > be guaranteed to not need flushing
> > 
> > This sounds like a one-line patch; might be better to just do it rather 
> > than having to churn all these things twice.
> > >  If (as I said in the patch summary) raw_string_ostream were changed to 
> > > be guaranteed to not need flushing
> > 
> > This sounds like a one-line patch; might be better to just do it rather 
> > than having to churn all these things twice.
> 
> I guess this change kind of freaks me out. Currently you can call 
> `SetBuffered()` on `raw_string_ostream` (though I don't know why you 
> would...), which creates an intermediate buffer and then `flush()` syncs the 
> buffer with the underlying `std::string&`. Removing that ability would be a 
> breaking change, and I'm not sure how we could make it while being confident 
> we're not breaking anything downstream. 
> 
> (On the other hand, you can call `SetBuffered()` on `raw_svector_ostream` 
> too, whose documentation more or less says it doesn't support buffering. If 
> I'm reading right, you get an assert failure in `~raw_ostream()` if you do.)
I don't think we need to be afraid. `raw_ostream` seems like a bit of a leaky 
abstraction, but IMO we need to trust that code doesn't arbitrarily call 
`SetBuffered()` on without full control. It's essentially never the right thing 
to do except from subclasses (where it could be `protected`) and maybe in unit 
tests for convenience. As you pointed, out this would also break 
`svector_stream`.

What I'd be more worried about is the micro-performance-penalty of std::string 
null-terminating with every write. But probably that's not particularly 
important either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Logan Smith via Phabricator via cfe-commits]

logan-5 updated this revision to Diff 392996.
logan-5 added a comment.

Eliminate some explicit `.flush()`es by using temporary `raw_string_ostream`s 
where possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/Testing/TestClangConfig.h
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclarationName.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/lib/Basic/Version.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/TestModuleFileExtension.cpp
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -203,7 +203,8 @@
 
 OS << "]";
 
-return OS.str();
+OS.flush();
+return Storage;
   }
 
   std::string dumpNodes(ArrayRef Nodes) {
@@ -218,7 +219,8 @@
 
 OS << "]";
 
-return OS.str();
+OS.flush();
+return Storage;
   }
 };
 
Index: clang/unittests/Analysis/MacroExpansionContextTest.cpp
===
--- clang/unittests/Analysis/MacroExpansionContextTest.cpp
+++ clang/unittests/Analysis/MacroExpansionContextTest.cpp
@@ -95,14 +95,16 @@
 std::string Buf;
 llvm::raw_string_ostream OS{Buf};
 Ctx.dumpExpandedTextsToStream(OS);
-return OS.str();
+OS.flush();
+return Buf;
   }
 
   static std::string dumpExpansionRanges(const MacroExpansionContext ) {
 std::string Buf;
 llvm::raw_string_ostream OS{Buf};
 Ctx.dumpExpansionRangesToStream(OS);
-return OS.str();
+OS.flush();
+return Buf;
   }
 };
 
Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -111,7 +111,8 @@
 
   Dumper.Visit(std::forward(N)...);
 
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }
 
 template 
@@ -126,7 +127,8 @@
 
   Dumper.Visit(std::forward(N)...);
 
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }
 
 const FunctionDecl *getFunctionNode(clang::ASTUnit *AST,
Index: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
===
--- clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -139,7 +139,8 @@
 
   Passes.run(*M);
 
-  return OS.str();
+  OS.flush();
+  return outString;
 }
 
 // Takes a function and runs it on a set of inputs
Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -263,7 +263,8 @@
   OS << " ";
 }
   });
-  return OS.str();
+  OS.flush();
+  return Storage;
 }
 
 void syntax::Node::assertInvariants() const {
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -927,5 +927,6 @@
   M.EndExpanded);
 }
   }
-  return OS.str();
+  OS.flush();
+  return Dump;
 }
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -444,7 +444,8 @@
   std::string s;
   llvm::raw_string_ostream os(s);
   dumpToStream(os);
-  return os.str();
+  os.flush();
+  return s;
 }
 
 void MemRegion::dumpToStream(raw_ostream ) const {
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -437,7 +437,8 @@
   for (auto BI : *Buf)
 os << BI;
 
-  return os.str();
+  os.flush();
+  return file;
 }
 
 void HTMLDiagnostics::dumpCoverageData(
@@ -534,7 +535,8 @@
 
 )<<<";
 
-  return os.str();
+  os.flush();
+  return s;
 }
 
 void 

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Like, that's the whole reason why `nonloc::LocAsInteger` exists: so that we 
could cast a pointer to an integer and actually have a way to represent the 
resulting value as `NonLoc`.

I'm confident that there's a way to get it right, simply because the program 
under analysis is type-correct. If it's the simplifier, let's fix the 
simplifier. If the original value is not verbose enough, let's fix the original 
value. But I really think we should keep this assertion working 24/7. I'm sure 
when you find the root cause it'll all make sense to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D115409: [SelectionDAGBuilder] drop special handling for CallBr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 392994.
nickdesaulniers added a comment.

- phab sux


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115409

Files:
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll

Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -204,3 +204,31 @@
   %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
   ret i32 %retval.0
 }
+
+; Test5 - test that we don't rely on a positional constraint. ie. +r in
+; GCCAsmStmt being turned into "={esp},0" since after D87279 they're turned
+; into "={esp},{esp}". This previously caused an ICE; this test is more so
+; about avoiding another ICE than what the actual output is.
+define dso_local void @test5() {
+; CHECK-LABEL: test5:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:#APP
+; CHECK-NEXT:#NO_APP
+; CHECK-NEXT:  .Ltmp6: # Block address taken
+; CHECK-NEXT:  # %bb.1:
+; CHECK-NEXT:retl
+  %1 = call i32 @llvm.read_register.i32(metadata !3)
+  %2 = callbr i32 asm "", "={esp},i,{esp},~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@test5, %4), i32 %1)
+  to label %3 [label %4]
+
+3:
+  call void @llvm.write_register.i32(metadata !3, i32 %2)
+  br label %4
+
+4:
+  ret void
+}
+
+declare i32 @llvm.read_register.i32(metadata)
+declare void @llvm.write_register.i32(metadata, i32)
+!3 = !{!"esp"}
Index: llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
+++ llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
@@ -5,7 +5,10 @@
 ; inlineasm_br. Not sure how to get a MachineIR change so this reads the debug
 ; output from SelectionDAG.
 
-; CHECK: t0: ch = EntryToken
+; CHECK: Initial selection DAG: %bb.0 'test:entry'
+; CHECK-NEXT: SelectionDAG has 33 nodes:
+; CHECK-NEXT: t0: ch = EntryToken
+; CHECK-NEXT: t16: i64 = BlockAddress<@test, %fail> 0
 ; CHECK-NEXT: t4: i32,ch = CopyFromReg t0, Register:i32 %3
 ; CHECK-NEXT: t10: i32 = add t4, Constant:i32<1>
 ; CHECK-NEXT: t12: ch = CopyToReg t0, Register:i32 %0, t10
@@ -16,7 +19,8 @@
 ; CHECK-NEXT: t2: i32,ch = CopyFromReg t0, Register:i32 %2
 ; CHECK-NEXT: t8: i32 = add t2, Constant:i32<4>
 ; CHECK-NEXT: t22: ch,glue = CopyToReg t17, Register:i32 %5, t8
-; CHECK-NEXT: t29: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t30: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t32: ch = br t30, BasicBlock:ch
 
 define i32 @test(i32 %a, i32 %b, i32 %c) {
 entry:
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -8553,7 +8553,6 @@
 
   unsigned ArgNo = 0;   // ArgNo - The argument of the CallInst.
   unsigned ResNo = 0;   // ResNo - The result number of the next output.
-  unsigned NumMatchingOps = 0;
   for (auto  : TargetConstraints) {
 ConstraintOperands.push_back(SDISelAsmOperandInfo(T));
 SDISelAsmOperandInfo  = ConstraintOperands.back();
@@ -8563,19 +8562,7 @@
 (OpInfo.Type == InlineAsm::isOutput && OpInfo.isIndirect)) {
   OpInfo.CallOperandVal = Call.getArgOperand(ArgNo++);
 
-  // Process the call argument. BasicBlocks are labels, currently appearing
-  // only in asm's.
-  if (isa(Call) &&
-  ArgNo - 1 >= (cast()->arg_size() -
-cast()->getNumIndirectDests() -
-NumMatchingOps) &&
-  (NumMatchingOps == 0 ||
-   ArgNo - 1 <
-   (cast()->arg_size() - NumMatchingOps))) {
-const auto *BA = cast(OpInfo.CallOperandVal);
-EVT VT = TLI.getValueType(DAG.getDataLayout(), BA->getType(), true);
-OpInfo.CallOperand = DAG.getTargetBlockAddress(BA, VT);
-  } else if (const auto *BB = dyn_cast(OpInfo.CallOperandVal)) {
+  if (const auto *BB = dyn_cast(OpInfo.CallOperandVal)) {
 OpInfo.CallOperand = DAG.getBasicBlock(FuncInfo.MBBMap[BB]);
   } else {
 OpInfo.CallOperand = getValue(OpInfo.CallOperandVal);
@@ -8601,9 +8588,6 @@
   OpInfo.ConstraintVT = MVT::Other;
 }
 
-if (OpInfo.hasMatchingInput())
-  ++NumMatchingOps;
-
 if (!HasSideEffect)
   HasSideEffect = OpInfo.hasMemory(TLI);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 392993.
nickdesaulniers added a comment.

- phab sux


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410

Files:
  llvm/test/Assembler/call-arg-is-callee.ll
  llvm/test/Bitcode/callbr.ll
  llvm/test/Bitcode/callbr.ll.bc
  llvm/test/CodeGen/AArch64/callbr-asm-label.ll
  llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll
  llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
  llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll
  llvm/test/CodeGen/SystemZ/asm-20.ll
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-blockplacement.ll
  llvm/test/CodeGen/X86/callbr-asm-branch-folding.ll
  llvm/test/CodeGen/X86/callbr-asm-destinations.ll
  llvm/test/CodeGen/X86/callbr-asm-errors.ll
  llvm/test/CodeGen/X86/callbr-asm-instr-scheduling.ll
  llvm/test/CodeGen/X86/callbr-asm-kill.mir
  llvm/test/CodeGen/X86/callbr-asm-label-addr.ll
  llvm/test/CodeGen/X86/callbr-asm-obj-file.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs-pred-succ.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm-phi-placement.ll
  llvm/test/CodeGen/X86/callbr-asm-sink.ll
  llvm/test/CodeGen/X86/callbr-asm.ll
  llvm/test/CodeGen/X86/callbr-codegenprepare.ll
  llvm/test/CodeGen/X86/shrinkwrap-callbr.ll
  llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
  llvm/test/Transforms/CallSiteSplitting/callsite-split-callbr.ll
  llvm/test/Transforms/Coroutines/coro-debug.ll
  llvm/test/Transforms/GVN/callbr-loadpre-critedge.ll
  llvm/test/Transforms/GVN/callbr-scalarpre-critedge.ll
  llvm/test/Transforms/GVN/critical-edge-split-failure.ll
  llvm/test/Transforms/IROutliner/illegal-callbr.ll
  llvm/test/Transforms/Inline/blockaddress.ll
  llvm/test/Transforms/Inline/callbr.ll
  llvm/test/Transforms/JumpThreading/callbr-edge-split.ll
  llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
  llvm/test/Transforms/LICM/callbr-crash.ll
  llvm/test/Transforms/LoopDeletion/two-predecessors.ll
  llvm/test/Transforms/LoopRotate/callbr.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting2.ll
  llvm/test/Transforms/LoopUnroll/callbr.ll
  llvm/test/Transforms/LoopUnswitch/callbr.ll
  llvm/test/Transforms/PGOProfile/callbr.ll
  llvm/test/Transforms/SimpleLoopUnswitch/not-safe-to-clone.ll
  llvm/test/Transforms/SimplifyCFG/callbr-destinations.ll
  llvm/test/Verifier/callbr.ll
  llvm/test/tools/llvm-diff/callbr.ll
  llvm/test/tools/llvm-diff/phinode.ll
  
llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll

Index: llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
===
--- llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
+++ llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
@@ -13,9 +13,9 @@
 bb4:
 ; CHECK-INTERESTINGNESS: callbr void asm
 ; CHECK-INTERESTINGNESS-SAME: blockaddress
-; CHECK-FINAL: callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+; CHECK-FINAL: callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
 ; CHECK-ALL: to label %bb5 [label %bb11]
-  callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+  callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
   to label %bb5 [label %bb11]
 
 ; CHECK-ALL: bb5:
Index: llvm/test/tools/llvm-diff/phinode.ll
===
--- llvm/test/tools/llvm-diff/phinode.ll
+++ llvm/test/tools/llvm-diff/phinode.ll
@@ -9,7 +9,7 @@
 ; CHECK-NEXT:<   %7 = phi i32 [ 0, %2 ], [ -1, %1 ]
 ; CHECK-NEXT:<   ret i32 %7
 define i32 @foo(i32 %0) #0 {
-  callbr void asm sideeffect "", "X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %6))
+  callbr void asm sideeffect "", "i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %6))
   to label %2 [label %6]
 
 2:
Index: llvm/test/tools/llvm-diff/callbr.ll
===
--- llvm/test/tools/llvm-diff/callbr.ll
+++ llvm/test/tools/llvm-diff/callbr.ll
@@ -2,7 +2,7 @@
 
 define void @foo() {
 entry:
-  callbr void asm sideeffect "", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %return), i8* blockaddress(@foo, %t_no))
+  callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %return), i8* blockaddress(@foo, %t_no))
   to label %asm.fallthrough [label %return, label %t_no]
 
 asm.fallthrough:
@@ -18,14 +18,14 @@
 ; CHECK:  in function bar:
 ; CHECK-NOT:  in function foo:
 ; CHECK-NEXT:  in block %entry:
-; CHECK-NEXT:>   callbr void asm 

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 392990.
nickdesaulniers added a comment.

- update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410

Files:
  llvm/test/Assembler/call-arg-is-callee.ll
  llvm/test/Bitcode/callbr.ll
  llvm/test/Bitcode/callbr.ll.bc
  llvm/test/CodeGen/AArch64/callbr-asm-label.ll
  llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll
  llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
  llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll
  llvm/test/CodeGen/SystemZ/asm-20.ll
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-blockplacement.ll
  llvm/test/CodeGen/X86/callbr-asm-branch-folding.ll
  llvm/test/CodeGen/X86/callbr-asm-destinations.ll
  llvm/test/CodeGen/X86/callbr-asm-errors.ll
  llvm/test/CodeGen/X86/callbr-asm-instr-scheduling.ll
  llvm/test/CodeGen/X86/callbr-asm-kill.mir
  llvm/test/CodeGen/X86/callbr-asm-label-addr.ll
  llvm/test/CodeGen/X86/callbr-asm-obj-file.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs-pred-succ.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm-phi-placement.ll
  llvm/test/CodeGen/X86/callbr-asm-sink.ll
  llvm/test/CodeGen/X86/callbr-asm.ll
  llvm/test/CodeGen/X86/callbr-codegenprepare.ll
  llvm/test/CodeGen/X86/shrinkwrap-callbr.ll
  llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
  llvm/test/Transforms/CallSiteSplitting/callsite-split-callbr.ll
  llvm/test/Transforms/Coroutines/coro-debug.ll
  llvm/test/Transforms/GVN/callbr-loadpre-critedge.ll
  llvm/test/Transforms/GVN/callbr-scalarpre-critedge.ll
  llvm/test/Transforms/GVN/critical-edge-split-failure.ll
  llvm/test/Transforms/IROutliner/illegal-callbr.ll
  llvm/test/Transforms/Inline/blockaddress.ll
  llvm/test/Transforms/Inline/callbr.ll
  llvm/test/Transforms/JumpThreading/callbr-edge-split.ll
  llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
  llvm/test/Transforms/LICM/callbr-crash.ll
  llvm/test/Transforms/LoopDeletion/two-predecessors.ll
  llvm/test/Transforms/LoopRotate/callbr.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting2.ll
  llvm/test/Transforms/LoopUnroll/callbr.ll
  llvm/test/Transforms/LoopUnswitch/callbr.ll
  llvm/test/Transforms/PGOProfile/callbr.ll
  llvm/test/Transforms/SimpleLoopUnswitch/not-safe-to-clone.ll
  llvm/test/Transforms/SimplifyCFG/callbr-destinations.ll
  llvm/test/Verifier/callbr.ll
  llvm/test/tools/llvm-diff/callbr.ll
  llvm/test/tools/llvm-diff/phinode.ll
  
llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll

Index: llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
===
--- llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
+++ llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
@@ -13,9 +13,9 @@
 bb4:
 ; CHECK-INTERESTINGNESS: callbr void asm
 ; CHECK-INTERESTINGNESS-SAME: blockaddress
-; CHECK-FINAL: callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+; CHECK-FINAL: callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
 ; CHECK-ALL: to label %bb5 [label %bb11]
-  callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+  callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
   to label %bb5 [label %bb11]
 
 ; CHECK-ALL: bb5:
Index: llvm/test/tools/llvm-diff/phinode.ll
===
--- llvm/test/tools/llvm-diff/phinode.ll
+++ llvm/test/tools/llvm-diff/phinode.ll
@@ -9,7 +9,7 @@
 ; CHECK-NEXT:<   %7 = phi i32 [ 0, %2 ], [ -1, %1 ]
 ; CHECK-NEXT:<   ret i32 %7
 define i32 @foo(i32 %0) #0 {
-  callbr void asm sideeffect "", "X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %6))
+  callbr void asm sideeffect "", "i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %6))
   to label %2 [label %6]
 
 2:
Index: llvm/test/tools/llvm-diff/callbr.ll
===
--- llvm/test/tools/llvm-diff/callbr.ll
+++ llvm/test/tools/llvm-diff/callbr.ll
@@ -2,7 +2,7 @@
 
 define void @foo() {
 entry:
-  callbr void asm sideeffect "", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %return), i8* blockaddress(@foo, %t_no))
+  callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %return), i8* blockaddress(@foo, %t_no))
   to label %asm.fallthrough [label %return, label %t_no]
 
 asm.fallthrough:
@@ -18,14 +18,14 @@
 ; CHECK:  in function bar:
 ; CHECK-NOT:  in function foo:
 ; CHECK-NEXT:  in block %entry:
-; CHECK-NEXT:>   callbr void asm 

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> There is the reinterpret-cast operation which is capable of crossing these 
> two domains, producing an expression that can participate in arithmetic 
> operations, but on the abstract domain side, we still stick to Locs

Such cast should turn the `loc::ConcreteInt` into a `nonloc::ConcreteInt` with 
the same integral value.

The distinction between Loc and NonLoc is very important. It's at the core of 
our type correctness. We should fight tooth and nail to preserve it because 
assertions such as the one removed here help us discover a lot of bugs in other 
places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D115409: [SelectionDAGBuilder] drop special handling for CallBr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 392988.
nickdesaulniers added a comment.
Herald added subscribers: cfe-commits, wenlei, asbirlea, zzheng, kbarton, 
nemanjai.
Herald added a project: clang.

- I'm just wrestling with arc at this point


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115409

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/CodeGen/asm.c
  clang/test/Modules/asm-goto.c
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/Assembler/call-arg-is-callee.ll
  llvm/test/Bitcode/callbr.ll
  llvm/test/Bitcode/callbr.ll.bc
  llvm/test/CodeGen/AArch64/callbr-asm-label.ll
  llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll
  llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
  llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll
  llvm/test/CodeGen/SystemZ/asm-20.ll
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-blockplacement.ll
  llvm/test/CodeGen/X86/callbr-asm-branch-folding.ll
  llvm/test/CodeGen/X86/callbr-asm-destinations.ll
  llvm/test/CodeGen/X86/callbr-asm-errors.ll
  llvm/test/CodeGen/X86/callbr-asm-instr-scheduling.ll
  llvm/test/CodeGen/X86/callbr-asm-kill.mir
  llvm/test/CodeGen/X86/callbr-asm-label-addr.ll
  llvm/test/CodeGen/X86/callbr-asm-obj-file.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs-pred-succ.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm-phi-placement.ll
  llvm/test/CodeGen/X86/callbr-asm-sink.ll
  llvm/test/CodeGen/X86/callbr-asm.ll
  llvm/test/CodeGen/X86/callbr-codegenprepare.ll
  llvm/test/CodeGen/X86/shrinkwrap-callbr.ll
  llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
  llvm/test/Transforms/CallSiteSplitting/callsite-split-callbr.ll
  llvm/test/Transforms/Coroutines/coro-debug.ll
  llvm/test/Transforms/GVN/callbr-loadpre-critedge.ll
  llvm/test/Transforms/GVN/callbr-scalarpre-critedge.ll
  llvm/test/Transforms/GVN/critical-edge-split-failure.ll
  llvm/test/Transforms/IROutliner/illegal-callbr.ll
  llvm/test/Transforms/Inline/blockaddress.ll
  llvm/test/Transforms/Inline/callbr.ll
  llvm/test/Transforms/JumpThreading/callbr-edge-split.ll
  llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
  llvm/test/Transforms/LICM/callbr-crash.ll
  llvm/test/Transforms/LoopDeletion/two-predecessors.ll
  llvm/test/Transforms/LoopRotate/callbr.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting2.ll
  llvm/test/Transforms/LoopUnroll/callbr.ll
  llvm/test/Transforms/LoopUnswitch/callbr.ll
  llvm/test/Transforms/PGOProfile/callbr.ll
  llvm/test/Transforms/SimpleLoopUnswitch/not-safe-to-clone.ll
  llvm/test/Transforms/SimplifyCFG/callbr-destinations.ll
  llvm/test/Verifier/callbr.ll
  llvm/test/tools/llvm-diff/callbr.ll
  llvm/test/tools/llvm-diff/phinode.ll
  
llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll

Index: llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
===
--- llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
+++ llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
@@ -13,9 +13,9 @@
 bb4:
 ; CHECK-INTERESTINGNESS: callbr void asm
 ; CHECK-INTERESTINGNESS-SAME: blockaddress
-; CHECK-FINAL: callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+; CHECK-FINAL: callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
 ; CHECK-ALL: to label %bb5 [label %bb11]
-  callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+  callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
   to label %bb5 [label %bb11]
 
 ; CHECK-ALL: bb5:
Index: llvm/test/tools/llvm-diff/phinode.ll
===
--- llvm/test/tools/llvm-diff/phinode.ll
+++ llvm/test/tools/llvm-diff/phinode.ll
@@ -9,7 +9,7 @@
 ; CHECK-NEXT:<   %7 = phi i32 [ 0, %2 ], [ -1, %1 ]
 ; CHECK-NEXT:<   ret i32 %7
 define i32 @foo(i32 %0) #0 {
-  callbr void asm sideeffect "", "X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %6))
+  callbr void asm sideeffect "", "i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %6))
   to label %2 [label %6]
 
 2:
Index: llvm/test/tools/llvm-diff/callbr.ll
===
--- llvm/test/tools/llvm-diff/callbr.ll
+++ llvm/test/tools/llvm-diff/callbr.ll
@@ -2,7 +2,7 @@
 
 define void @foo() {
 entry:
-  callbr void asm sideeffect "", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %return), i8* blockaddress(@foo, %t_no))
+  callbr void asm sideeffect "", 

[PATCH] D115311: [clang][CGStmt] emit i constraint rather than X for asm goto indirect dests

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 392984.
nickdesaulniers retitled this revision from "[WIP] alternative approach to 
D114895" to "[clang][CGStmt] emit i constraint rather than X for asm goto 
indirect dests".
nickdesaulniers edited the summary of this revision.
nickdesaulniers added a comment.

- verbatim


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115311

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/CodeGen/asm.c
  clang/test/Modules/asm-goto.c
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll

Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -204,3 +204,31 @@
   %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
   ret i32 %retval.0
 }
+
+; Test5 - test that we don't rely on a positional constraint. ie. +r in
+; GCCAsmStmt being turned into "={esp},0" since after D87279 they're turned
+; into "={esp},{esp}". This previously caused an ICE; this test is more so
+; about avoiding another ICE than what the actual output is.
+define dso_local void @test5() {
+; CHECK-LABEL: test5:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:#APP
+; CHECK-NEXT:#NO_APP
+; CHECK-NEXT:  .Ltmp6: # Block address taken
+; CHECK-NEXT:  # %bb.1:
+; CHECK-NEXT:retl
+  %1 = call i32 @llvm.read_register.i32(metadata !3)
+  %2 = callbr i32 asm "", "={esp},i,{esp},~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@test5, %4), i32 %1)
+  to label %3 [label %4]
+
+3:
+  call void @llvm.write_register.i32(metadata !3, i32 %2)
+  br label %4
+
+4:
+  ret void
+}
+
+declare i32 @llvm.read_register.i32(metadata)
+declare void @llvm.write_register.i32(metadata, i32)
+!3 = !{!"esp"}
Index: llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
+++ llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
@@ -5,7 +5,10 @@
 ; inlineasm_br. Not sure how to get a MachineIR change so this reads the debug
 ; output from SelectionDAG.
 
-; CHECK: t0: ch = EntryToken
+; CHECK: Initial selection DAG: %bb.0 'test:entry'
+; CHECK-NEXT: SelectionDAG has 33 nodes:
+; CHECK-NEXT: t0: ch = EntryToken
+; CHECK-NEXT: t16: i64 = BlockAddress<@test, %fail> 0
 ; CHECK-NEXT: t4: i32,ch = CopyFromReg t0, Register:i32 %3
 ; CHECK-NEXT: t10: i32 = add t4, Constant:i32<1>
 ; CHECK-NEXT: t12: ch = CopyToReg t0, Register:i32 %0, t10
@@ -16,7 +19,8 @@
 ; CHECK-NEXT: t2: i32,ch = CopyFromReg t0, Register:i32 %2
 ; CHECK-NEXT: t8: i32 = add t2, Constant:i32<4>
 ; CHECK-NEXT: t22: ch,glue = CopyToReg t17, Register:i32 %5, t8
-; CHECK-NEXT: t29: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t30: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t32: ch = br t30, BasicBlock:ch
 
 define i32 @test(i32 %a, i32 %b, i32 %c) {
 entry:
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -8553,7 +8553,6 @@
 
   unsigned ArgNo = 0;   // ArgNo - The argument of the CallInst.
   unsigned ResNo = 0;   // ResNo - The result number of the next output.
-  unsigned NumMatchingOps = 0;
   for (auto  : TargetConstraints) {
 ConstraintOperands.push_back(SDISelAsmOperandInfo(T));
 SDISelAsmOperandInfo  = ConstraintOperands.back();
@@ -8563,19 +8562,7 @@
 (OpInfo.Type == InlineAsm::isOutput && OpInfo.isIndirect)) {
   OpInfo.CallOperandVal = Call.getArgOperand(ArgNo++);
 
-  // Process the call argument. BasicBlocks are labels, currently appearing
-  // only in asm's.
-  if (isa(Call) &&
-  ArgNo - 1 >= (cast()->arg_size() -
-cast()->getNumIndirectDests() -
-NumMatchingOps) &&
-  (NumMatchingOps == 0 ||
-   ArgNo - 1 <
-   (cast()->arg_size() - NumMatchingOps))) {
-const auto *BA = cast(OpInfo.CallOperandVal);
-EVT VT = TLI.getValueType(DAG.getDataLayout(), BA->getType(), true);
-OpInfo.CallOperand = DAG.getTargetBlockAddress(BA, VT);
-  } else if (const auto *BB = dyn_cast(OpInfo.CallOperandVal)) {
+  if (const auto *BB = dyn_cast(OpInfo.CallOperandVal)) {
 OpInfo.CallOperand = DAG.getBasicBlock(FuncInfo.MBBMap[BB]);
   } else {
 OpInfo.CallOperand = getValue(OpInfo.CallOperandVal);
@@ -8601,9 +8588,6 @@
   OpInfo.ConstraintVT = MVT::Other;
 }
 
-if (OpInfo.hasMatchingInput())
-  ++NumMatchingOps;
-
 if (!HasSideEffect)
   

[PATCH] D115311: [WIP] alternative approach to D114895

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 392983.
nickdesaulniers added a comment.
Herald added subscribers: llvm-commits, pengfei, hiraditya.
Herald added a project: LLVM.

- write an actual commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115311

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/CodeGen/asm.c
  clang/test/Modules/asm-goto.c
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll

Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -204,3 +204,31 @@
   %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
   ret i32 %retval.0
 }
+
+; Test5 - test that we don't rely on a positional constraint. ie. +r in
+; GCCAsmStmt being turned into "={esp},0" since after D87279 they're turned
+; into "={esp},{esp}". This previously caused an ICE; this test is more so
+; about avoiding another ICE than what the actual output is.
+define dso_local void @test5() {
+; CHECK-LABEL: test5:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:#APP
+; CHECK-NEXT:#NO_APP
+; CHECK-NEXT:  .Ltmp6: # Block address taken
+; CHECK-NEXT:  # %bb.1:
+; CHECK-NEXT:retl
+  %1 = call i32 @llvm.read_register.i32(metadata !3)
+  %2 = callbr i32 asm "", "={esp},i,{esp},~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@test5, %4), i32 %1)
+  to label %3 [label %4]
+
+3:
+  call void @llvm.write_register.i32(metadata !3, i32 %2)
+  br label %4
+
+4:
+  ret void
+}
+
+declare i32 @llvm.read_register.i32(metadata)
+declare void @llvm.write_register.i32(metadata, i32)
+!3 = !{!"esp"}
Index: llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
+++ llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
@@ -5,7 +5,10 @@
 ; inlineasm_br. Not sure how to get a MachineIR change so this reads the debug
 ; output from SelectionDAG.
 
-; CHECK: t0: ch = EntryToken
+; CHECK: Initial selection DAG: %bb.0 'test:entry'
+; CHECK-NEXT: SelectionDAG has 33 nodes:
+; CHECK-NEXT: t0: ch = EntryToken
+; CHECK-NEXT: t16: i64 = BlockAddress<@test, %fail> 0
 ; CHECK-NEXT: t4: i32,ch = CopyFromReg t0, Register:i32 %3
 ; CHECK-NEXT: t10: i32 = add t4, Constant:i32<1>
 ; CHECK-NEXT: t12: ch = CopyToReg t0, Register:i32 %0, t10
@@ -16,7 +19,8 @@
 ; CHECK-NEXT: t2: i32,ch = CopyFromReg t0, Register:i32 %2
 ; CHECK-NEXT: t8: i32 = add t2, Constant:i32<4>
 ; CHECK-NEXT: t22: ch,glue = CopyToReg t17, Register:i32 %5, t8
-; CHECK-NEXT: t29: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t30: ch,glue = inlineasm_br t22, {{.*}}, t22:1
+; CHECK-NEXT: t32: ch = br t30, BasicBlock:ch
 
 define i32 @test(i32 %a, i32 %b, i32 %c) {
 entry:
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -8553,7 +8553,6 @@
 
   unsigned ArgNo = 0;   // ArgNo - The argument of the CallInst.
   unsigned ResNo = 0;   // ResNo - The result number of the next output.
-  unsigned NumMatchingOps = 0;
   for (auto  : TargetConstraints) {
 ConstraintOperands.push_back(SDISelAsmOperandInfo(T));
 SDISelAsmOperandInfo  = ConstraintOperands.back();
@@ -8563,19 +8562,7 @@
 (OpInfo.Type == InlineAsm::isOutput && OpInfo.isIndirect)) {
   OpInfo.CallOperandVal = Call.getArgOperand(ArgNo++);
 
-  // Process the call argument. BasicBlocks are labels, currently appearing
-  // only in asm's.
-  if (isa(Call) &&
-  ArgNo - 1 >= (cast()->arg_size() -
-cast()->getNumIndirectDests() -
-NumMatchingOps) &&
-  (NumMatchingOps == 0 ||
-   ArgNo - 1 <
-   (cast()->arg_size() - NumMatchingOps))) {
-const auto *BA = cast(OpInfo.CallOperandVal);
-EVT VT = TLI.getValueType(DAG.getDataLayout(), BA->getType(), true);
-OpInfo.CallOperand = DAG.getTargetBlockAddress(BA, VT);
-  } else if (const auto *BB = dyn_cast(OpInfo.CallOperandVal)) {
+  if (const auto *BB = dyn_cast(OpInfo.CallOperandVal)) {
 OpInfo.CallOperand = DAG.getBasicBlock(FuncInfo.MBBMap[BB]);
   } else {
 OpInfo.CallOperand = getValue(OpInfo.CallOperandVal);
@@ -8601,9 +8588,6 @@
   OpInfo.ConstraintVT = MVT::Other;
 }
 
-if (OpInfo.hasMatchingInput())
-  ++NumMatchingOps;
-
 if (!HasSideEffect)
   HasSideEffect = OpInfo.hasMemory(TLI);
 
Index: clang/test/Modules/asm-goto.c

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
Herald added subscribers: wenlei, pengfei, asbirlea, zzheng, kbarton, 
hiraditya, nemanjai.
nickdesaulniers requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

In D115311 , we're looking to modify clang to 
emit i constraints rather
than X constraints for callbr's indirect destinations. Prior to doing
so, update all of the existing tests in llvm/ to match.

[clang][CGStmt] emit i constraint rather than X for asm goto indirect dests

As suggested in:
https://reviews.llvm.org/D114895#3177794
this will help simplify SelectionDAGISEL.

[SelectionDAGBuilder] drop special handling for CallBr

Now that indirect destinations are using "i" constraints rather than "X"
we no longer need this special handling.

Add a test case that was previously ICE'ing.

Reported-by: kernel test robot 
Fixes: https://github.com/ClangBuiltLinux/linux/issues/1512


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115410

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/CodeGen/asm.c
  clang/test/Modules/asm-goto.c
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/Assembler/call-arg-is-callee.ll
  llvm/test/Bitcode/callbr.ll
  llvm/test/Bitcode/callbr.ll.bc
  llvm/test/CodeGen/AArch64/callbr-asm-label.ll
  llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll
  llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
  llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll
  llvm/test/CodeGen/SystemZ/asm-20.ll
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-blockplacement.ll
  llvm/test/CodeGen/X86/callbr-asm-branch-folding.ll
  llvm/test/CodeGen/X86/callbr-asm-destinations.ll
  llvm/test/CodeGen/X86/callbr-asm-errors.ll
  llvm/test/CodeGen/X86/callbr-asm-instr-scheduling.ll
  llvm/test/CodeGen/X86/callbr-asm-kill.mir
  llvm/test/CodeGen/X86/callbr-asm-label-addr.ll
  llvm/test/CodeGen/X86/callbr-asm-obj-file.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs-pred-succ.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm-phi-placement.ll
  llvm/test/CodeGen/X86/callbr-asm-sink.ll
  llvm/test/CodeGen/X86/callbr-asm.ll
  llvm/test/CodeGen/X86/callbr-codegenprepare.ll
  llvm/test/CodeGen/X86/shrinkwrap-callbr.ll
  llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
  llvm/test/Transforms/CallSiteSplitting/callsite-split-callbr.ll
  llvm/test/Transforms/Coroutines/coro-debug.ll
  llvm/test/Transforms/GVN/callbr-loadpre-critedge.ll
  llvm/test/Transforms/GVN/callbr-scalarpre-critedge.ll
  llvm/test/Transforms/GVN/critical-edge-split-failure.ll
  llvm/test/Transforms/IROutliner/illegal-callbr.ll
  llvm/test/Transforms/Inline/blockaddress.ll
  llvm/test/Transforms/Inline/callbr.ll
  llvm/test/Transforms/JumpThreading/callbr-edge-split.ll
  llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
  llvm/test/Transforms/LICM/callbr-crash.ll
  llvm/test/Transforms/LoopDeletion/two-predecessors.ll
  llvm/test/Transforms/LoopRotate/callbr.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting2.ll
  llvm/test/Transforms/LoopUnroll/callbr.ll
  llvm/test/Transforms/LoopUnswitch/callbr.ll
  llvm/test/Transforms/PGOProfile/callbr.ll
  llvm/test/Transforms/SimpleLoopUnswitch/not-safe-to-clone.ll
  llvm/test/Transforms/SimplifyCFG/callbr-destinations.ll
  llvm/test/Verifier/callbr.ll
  llvm/test/tools/llvm-diff/callbr.ll
  llvm/test/tools/llvm-diff/phinode.ll
  
llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll

Index: llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
===
--- llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
+++ llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
@@ -13,9 +13,9 @@
 bb4:
 ; CHECK-INTERESTINGNESS: callbr void asm
 ; CHECK-INTERESTINGNESS-SAME: blockaddress
-; CHECK-FINAL: callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+; CHECK-FINAL: callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
 ; CHECK-ALL: to label %bb5 [label %bb11]
-  callbr void asm sideeffect "", "X"(i8* blockaddress(@func, %bb11))
+  callbr void asm sideeffect "", "i"(i8* blockaddress(@func, %bb11))
   to label %bb5 [label %bb11]
 
 ; CHECK-ALL: bb5:
Index: llvm/test/tools/llvm-diff/phinode.ll
===
--- llvm/test/tools/llvm-diff/phinode.ll
+++ llvm/test/tools/llvm-diff/phinode.ll
@@ -9,7 +9,7 @@
 ; CHECK-NEXT:<   %7 = phi i32 [ 0, %2 ], [ -1, %1 ]
 ; CHECK-NEXT:<   ret i32 %7
 

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Logan Smith via Phabricator via cfe-commits]

logan-5 added a comment.

In D115374#3181383 , @dexonsmith 
wrote:

> I don't feel strongly, but IMO the code might be a bit harder to 
> read/maintain with the explicit flush. I worry that it'd be easy to move the 
> `flush()` away from the `return`. Not sure I'm right; could just be 
> familiarity with `str()`.

I definitely hear you. I don't really mind it personally, and I did it this way 
because I saw precedent in a couple spots (there's one on 
CompilerInvocation.cpp:653, several in clangd, etc.). I definitely see how it 
could be a little bit spooky though.

>   std::string Str;
>   llvm::raw_string_ostream(Str) << ...;
>   return Str;

I like this idea. I'd be happy to go back through and change the simple ones to 
this pattern.

> Another option:
>
>   std::string Result;
>   llvm::raw_string_ostream OS(Str);
>   OS << ...;
>   return OS.take();
>
> Where `raw_string_ostream::take()` is just `return std::move(str())`. It 
> doesn't get NRVO, but I'm not sure that really matters in most of these 
> places. Benefit is that it's a minimal change and the name is clear / matches 
> other LLVM things.

I suppose the question then becomes whether to name it `take()` or `str() &&` 
for symmetry with C++20's `std::ostringstream`. (Also for the record, I agree 
that NRVOing some std::strings isn't going to make a giant difference; my 
opinion is simply that if we can get it, we might as well.)




Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36
+OS.flush();
+return Str;
   }

dexonsmith wrote:
> logan-5 wrote:
> > Quuxplusone wrote:
> > > FWIW, it appears to me that in most (all?) of these cases, what's really 
> > > wanted is not "a string //and// a stream" but rather "a stream that owns 
> > > a string" (`std::ostringstream` or the LLVM-codebase equivalent thereof). 
> > > Then the return can be `return std::move(OS).str();` — for 
> > > `std::ostringstream`, this Does The Right Thing since C++20, and if LLVM 
> > > had its own stringstream it could make it Do The Right Thing today.
> > > https://en.cppreference.com/w/cpp/io/basic_ostringstream/str
> > > 
> > True enough. Although `return std::move(OS).str();` is still much harder to 
> > type than the less efficient `return OS.str();`, and it requires at minimum 
> > a move of the underlying string--whereas `return Str;` is the easiest of 
> > all to type, and it opens things up for NRVO. If (as I said in the patch 
> > summary) `raw_string_ostream` were changed to be guaranteed to not need 
> > flushing, `return Str;` would IMHO be cemented as the clear winner.
> > 
> > That said, you're clearly right that all these cases are semantically 
> > trying to do "a stream that owns a string", and it's clunky to execute with 
> > the existing APIs.
> >  If (as I said in the patch summary) raw_string_ostream were changed to be 
> > guaranteed to not need flushing
> 
> This sounds like a one-line patch; might be better to just do it rather than 
> having to churn all these things twice.
> >  If (as I said in the patch summary) raw_string_ostream were changed to be 
> > guaranteed to not need flushing
> 
> This sounds like a one-line patch; might be better to just do it rather than 
> having to churn all these things twice.

I guess this change kind of freaks me out. Currently you can call 
`SetBuffered()` on `raw_string_ostream` (though I don't know why you would...), 
which creates an intermediate buffer and then `flush()` syncs the buffer with 
the underlying `std::string&`. Removing that ability would be a breaking 
change, and I'm not sure how we could make it while being confident we're not 
breaking anything downstream. 

(On the other hand, you can call `SetBuffered()` on `raw_svector_ostream` too, 
whose documentation more or less says it doesn't support buffering. If I'm 
reading right, you get an assert failure in `~raw_ostream()` if you do.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

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

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added a comment.

Sure, adding an option is easy, if that's the consensus. What would you call it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114995

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

[PATCH] D115386: AST: Avoid using SmallVector::set_size() in UnresolvedSet

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/lib/Sema/SemaLookup.cpp:623
 
-  Decls.set_size(N);
+  Decls.truncate(N);
 

Two things to confirm here.

First is that the destructors are trivial. From 
clang/include/clang/AST/DeclAccessPair.h:
```
lang=c++
class DeclAccessPair {
  uintptr_t Ptr; // we'd use llvm::PointerUnion, but it isn't trivial
```
(If they hadn't been trivial, then hypothetically there could been other code 
somewhere that ran the destructors later...)

Second is that `set_size()` was only used for truncation. I confirmed that that 
three ways:
- Looking backward, `N` starts as `Decls.size()` and the only changes are 
decrement operatoers.
- Looking forward, there's no code that would initialize / assign to the new 
member (so if it increased size, it would likely have led to problems 
elsewhere).
- Tests pass.


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

https://reviews.llvm.org/D115386

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

[PATCH] D115386: AST: Avoid using SmallVector::set_size() in UnresolvedSet

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith updated this revision to Diff 392977.
dexonsmith added reviewers: Bigcheese, jansvoboda11, vsapsai.
dexonsmith added a comment.

Adding a couple of other possible reviewers.

[no change, just retriggering bots after spurious failure]


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

https://reviews.llvm.org/D115386

Files:
  clang/include/clang/AST/UnresolvedSet.h
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -620,7 +620,7 @@
 getSema().diagnoseEquivalentInternalLinkageDeclarations(
 getNameLoc(), HasNonFunction, EquivalentNonFunctions);
 
-  Decls.set_size(N);
+  Decls.truncate(N);
 
   if (HasNonFunction && (HasFunction || HasUnresolved))
 Ambiguous = true;
Index: clang/include/clang/AST/UnresolvedSet.h
===
--- clang/include/clang/AST/UnresolvedSet.h
+++ clang/include/clang/AST/UnresolvedSet.h
@@ -121,7 +121,7 @@
   void setAccess(iterator I, AccessSpecifier AS) { I.I->setAccess(AS); }
 
   void clear() { decls().clear(); }
-  void set_size(unsigned N) { decls().set_size(N); }
+  void truncate(unsigned N) { decls().truncate(N); }
 
   bool empty() const { return decls().empty(); }
   unsigned size() const { return decls().size(); }


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -620,7 +620,7 @@
 getSema().diagnoseEquivalentInternalLinkageDeclarations(
 getNameLoc(), HasNonFunction, EquivalentNonFunctions);
 
-  Decls.set_size(N);
+  Decls.truncate(N);
 
   if (HasNonFunction && (HasFunction || HasUnresolved))
 Ambiguous = true;
Index: clang/include/clang/AST/UnresolvedSet.h
===
--- clang/include/clang/AST/UnresolvedSet.h
+++ clang/include/clang/AST/UnresolvedSet.h
@@ -121,7 +121,7 @@
   void setAccess(iterator I, AccessSpecifier AS) { I.I->setAccess(AS); }
 
   void clear() { decls().clear(); }
-  void set_size(unsigned N) { decls().set_size(N); }
+  void truncate(unsigned N) { decls().truncate(N); }
 
   bool empty() const { return decls().empty(); }
   unsigned size() const { return decls().size(); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

LLVM build master will be offline for 1 hr for maintenance

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM Lab along with the both production and staging build bots will be
offline for about 1 hrs starting at 7:00 PM PST for maintenance.

Thank you for understanding.

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

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36
+OS.flush();
+return Str;
   }

logan-5 wrote:
> Quuxplusone wrote:
> > FWIW, it appears to me that in most (all?) of these cases, what's really 
> > wanted is not "a string //and// a stream" but rather "a stream that owns a 
> > string" (`std::ostringstream` or the LLVM-codebase equivalent thereof). 
> > Then the return can be `return std::move(OS).str();` — for 
> > `std::ostringstream`, this Does The Right Thing since C++20, and if LLVM 
> > had its own stringstream it could make it Do The Right Thing today.
> > https://en.cppreference.com/w/cpp/io/basic_ostringstream/str
> > 
> True enough. Although `return std::move(OS).str();` is still much harder to 
> type than the less efficient `return OS.str();`, and it requires at minimum a 
> move of the underlying string--whereas `return Str;` is the easiest of all to 
> type, and it opens things up for NRVO. If (as I said in the patch summary) 
> `raw_string_ostream` were changed to be guaranteed to not need flushing, 
> `return Str;` would IMHO be cemented as the clear winner.
> 
> That said, you're clearly right that all these cases are semantically trying 
> to do "a stream that owns a string", and it's clunky to execute with the 
> existing APIs.
>  If (as I said in the patch summary) raw_string_ostream were changed to be 
> guaranteed to not need flushing

This sounds like a one-line patch; might be better to just do it rather than 
having to churn all these things twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

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

[PATCH] D115403: [clang][driver] update the darwin driver to point to correct macho_embedded path

[John Ericson via Phabricator via cfe-commits]

Ericson2314 accepted this revision.
Ericson2314 added a comment.
This revision is now accepted and ready to land.

Looks good. Thanks for fixing this for me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115403

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

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I don't feel strongly, but IMO the code might be a bit harder to read/maintain 
with the explicit flush. I worry that it'd be easy to move the `flush()` away 
from the `return`. Not sure I'm right; could just be familiarity with `str()`.

Have you considered other options? Two below.

One option:

  std::string Result;
  {
llvm::raw_string_ostream OS(Str);
OS << ...;
  }
  return Result;

which in branch-free cases could shorten to:

  std::string Str;
  llvm::raw_string_ostream(Str) << ...;
  return Str;

I personally find the lifetime a bit more obvious than the explicit flush call.

Another option:

  std::string Result;
  llvm::raw_string_ostream OS(Str);
  OS << ...;
  return OS.take();

Where `raw_string_ostream::take()` is just `return std::move(str())`. It 
doesn't get NRVO, but I'm not sure that really matters in most of these places. 
Benefit is that it's a minimal change and the name is clear / matches other 
LLVM things.




Comment at: clang/lib/AST/DeclarationName.cpp:236-240
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << *this;
-  return OS.str();
+  OS.flush();
+  return Result;

For trivial cases like this, maybe it'd be worth creating a helper function 
that gets reused. Maybe called `llvm::raw_string_ostream::toString()`:
```
lang=c++
template 
std::string raw_string_ostream::toString(const T ) {
  std::string Result;
  raw_string_ostream(Result) << V;
  return Result;
}
```
and the code here would be:
```
lang=c++
  return llvm::raw_string_ostream::toString(*this);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

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

[PATCH] D115332: [clang][deps] Use lock_guard instead of unique_lock

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith 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/D115332/new/

https://reviews.llvm.org/D115332

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

[PATCH] D115403: [clang][driver] update the darwin driver to point to correct macho_embedded path

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: bro4all, Ericson2314, ravikandhadai.
Herald added a subscriber: ributzka.
arphaman requested review of this revision.
Herald added a project: clang.

Compiler-rt started emitting the macho_embedded libraries in 
/lib/darwin/macho_embedded in https://reviews.llvm.org/D105765 / 
1e03c37b97b6176a60404d84665c40321f4e33a4 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115403

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/macho_embedded/libclang_rt.hard_pic.a
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/macho_embedded/libclang_rt.hard_static.a
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/macho_embedded/libclang_rt.soft_pic.a
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/macho_embedded/libclang_rt.soft_static.a
  
clang/test/Driver/Inputs/resource_dir/lib/macho_embedded/libclang_rt.hard_pic.a
  
clang/test/Driver/Inputs/resource_dir/lib/macho_embedded/libclang_rt.hard_static.a
  
clang/test/Driver/Inputs/resource_dir/lib/macho_embedded/libclang_rt.soft_pic.a
  
clang/test/Driver/Inputs/resource_dir/lib/macho_embedded/libclang_rt.soft_static.a


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1152,8 +1152,9 @@
   DarwinLibName += getOSLibraryNameSuffix();
   DarwinLibName += IsShared ? "_dynamic.dylib" : ".a";
   SmallString<128> Dir(getDriver().ResourceDir);
-  llvm::sys::path::append(
-  Dir, "lib", (Opts & RLO_IsEmbedded) ? "macho_embedded" : "darwin");
+  llvm::sys::path::append(Dir, "lib", "darwin");
+  if (Opts & RLO_IsEmbedded)
+llvm::sys::path::append(Dir, "macho_embedded");
 
   SmallString<128> P(Dir);
   llvm::sys::path::append(P, DarwinLibName);


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1152,8 +1152,9 @@
   DarwinLibName += getOSLibraryNameSuffix();
   DarwinLibName += IsShared ? "_dynamic.dylib" : ".a";
   SmallString<128> Dir(getDriver().ResourceDir);
-  llvm::sys::path::append(
-  Dir, "lib", (Opts & RLO_IsEmbedded) ? "macho_embedded" : "darwin");
+  llvm::sys::path::append(Dir, "lib", "darwin");
+  if (Opts & RLO_IsEmbedded)
+llvm::sys::path::append(Dir, "macho_embedded");
 
   SmallString<128> P(Dir);
   llvm::sys::path::append(P, DarwinLibName);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115378: OpenMP: Avoid using SmallVector::set_size()

[Duncan P. N. Exon Smith 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 rGcfd1d49dc0cc: OpenMP: Avoid using SmallVector::set_size() 
(authored by dexonsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115378

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1831,7 +1831,7 @@
 
   Value *Leftover = Result->getIndVar();
   SmallVector NewIndVars;
-  NewIndVars.set_size(NumLoops);
+  NewIndVars.resize(NumLoops);
   for (int i = NumLoops - 1; i >= 1; --i) {
 Value *OrigTripCount = Loops[i]->getTripCount();
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1972,7 +1972,7 @@
 
   // Pop the \p Depth loops requested by the call from that stack and restore
   // the previous context.
-  OMPLoopNestStack.set_size(OMPLoopNestStack.size() - Depth);
+  OMPLoopNestStack.pop_back_n(Depth);
   ExpectedOMPLoopDepth = ParentExpectedOMPLoopDepth;
 
   return Result;


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1831,7 +1831,7 @@
 
   Value *Leftover = Result->getIndVar();
   SmallVector NewIndVars;
-  NewIndVars.set_size(NumLoops);
+  NewIndVars.resize(NumLoops);
   for (int i = NumLoops - 1; i >= 1; --i) {
 Value *OrigTripCount = Loops[i]->getTripCount();
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1972,7 +1972,7 @@
 
   // Pop the \p Depth loops requested by the call from that stack and restore
   // the previous context.
-  OMPLoopNestStack.set_size(OMPLoopNestStack.size() - Depth);
+  OMPLoopNestStack.pop_back_n(Depth);
   ExpectedOMPLoopDepth = ParentExpectedOMPLoopDepth;
 
   return Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cfd1d49 - OpenMP: Avoid using SmallVector::set_size()

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2021-12-08T15:22:50-08:00
New Revision: cfd1d49dc0cc4369ace2e9485bdba04b27f158b5

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

LOG: OpenMP: Avoid using SmallVector::set_size()

Update `OpenMPIRBuilder::collapseLoops()` to call `resize()` instead of
`set_size()`. The latter asserts on capacity limits and cannot grow,
which seems likely to be unintentional here (if it is, I think a local
assertion would be good for clarity).

Also update `CodeGenFunction::EmitOMPCollapsedCanonicalLoopNest()` to
use `pop_back_n()` instead of `set_size()`.

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index f6853a22cd361..6f0ef7bb7fe52 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1972,7 +1972,7 @@ CodeGenFunction::EmitOMPCollapsedCanonicalLoopNest(const 
Stmt *S, int Depth) {
 
   // Pop the \p Depth loops requested by the call from that stack and restore
   // the previous context.
-  OMPLoopNestStack.set_size(OMPLoopNestStack.size() - Depth);
+  OMPLoopNestStack.pop_back_n(Depth);
   ExpectedOMPLoopDepth = ParentExpectedOMPLoopDepth;
 
   return Result;

diff  --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp 
b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index d5776964889b4..81f5ec54d1ce3 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1831,7 +1831,7 @@ OpenMPIRBuilder::collapseLoops(DebugLoc DL, 
ArrayRef Loops,
 
   Value *Leftover = Result->getIndVar();
   SmallVector NewIndVars;
-  NewIndVars.set_size(NumLoops);
+  NewIndVars.resize(NumLoops);
   for (int i = NumLoops - 1; i >= 1; --i) {
 Value *OrigTripCount = Loops[i]->getTripCount();
 



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

[PATCH] D115378: OpenMP: Avoid using SmallVector::set_size()

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur accepted this revision.
Meinersbur added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115378

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

[PATCH] D115346: [clang][deps] Squash caches for original and minimized files

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

This looks really nice.

One problem is that it's subtle to confirm whether it's thread-safe. The local 
cache access parts of CachedFileSystemEntry lock-free, but the 
CachedFileSystemEntry might get changed later (i.e., filled in with minimized 
content). Are accessed fields guaranteed by context not to be the ones that 
mutate later?




Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:50-51
+
+  /// Minimize contents of the file.
+  static void minimize(CachedFileSystemEntry );
 

Might be more natural as a non-static:
```
lang=c++
void minimize();
```




Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:69-75
+  llvm::ErrorOr getContents(bool Minimized) const {
+assert(isValid() && "not initialized");
 if (!MaybeStat)
   return MaybeStat.getError();
 assert(!MaybeStat->isDirectory() && "not a file");
-assert(isValid() && "not initialized");
-return Contents->getBuffer();
+return (Minimized ? MinimizedContents : OriginalContents)->getBuffer();
   }

I *think* this is thread-safe -- since `Minimized` should be the same as when 
the local cache entry was created -- but it's a bit subtle.

The problematic case I am worried about:
- First use in local cache is non-minimized.
- Creates shared cache entry that's not minimized.
- Some other local cache wants it to be minimized.
- Later use in local cache is minimized.
- Accessing `Minimized` pointer races with the other thread setting it.

If the local cache is guaranteed to always pass the same value for `Minimized` 
as when it fist accessed it, then there shouldn't be a problem...

I wonder if there's a way to make it less subtle?



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:106-108
+  std::unique_ptr OriginalContents;
+  std::unique_ptr MinimizedContents;
   PreprocessorSkippedRangeMapping PPSkippedRangeMapping;

I'm finding it a bit subtle detecting if there are races on access / setting of 
these, but I think it's correct.
- I think I verified that they are "set once".
- All the setters are guarded by locks.
- The first getter per "local" cache is guarded by a lock.
- Subsequent getters are not.

The key question: are the subsequent getters ONLY used when the first getter 
was successful?

One way to make it more obvious:
```
lang=c++
  struct ContentWithPPRanges {
std::unique_ptr Content;
PreprocessorSkippedRangeMapping PPSkippedRangeMapping;
  };

private:
  // Always accessed,mutated under a lock. Not mutated after they escape.
  std::unique_ptr Original;
  std::unique_ptr Minimized;
  PreprocessorSkippedRangeMapping PPSkippedRangeMapping;

  // Used in getters. Pointed-at memory immutable after these are set.
  std::atomic OriginalAccess;
  std::atomic MinimizedAccess;
  std::atomic PPRangesAccess;
```
I don't think this is the only approach though.




Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:189-191
+if (ShouldBeMinimized && CacheEntry.hasOriginalContents() &&
+!CacheEntry.hasMinimizedContents())
+  CachedFileSystemEntry::minimize(CacheEntry);

Is the DependencyScanningWorkerFilesystem guaranteed to always want either 
minimized or not minimized?

IOW, if the same filesystem is reused, and the first time only the original 
file was needed and later the minimized was needed, I don't see the code path 
for minimizing the file later. But maybe reusing one of these FSs is not 
supported?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115346

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

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:31
+/// states of its predecessor basic blocks.
+TypeErasedDataflowAnalysisState computeBlockInputState(
+std::vector> ,

should this be declared in the header? If not, please mark static.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:62
+/// `Block`.
+TypeErasedDataflowAnalysisState transferBlock(
+std::vector> ,

Please declare this function in the header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115235

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

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

[Snehasish Kumar via Phabricator via cfe-commits]

snehasish added inline comments.



Comment at: llvm/include/llvm/ProfileData/InstrProfReader.h:93
 
-  virtual bool isIRLevelProfile() const = 0;
-

tejohnson wrote:
> It seems like these helper methods could still be defined using the new 
> bitset, which would reduce some of the code churn?
My rationale for removing the helpers was 
- to have only one way of checking the bits
- the checks are clustered together so users would get the bitset once rather 
than invoking multiple methods for each check
- extending the bitset would add now mean that adding more helpers?
- retaining the helpers doesn't obviate the need for getProfileKind since the 
simplified code in mergeProfileKind directly operates on the enum value

I agree retaining the helpers reduces code churn and I've made the changes to 
keep them. PTAL, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115393

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

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

[Snehasish Kumar via Phabricator via cfe-commits]

snehasish updated this revision to Diff 392938.
snehasish added a comment.

Remove extra line in PGOInstrumentation.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115393

Files:
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/ProfileData/InstrProfWriter.h
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -251,9 +251,7 @@
   }
 
   auto Reader = std::move(ReaderOrErr.get());
-  bool IsIRProfile = Reader->isIRLevelProfile();
-  bool HasCSIRProfile = Reader->hasCSIRLevelProfile();
-  if (Error E = WC->Writer.setIsIRLevelProfile(IsIRProfile, HasCSIRProfile)) {
+  if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
 consumeError(std::move(E));
 WC->Errors.emplace_back(
 make_error(
@@ -262,7 +260,6 @@
 Filename);
 return;
   }
-  WC->Writer.setInstrEntryBBEnabled(Reader->instrEntryBBEnabled());
 
   for (auto  : *Reader) {
 if (Remapper)
Index: llvm/lib/ProfileData/InstrProfWriter.cpp
===
--- llvm/lib/ProfileData/InstrProfWriter.cpp
+++ llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -165,9 +165,8 @@
 
 } // end namespace llvm
 
-InstrProfWriter::InstrProfWriter(bool Sparse, bool InstrEntryBBEnabled)
-: Sparse(Sparse), InstrEntryBBEnabled(InstrEntryBBEnabled),
-  InfoObj(new InstrProfRecordWriterTrait()) {}
+InstrProfWriter::InstrProfWriter(bool Sparse)
+: Sparse(Sparse), InfoObj(new InstrProfRecordWriterTrait()) {}
 
 InstrProfWriter::~InstrProfWriter() { delete InfoObj; }
 
@@ -302,13 +301,11 @@
   IndexedInstrProf::Header Header;
   Header.Magic = IndexedInstrProf::Magic;
   Header.Version = IndexedInstrProf::ProfVersion::CurrentVersion;
-  if (ProfileKind == PF_IRLevel)
-Header.Version |= VARIANT_MASK_IR_PROF;
-  if (ProfileKind == PF_IRLevelWithCS) {
+  if (static_cast(ProfileKind & InstrProfKind::IR))
 Header.Version |= VARIANT_MASK_IR_PROF;
+  if (static_cast(ProfileKind & InstrProfKind::CS))
 Header.Version |= VARIANT_MASK_CSIR_PROF;
-  }
-  if (InstrEntryBBEnabled)
+  if (static_cast(ProfileKind & InstrProfKind::BB))
 Header.Version |= VARIANT_MASK_INSTR_ENTRY;
 
   Header.Unused = 0;
@@ -336,7 +333,7 @@
 OS.write(0);
   uint64_t CSSummaryOffset = 0;
   uint64_t CSSummarySize = 0;
-  if (ProfileKind == PF_IRLevelWithCS) {
+  if (static_cast(ProfileKind & InstrProfKind::CS)) {
 CSSummaryOffset = OS.tell();
 CSSummarySize = SummarySize / sizeof(uint64_t);
 for (unsigned I = 0; I < CSSummarySize; I++)
@@ -357,7 +354,7 @@
 
   // For Context Sensitive summary.
   std::unique_ptr TheCSSummary = nullptr;
-  if (ProfileKind == PF_IRLevelWithCS) {
+  if (static_cast(ProfileKind & InstrProfKind::CS)) {
 TheCSSummary = IndexedInstrProf::allocSummary(SummarySize);
 std::unique_ptr CSPS = CSISB.getSummary();
 setSummary(TheCSSummary.get(), *CSPS);
@@ -469,11 +466,13 @@
 }
 
 Error InstrProfWriter::writeText(raw_fd_ostream ) {
-  if (ProfileKind == PF_IRLevel)
-OS << "# IR level Instrumentation Flag\n:ir\n";
-  else if (ProfileKind == PF_IRLevelWithCS)
+  // Check CS first since it implies an IR level profile.
+  if (static_cast(ProfileKind & InstrProfKind::CS))
 OS << "# CSIR level Instrumentation Flag\n:csir\n";
-  if (InstrEntryBBEnabled)
+  else if (static_cast(ProfileKind & InstrProfKind::IR))
+OS << "# IR level Instrumentation Flag\n:ir\n";
+
+  if (static_cast(ProfileKind & InstrProfKind::BB))
 OS << "# Always instrument the function entry block\n:entry_first\n";
   InstrProfSymtab Symtab;
 
Index: llvm/lib/ProfileData/InstrProfReader.cpp
===
--- llvm/lib/ProfileData/InstrProfReader.cpp
+++ llvm/lib/ProfileData/InstrProfReader.cpp
@@ -151,30 +151,24 @@
 // with a leading ':' will be reported an error format.
 Error TextInstrProfReader::readHeader() {
   Symtab.reset(new InstrProfSymtab());
-  bool IsIRInstr = false;
-  bool IsEntryFirst = false;
-  bool IsCS = false;
 
   while (Line->startswith(":")) {
 StringRef Str = Line->substr(1);
 if (Str.equals_insensitive("ir"))
-  IsIRInstr = true;
+  ProfileKind |= InstrProfKind::IR;
 else if (Str.equals_insensitive("fe"))
-  IsIRInstr = false;
+  ProfileKind |= InstrProfKind::FE;
 else if (Str.equals_insensitive("csir")) {
-  IsIRInstr = true;
-  IsCS = true;
+  ProfileKind |= InstrProfKind::IR;
+  ProfileKind |= InstrProfKind::CS;
 } else if (Str.equals_insensitive("entry_first"))
-  IsEntryFirst = true;
+  

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

[Snehasish Kumar via Phabricator via cfe-commits]

snehasish updated this revision to Diff 392937.
snehasish added a comment.

Reintroduce the helpers while keeping the bitset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115393

Files:
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/ProfileData/InstrProfWriter.h
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -251,9 +251,7 @@
   }
 
   auto Reader = std::move(ReaderOrErr.get());
-  bool IsIRProfile = Reader->isIRLevelProfile();
-  bool HasCSIRProfile = Reader->hasCSIRLevelProfile();
-  if (Error E = WC->Writer.setIsIRLevelProfile(IsIRProfile, HasCSIRProfile)) {
+  if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
 consumeError(std::move(E));
 WC->Errors.emplace_back(
 make_error(
@@ -262,7 +260,6 @@
 Filename);
 return;
   }
-  WC->Writer.setInstrEntryBBEnabled(Reader->instrEntryBBEnabled());
 
   for (auto  : *Reader) {
 if (Remapper)
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1827,6 +1827,7 @@
   StringRef("Cannot get PGOReader")));
 return false;
   }
+
   if (!PGOReader->hasCSIRLevelProfile() && IsCS)
 return false;
 
Index: llvm/lib/ProfileData/InstrProfWriter.cpp
===
--- llvm/lib/ProfileData/InstrProfWriter.cpp
+++ llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -165,9 +165,8 @@
 
 } // end namespace llvm
 
-InstrProfWriter::InstrProfWriter(bool Sparse, bool InstrEntryBBEnabled)
-: Sparse(Sparse), InstrEntryBBEnabled(InstrEntryBBEnabled),
-  InfoObj(new InstrProfRecordWriterTrait()) {}
+InstrProfWriter::InstrProfWriter(bool Sparse)
+: Sparse(Sparse), InfoObj(new InstrProfRecordWriterTrait()) {}
 
 InstrProfWriter::~InstrProfWriter() { delete InfoObj; }
 
@@ -302,13 +301,11 @@
   IndexedInstrProf::Header Header;
   Header.Magic = IndexedInstrProf::Magic;
   Header.Version = IndexedInstrProf::ProfVersion::CurrentVersion;
-  if (ProfileKind == PF_IRLevel)
-Header.Version |= VARIANT_MASK_IR_PROF;
-  if (ProfileKind == PF_IRLevelWithCS) {
+  if (static_cast(ProfileKind & InstrProfKind::IR))
 Header.Version |= VARIANT_MASK_IR_PROF;
+  if (static_cast(ProfileKind & InstrProfKind::CS))
 Header.Version |= VARIANT_MASK_CSIR_PROF;
-  }
-  if (InstrEntryBBEnabled)
+  if (static_cast(ProfileKind & InstrProfKind::BB))
 Header.Version |= VARIANT_MASK_INSTR_ENTRY;
 
   Header.Unused = 0;
@@ -336,7 +333,7 @@
 OS.write(0);
   uint64_t CSSummaryOffset = 0;
   uint64_t CSSummarySize = 0;
-  if (ProfileKind == PF_IRLevelWithCS) {
+  if (static_cast(ProfileKind & InstrProfKind::CS)) {
 CSSummaryOffset = OS.tell();
 CSSummarySize = SummarySize / sizeof(uint64_t);
 for (unsigned I = 0; I < CSSummarySize; I++)
@@ -357,7 +354,7 @@
 
   // For Context Sensitive summary.
   std::unique_ptr TheCSSummary = nullptr;
-  if (ProfileKind == PF_IRLevelWithCS) {
+  if (static_cast(ProfileKind & InstrProfKind::CS)) {
 TheCSSummary = IndexedInstrProf::allocSummary(SummarySize);
 std::unique_ptr CSPS = CSISB.getSummary();
 setSummary(TheCSSummary.get(), *CSPS);
@@ -469,11 +466,13 @@
 }
 
 Error InstrProfWriter::writeText(raw_fd_ostream ) {
-  if (ProfileKind == PF_IRLevel)
-OS << "# IR level Instrumentation Flag\n:ir\n";
-  else if (ProfileKind == PF_IRLevelWithCS)
+  // Check CS first since it implies an IR level profile.
+  if (static_cast(ProfileKind & InstrProfKind::CS))
 OS << "# CSIR level Instrumentation Flag\n:csir\n";
-  if (InstrEntryBBEnabled)
+  else if (static_cast(ProfileKind & InstrProfKind::IR))
+OS << "# IR level Instrumentation Flag\n:ir\n";
+
+  if (static_cast(ProfileKind & InstrProfKind::BB))
 OS << "# Always instrument the function entry block\n:entry_first\n";
   InstrProfSymtab Symtab;
 
Index: llvm/lib/ProfileData/InstrProfReader.cpp
===
--- llvm/lib/ProfileData/InstrProfReader.cpp
+++ llvm/lib/ProfileData/InstrProfReader.cpp
@@ -151,30 +151,24 @@
 // with a leading ':' will be reported an error format.
 Error TextInstrProfReader::readHeader() {
   Symtab.reset(new InstrProfSymtab());
-  bool IsIRInstr = false;
-  bool IsEntryFirst = false;
-  bool IsCS = false;
 
   while (Line->startswith(":")) {
 

[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Not exactly that. The weak symbol isn't the function name, as that gets renamed 
or inlined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283

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

[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

[Konstantin Pyzhov via Phabricator via cfe-commits]

kpyzhov added a comment.

In D115283#3180836 , @yaxunl wrote:

> If we only need to check whether `__ockl_hostcall_internal` exists in the 
> final module in LLVM codegen to determine whether we need the hostcall 
> metadata, probably we don't even need a function attribute or even module 
> flag.

Right, we used to do exactly that, but then it turned out that it does not work 
with -fgpu-rdc since IPO may rename the __ockl_hostcall_internal().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283

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

[PATCH] D112881: [clang-tidy] Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions

[Paul Altin via Phabricator via cfe-commits]

paulaltin added a comment.

> Thanks! Do you need someone to commit on your behalf? If so, what name and 
> email address would you like used for patch attribution?

That would be great, thanks! You can use "Paul Altin 
" for the attribution.

> We're definitely happy to consider these kinds of changes, thank you! We 
> usually prefer finding ways to silence the diagnostic in code (like casting 
> to void to silence an unused variable warning, etc) over configuration 
> options, but that's not always plausible to do and so extra configuration 
> options are a good fallback solution.

Ok, fantastic. I'll go back through the checks that we've disabled and see what 
I can suggest.

Thanks @aaron.ballman!


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

https://reviews.llvm.org/D112881

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

[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Openmp defines a weak symbol in the hostcall_invoke function. Optimisation and 
deadstripping friendly, no compiler support necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283

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

[PATCH] D115378: OpenMP: Avoid using SmallVector::set_size()

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115378

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

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/include/llvm/ProfileData/InstrProfReader.h:93
 
-  virtual bool isIRLevelProfile() const = 0;
-

It seems like these helper methods could still be defined using the new bitset, 
which would reduce some of the code churn?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115393

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

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

[Snehasish Kumar via Phabricator via cfe-commits]

snehasish updated this revision to Diff 392927.
snehasish added a comment.

Cleanup for review -

- Added some more descriptive comments.
- Removed a couple of unintentional blank lines.
- Removed a couple of commented lines of code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115393

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/ProfileData/InstrProfWriter.h
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -251,9 +251,7 @@
   }
 
   auto Reader = std::move(ReaderOrErr.get());
-  bool IsIRProfile = Reader->isIRLevelProfile();
-  bool HasCSIRProfile = Reader->hasCSIRLevelProfile();
-  if (Error E = WC->Writer.setIsIRLevelProfile(IsIRProfile, HasCSIRProfile)) {
+  if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
 consumeError(std::move(E));
 WC->Errors.emplace_back(
 make_error(
@@ -262,7 +260,6 @@
 Filename);
 return;
   }
-  WC->Writer.setInstrEntryBBEnabled(Reader->instrEntryBBEnabled());
 
   for (auto  : *Reader) {
 if (Remapper)
@@ -2078,7 +2075,8 @@
 exitWithError(std::move(E), Filename);
 
   auto Reader = std::move(ReaderOrErr.get());
-  bool IsIRInstr = Reader->isIRLevelProfile();
+  bool IsIRInstr =
+  static_cast(Reader->getProfileKind() & InstrProfKind::IR);
   size_t ShownFunctions = 0;
   size_t BelowCutoffFunctions = 0;
   int NumVPKind = IPVK_Last - IPVK_First + 1;
@@ -2104,7 +2102,7 @@
 OS << ":ir\n";
 
   for (const auto  : *Reader) {
-if (Reader->isIRLevelProfile()) {
+if (static_cast(Reader->getProfileKind() & InstrProfKind::IR)) {
   bool FuncIsCS = NamedInstrProfRecord::hasCSFlagInHash(Func.Hash);
   if (FuncIsCS != ShowCS)
 continue;
@@ -2203,10 +2201,11 @@
   if (TextFormat)
 return 0;
   std::unique_ptr PS(Builder.getSummary());
-  bool IsIR = Reader->isIRLevelProfile();
+  bool IsIR = static_cast(Reader->getProfileKind() & InstrProfKind::IR);
   OS << "Instrumentation level: " << (IsIR ? "IR" : "Front-end");
   if (IsIR)
-OS << "  entry_first = " << Reader->instrEntryBBEnabled();
+OS << "  entry_first = "
+   << static_cast(Reader->getProfileKind() & InstrProfKind::BB);
   OS << "\n";
   if (ShowAllFunctions || !ShowFunction.empty())
 OS << "Functions shown: " << ShownFunctions << "\n";
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1827,11 +1827,13 @@
   StringRef("Cannot get PGOReader")));
 return false;
   }
-  if (!PGOReader->hasCSIRLevelProfile() && IsCS)
+
+  const InstrProfKind ProfileKind = PGOReader->getProfileKind();
+  if (!static_cast(ProfileKind & InstrProfKind::CS) && IsCS)
 return false;
 
   // TODO: might need to change the warning once the clang option is finalized.
-  if (!PGOReader->isIRLevelProfile()) {
+  if (!static_cast(ProfileKind & InstrProfKind::IR)) {
 Ctx.diagnose(DiagnosticInfoPGOProfile(
 ProfileFileName.data(), "Not an IR level instrumentation profile"));
 return false;
@@ -1852,7 +1854,7 @@
 
   // If the profile marked as always instrument the entry BB, do the
   // same. Note this can be overwritten by the internal option in CFGMST.h
-  bool InstrumentFuncEntry = PGOReader->instrEntryBBEnabled();
+  bool InstrumentFuncEntry = static_cast(ProfileKind & InstrProfKind::BB);
   if (PGOInstrumentEntry.getNumOccurrences() > 0)
 InstrumentFuncEntry = PGOInstrumentEntry;
   for (auto  : M) {
Index: llvm/lib/ProfileData/InstrProfWriter.cpp
===
--- llvm/lib/ProfileData/InstrProfWriter.cpp
+++ llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -165,9 +165,8 @@
 
 } // end namespace llvm
 
-InstrProfWriter::InstrProfWriter(bool Sparse, bool InstrEntryBBEnabled)
-: Sparse(Sparse), InstrEntryBBEnabled(InstrEntryBBEnabled),
-  InfoObj(new InstrProfRecordWriterTrait()) {}
+InstrProfWriter::InstrProfWriter(bool Sparse)
+: Sparse(Sparse), InfoObj(new InstrProfRecordWriterTrait()) {}
 
 InstrProfWriter::~InstrProfWriter() { delete InfoObj; }
 
@@ -302,13 +301,11 @@
   IndexedInstrProf::Header Header;
   Header.Magic = IndexedInstrProf::Magic;
   Header.Version = IndexedInstrProf::ProfVersion::CurrentVersion;
-  if (ProfileKind == PF_IRLevel)
-

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Logan Smith via Phabricator via cfe-commits]

logan-5 added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36
+OS.flush();
+return Str;
   }

Quuxplusone wrote:
> FWIW, it appears to me that in most (all?) of these cases, what's really 
> wanted is not "a string //and// a stream" but rather "a stream that owns a 
> string" (`std::ostringstream` or the LLVM-codebase equivalent thereof). Then 
> the return can be `return std::move(OS).str();` — for `std::ostringstream`, 
> this Does The Right Thing since C++20, and if LLVM had its own stringstream 
> it could make it Do The Right Thing today.
> https://en.cppreference.com/w/cpp/io/basic_ostringstream/str
> 
True enough. Although `return std::move(OS).str();` is still much harder to 
type than the less efficient `return OS.str();`, and it requires at minimum a 
move of the underlying string--whereas `return Str;` is the easiest of all to 
type, and it opens things up for NRVO. If (as I said in the patch summary) 
`raw_string_ostream` were changed to be guaranteed to not need flushing, 
`return Str;` would IMHO be cemented as the clear winner.

That said, you're clearly right that all these cases are semantically trying to 
do "a stream that owns a string", and it's clunky to execute with the existing 
APIs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

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

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

[Snehasish Kumar via Phabricator via cfe-commits]

snehasish created this revision.
snehasish added reviewers: xur, davidxl.
Herald added subscribers: wenlei, hiraditya.
snehasish requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This change refactors the ProfileKind enum into a bitset enum to
represent the different attributes a profile can have. This change
simplifies the logic in the instrprof writer when multiple profiles are
merged together. In the future we plan on introducing a new memory
profile section which will extend the enum by one additional entry.
Without this change when accounting for memory profiles will have to be
maintained separately and will make the logic more complex.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115393

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/include/llvm/ProfileData/InstrProfWriter.h
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -251,9 +251,7 @@
   }
 
   auto Reader = std::move(ReaderOrErr.get());
-  bool IsIRProfile = Reader->isIRLevelProfile();
-  bool HasCSIRProfile = Reader->hasCSIRLevelProfile();
-  if (Error E = WC->Writer.setIsIRLevelProfile(IsIRProfile, HasCSIRProfile)) {
+  if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
 consumeError(std::move(E));
 WC->Errors.emplace_back(
 make_error(
@@ -262,13 +260,13 @@
 Filename);
 return;
   }
-  WC->Writer.setInstrEntryBBEnabled(Reader->instrEntryBBEnabled());
 
   for (auto  : *Reader) {
 if (Remapper)
   I.Name = (*Remapper)(I.Name);
 const StringRef FuncName = I.Name;
 bool Reported = false;
+
 WC->Writer.addRecord(std::move(I), Input.Weight, [&](Error E) {
   if (Reported) {
 consumeError(std::move(E));
@@ -2078,7 +2076,8 @@
 exitWithError(std::move(E), Filename);
 
   auto Reader = std::move(ReaderOrErr.get());
-  bool IsIRInstr = Reader->isIRLevelProfile();
+  bool IsIRInstr =
+  static_cast(Reader->getProfileKind() & InstrProfKind::IR);
   size_t ShownFunctions = 0;
   size_t BelowCutoffFunctions = 0;
   int NumVPKind = IPVK_Last - IPVK_First + 1;
@@ -2104,7 +2103,7 @@
 OS << ":ir\n";
 
   for (const auto  : *Reader) {
-if (Reader->isIRLevelProfile()) {
+if (static_cast(Reader->getProfileKind() & InstrProfKind::IR)) {
   bool FuncIsCS = NamedInstrProfRecord::hasCSFlagInHash(Func.Hash);
   if (FuncIsCS != ShowCS)
 continue;
@@ -2203,10 +2202,11 @@
   if (TextFormat)
 return 0;
   std::unique_ptr PS(Builder.getSummary());
-  bool IsIR = Reader->isIRLevelProfile();
+  bool IsIR = static_cast(Reader->getProfileKind() & InstrProfKind::IR);
   OS << "Instrumentation level: " << (IsIR ? "IR" : "Front-end");
   if (IsIR)
-OS << "  entry_first = " << Reader->instrEntryBBEnabled();
+OS << "  entry_first = "
+   << static_cast(Reader->getProfileKind() & InstrProfKind::BB);
   OS << "\n";
   if (ShowAllFunctions || !ShowFunction.empty())
 OS << "Functions shown: " << ShownFunctions << "\n";
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1827,11 +1827,13 @@
   StringRef("Cannot get PGOReader")));
 return false;
   }
-  if (!PGOReader->hasCSIRLevelProfile() && IsCS)
+
+  const InstrProfKind ProfileKind = PGOReader->getProfileKind();
+  if (!static_cast(ProfileKind & InstrProfKind::CS) && IsCS)
 return false;
 
   // TODO: might need to change the warning once the clang option is finalized.
-  if (!PGOReader->isIRLevelProfile()) {
+  if (!static_cast(ProfileKind & InstrProfKind::IR)) {
 Ctx.diagnose(DiagnosticInfoPGOProfile(
 ProfileFileName.data(), "Not an IR level instrumentation profile"));
 return false;
@@ -1852,7 +1854,7 @@
 
   // If the profile marked as always instrument the entry BB, do the
   // same. Note this can be overwritten by the internal option in CFGMST.h
-  bool InstrumentFuncEntry = PGOReader->instrEntryBBEnabled();
+  bool InstrumentFuncEntry = static_cast(ProfileKind & InstrProfKind::BB);
   if (PGOInstrumentEntry.getNumOccurrences() > 0)
 InstrumentFuncEntry = PGOInstrumentEntry;
   for (auto  : M) {
Index: llvm/lib/ProfileData/InstrProfWriter.cpp
===
--- llvm/lib/ProfileData/InstrProfWriter.cpp
+++ 

[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

If we only need to check whether `__ockl_hostcall_internal` exists in the final 
module in LLVM codegen to determine whether we need the hostcall metadata, 
probably we don't even need a function attribute or even module flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283

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

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36
+OS.flush();
+return Str;
   }

FWIW, it appears to me that in most (all?) of these cases, what's really wanted 
is not "a string //and// a stream" but rather "a stream that owns a string" 
(`std::ostringstream` or the LLVM-codebase equivalent thereof). Then the return 
can be `return std::move(OS).str();` — for `std::ostringstream`, this Does The 
Right Thing since C++20, and if LLVM had its own stringstream it could make it 
Do The Right Thing today.
https://en.cppreference.com/w/cpp/io/basic_ostringstream/str



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115374

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

[PATCH] D115386: AST: Avoid using SmallVector::set_size() in UnresolvedSet

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added a reviewer: dblaikie.
dexonsmith requested review of this revision.
Herald added a project: clang.

Update UnresolvedSet to use (and expose) `SmallVector::truncate()` instead
of `SmallVector::set_size()`. The latter is going to made private in a
future commit to avoid misuse.

Currently depends on https://reviews.llvm.org/D115383. It seems to improve 
readability to use `truncate()` instead of `resize()` (known to shrink the 
size), and it keeps an assertion in place (not sure of value?) that this 
doesn't grow. I can't imagine the former's performance characteristics are 
important here, so I could be convinced to just use `resize()`...

Blocker for https://reviews.llvm.org/D115380.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115386

Files:
  clang/include/clang/AST/UnresolvedSet.h
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -620,7 +620,7 @@
 getSema().diagnoseEquivalentInternalLinkageDeclarations(
 getNameLoc(), HasNonFunction, EquivalentNonFunctions);
 
-  Decls.set_size(N);
+  Decls.truncate(N);
 
   if (HasNonFunction && (HasFunction || HasUnresolved))
 Ambiguous = true;
Index: clang/include/clang/AST/UnresolvedSet.h
===
--- clang/include/clang/AST/UnresolvedSet.h
+++ clang/include/clang/AST/UnresolvedSet.h
@@ -121,7 +121,7 @@
   void setAccess(iterator I, AccessSpecifier AS) { I.I->setAccess(AS); }
 
   void clear() { decls().clear(); }
-  void set_size(unsigned N) { decls().set_size(N); }
+  void truncate(unsigned N) { decls().truncate(N); }
 
   bool empty() const { return decls().empty(); }
   unsigned size() const { return decls().size(); }


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -620,7 +620,7 @@
 getSema().diagnoseEquivalentInternalLinkageDeclarations(
 getNameLoc(), HasNonFunction, EquivalentNonFunctions);
 
-  Decls.set_size(N);
+  Decls.truncate(N);
 
   if (HasNonFunction && (HasFunction || HasUnresolved))
 Ambiguous = true;
Index: clang/include/clang/AST/UnresolvedSet.h
===
--- clang/include/clang/AST/UnresolvedSet.h
+++ clang/include/clang/AST/UnresolvedSet.h
@@ -121,7 +121,7 @@
   void setAccess(iterator I, AccessSpecifier AS) { I.I->setAccess(AS); }
 
   void clear() { decls().clear(); }
-  void set_size(unsigned N) { decls().set_size(N); }
+  void truncate(unsigned N) { decls().truncate(N); }
 
   bool empty() const { return decls().empty(); }
   unsigned size() const { return decls().size(); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: RFC: proposing to relax standardization requirements for Clang extensions

[Richard Smith via cfe-commits]

On Tue, 7 Dec 2021 at 13:22, Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> tl;dr: our Clang "get involved" page implies that proposed extensions
> to Clang must also be proposed to a standards committee
> (https://clang.llvm.org/get_involved.html#criteria). This is a good
> goal, but is not inclusive or something we enforce with any
> consistency. I'm proposing to clarify our goals in this space and to
> better reflect existing practice.
>
> Inclusivity Issues:
> Participating in a standards body is not always free. In fact, it can
> be quite expensive depending on where you live and which committee you
> want to join. People who wish to join the C or C++ standards committee
> as a US national body member have yearly dues in excess of $2000/yr.
> Each national body sets their membership fees individually and the
> prices range anywhere from free to $7500+/yr. Also, a standards
> proposal is not a "one and done" activity and often takes multiple
> meetings to see even a trivial proposal adopted, but may require
> multiple years for larger proposals. For example, the [[]] attributes
> proposal took three years, and _BitInt took over two years with
> multiple senior-level engineers working on it. Further, there are
> associated travel costs (in non-pandemic times, anyway) with attending
> meetings. This is a very significant financial and time burden that I
> don't think we should require community members to take on.
>
> Consistency Issues:
> We have never been consistent at enforcing this requirement. Here are
> some examples off the top of my head:
>
> * Nullability qualifiers -- never proposed to WG14
> * VLAs in C++ -- never proposed to WG21
> * [[]] attributes in C -- proposed to WG14, accepted
> * _ExtInt -- proposed to WG14, accepted as _BitInt
> * enums with a fixed underlying type in C -- proposed to WG14 (not by
> a Clang community member), still not accepted yet
> * (This list is not intended to be exhaustive.)
>

VLAs in C++, [[]] in C, and enums with a fixed underlying type are all
instances of accepting a feature from one language mode in another. For
those cases, we do have a relevant standard on which to base our behaviour,
so I think those meet the current policy.

Nullability qualifiers are part of the Mac OS language platform's SDK. In
this instance, Apple as a platform vendor decided to add an extension, and
we as vendors of a compiler that intends to support that platform,
implement that extension. I don't think this is all that different from our
supporting, say,  to support intel platforms. Or our
supporting MS extensions to support the MS platform.

For _ExtInt, I specifically asked that this be proposed to WG14, and it was
(https://reviews.llvm.org/D59105#1422089).

So, I think we may be missing some detail from the policy:
-- sometimes the relevant organization may be the owner of a platform or
architecture we care about supporting, rather than a language
-- features supported for one language mode or platform are allowed to be
supported in other configurations too; if we're paying the cost of
supporting a feature, we should get as much benefit from it as we can


> Coupled with the time and monetary costs associated with
> standardization, inconsistently applying something we say "must"
> happen in our documentation is ripe for potential abuse (both
> malicious and unintentional).
>
> To this end, I'm proposing a change along the lines of:
>
> -  Representation within the appropriate governing organization: For
> -  extensions to a language governed by a standards committee (C, C++,
> OpenCL),
> -  the extension itself must have an active proposal and proponent within
> that
> -  committee and have a reasonable chance of acceptance. Clang should
> drive the
> -  standard, not diverge from it. This criterion does not apply to all
> -  extensions, since some extensions fall outside of the realm of the
> standards
> -  bodies.
> +  Plausibility of standardization where applicable: Extensions with an
> +  active proposal within a standards committee (C, C++, OpenCL, etc.) are
> +  preferred when appropriate. Proposed Clang-specific extensions that are
> being
> +  considered by a standards committee must have a feedback loop between
> the
> +  community and the committee. Clang should drive the standard, not
> diverge
> +  from it, so proposals currently in a standardization pipeline should
> not be
> +  treated as finalized features in Clang until the standardization
> process has
> +  completed and review can be done against the standard defining the
> feature.
> +  Regardless of whether an extension is proposed for standardization, it
> must
> +  be a conforming extension and it should not knowingly impede future
> +  standardization efforts.
> +  
>
> (I'll post an actual review for the changes, but I wanted folks to see
> what I was going for as part of the RFC.)
>
> The goal of the change is to make the following clear:
>
> * We /prefer/ extensions to go 

[PATCH] D115341: [clang][dataflow] Add framework for testing analyses.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:48
+  << std::declval())>
+std::ostream <<(std::ostream ,
+ const DataflowAnalysisState ) {

This would also be useful for debugging. I wonder whether utilities not 
strictly related to testing should be closer to the actual code, so someone 
wanting to use this for debugging does not need to include the header that is 
dedicated to the tests.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:77
+// the files provided in `VirtualMappedFiles`.
+bool runOnCode(llvm::StringRef Code,
+   const std::function ,

I feel like some of our tests keep recreating lightweight versions of the 
LibTooling. Not really an action item for this PR, just a rant :) I hope we 
will have some more centralized stuff at some point. 



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:94
+template 
+void runDataflow(
+llvm::StringRef Code,

Since this function is actually matching the dataflow results against 
expectations, I wonder if something like `checkDataflow` would better describe 
its function. But feel free to keep the current name.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:137
+TypeErasedBlockStates =
+runTypeErasedDataflowAnalysis(*cfg, Analysis, Env);
+

Wouldn't users end up calling the template (not the type erased) version of 
this function? I wonder if we should mimic how users would interact with the 
framework in the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115341

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

[PATCH] D115379: ASTMatchers: Avoid using SmallVector::set_size()

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: njames93, aaron.ballman.
dexonsmith requested review of this revision.
Herald added a project: clang.

Update `variadicMatcherDescriptor` to assert on reserved capacity and
to call `emplace_back()` instead of calling `set_size()` and constructing
the element in-place.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115379

Files:
  clang/lib/ASTMatchers/Dynamic/Marshallers.h


Index: clang/lib/ASTMatchers/Dynamic/Marshallers.h
===
--- clang/lib/ASTMatchers/Dynamic/Marshallers.h
+++ clang/lib/ASTMatchers/Dynamic/Marshallers.h
@@ -524,8 +524,9 @@
   }
   return {};
 }
-InnerArgs.set_size(i + 1);
-InnerArgsPtr[i] = new ([i]) ArgT(ArgTraits::get(Value));
+assert(InnerArgs.size() < InnerArgs.capacity());
+InnerArgs.emplace_back(ArgTraits::get(Value));
+InnerArgsPtr[i] = [i];
   }
   return outvalueToVariantMatcher(Func(InnerArgsPtr));
 }


Index: clang/lib/ASTMatchers/Dynamic/Marshallers.h
===
--- clang/lib/ASTMatchers/Dynamic/Marshallers.h
+++ clang/lib/ASTMatchers/Dynamic/Marshallers.h
@@ -524,8 +524,9 @@
   }
   return {};
 }
-InnerArgs.set_size(i + 1);
-InnerArgsPtr[i] = new ([i]) ArgT(ArgTraits::get(Value));
+assert(InnerArgs.size() < InnerArgs.capacity());
+InnerArgs.emplace_back(ArgTraits::get(Value));
+InnerArgsPtr[i] = [i];
   }
   return outvalueToVariantMatcher(Func(InnerArgsPtr));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115378: OpenMP: Avoid using SmallVector::set_size()

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added a reviewer: Meinersbur.
Herald added subscribers: guansong, hiraditya, yaxunl.
dexonsmith requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
Herald added projects: clang, LLVM.

Update `OpenMPIRBuilder::collapseLoops()` to call `resize()` instead of
`set_size()`. The latter asserts on capacity limits and cannot grow,
which seems likely to be unintentional here (if it is, I think a local
assertion would be good for clarity).

Also update `CodeGenFunction::EmitOMPCollapsedCanonicalLoopNest()` to
use `pop_back_n()` instead of `set_size()`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115378

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1831,7 +1831,7 @@
 
   Value *Leftover = Result->getIndVar();
   SmallVector NewIndVars;
-  NewIndVars.set_size(NumLoops);
+  NewIndVars.resize(NumLoops);
   for (int i = NumLoops - 1; i >= 1; --i) {
 Value *OrigTripCount = Loops[i]->getTripCount();
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1972,7 +1972,7 @@
 
   // Pop the \p Depth loops requested by the call from that stack and restore
   // the previous context.
-  OMPLoopNestStack.set_size(OMPLoopNestStack.size() - Depth);
+  OMPLoopNestStack.pop_back_n(Depth);
   ExpectedOMPLoopDepth = ParentExpectedOMPLoopDepth;
 
   return Result;


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1831,7 +1831,7 @@
 
   Value *Leftover = Result->getIndVar();
   SmallVector NewIndVars;
-  NewIndVars.set_size(NumLoops);
+  NewIndVars.resize(NumLoops);
   for (int i = NumLoops - 1; i >= 1; --i) {
 Value *OrigTripCount = Loops[i]->getTripCount();
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1972,7 +1972,7 @@
 
   // Pop the \p Depth loops requested by the call from that stack and restore
   // the previous context.
-  OMPLoopNestStack.set_size(OMPLoopNestStack.size() - Depth);
+  OMPLoopNestStack.pop_back_n(Depth);
   ExpectedOMPLoopDepth = ParentExpectedOMPLoopDepth;
 
   return Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, whisperity, alexfh, 
malcolm.parsons.
aaron.ballman added a comment.
Herald added a subscriber: rnkovacs.

Adding some additional reviewers. At a high level, I think this is a reasonable 
direction to go, but I wonder if this should be hidden behind a configuration 
option or not (basically, was there a reason we didn't cover that case 
originally or was it an oversight/left for future work)?




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp:48-49
 
+struct PositiveNotDefaultInt {
+  PositiveNotDefaultInt(int) : i(7) {}
+  // CHECK-FIXES: PositiveNotDefaultInt(int)  {}

The "default" here is not about the constructor but about the argument to the 
constructor, I believe. So I think this should be renamed to 
`PositiveConstantMemInit` or something like that (similar for the names below).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114995

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

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[Logan Smith via Phabricator via cfe-commits]

logan-5 created this revision.
logan-5 added a reviewer: aaron.ballman.
logan-5 added a project: clang.
Herald added subscribers: abrachet, ctetreau, dexonsmith, martong.
logan-5 requested review of this revision.
Herald added a subscriber: cfe-commits.

Returning `OS.str()` is guaranteed to copy the underlying string, whereas 
`OS.flush(); return Underlying;` makes `Underlying` a candidate for NRVO, or at 
worst, implicit move.

To keep this kind of inefficiency at bay in the future, the fast code should 
probably be made easier to type than the slow code (as it's currently the 
opposite). Perhaps this could be solved by:

- making `raw_string_ostream` guarantee+document that it does not need to be 
`flush()`ed (similar to `raw_svector_ostream`), //and/or//
- making `raw_string_ostream::str()` return a `StringRef` rather than 
`std::string&`, to discourage using it to initialize `std::string`s

Implementing those two ideas would make simply `return Underlying;` the 
natural, correct, and efficient choice. They are, however, out of scope for 
this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115374

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/Testing/TestClangConfig.h
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclarationName.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/lib/Basic/Version.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/TestModuleFileExtension.cpp
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -203,7 +203,8 @@
 
 OS << "]";
 
-return OS.str();
+OS.flush();
+return Storage;
   }
 
   std::string dumpNodes(ArrayRef Nodes) {
@@ -218,7 +219,8 @@
 
 OS << "]";
 
-return OS.str();
+OS.flush();
+return Storage;
   }
 };
 
Index: clang/unittests/Analysis/MacroExpansionContextTest.cpp
===
--- clang/unittests/Analysis/MacroExpansionContextTest.cpp
+++ clang/unittests/Analysis/MacroExpansionContextTest.cpp
@@ -95,14 +95,16 @@
 std::string Buf;
 llvm::raw_string_ostream OS{Buf};
 Ctx.dumpExpandedTextsToStream(OS);
-return OS.str();
+OS.flush();
+return Buf;
   }
 
   static std::string dumpExpansionRanges(const MacroExpansionContext ) {
 std::string Buf;
 llvm::raw_string_ostream OS{Buf};
 Ctx.dumpExpansionRangesToStream(OS);
-return OS.str();
+OS.flush();
+return Buf;
   }
 };
 
Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -111,7 +111,8 @@
 
   Dumper.Visit(std::forward(N)...);
 
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }
 
 template 
@@ -126,7 +127,8 @@
 
   Dumper.Visit(std::forward(N)...);
 
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }
 
 const FunctionDecl *getFunctionNode(clang::ASTUnit *AST,
Index: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
===
--- clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -139,7 +139,8 @@
 
   Passes.run(*M);
 
-  return OS.str();
+  OS.flush();
+  return outString;
 }
 
 // Takes a function and runs it on a set of inputs
Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -263,7 +263,8 @@
   OS << " ";
 }
   });
-  return OS.str();
+  OS.flush();
+  return Storage;
 }
 
 void syntax::Node::assertInvariants() const {
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -927,5 +927,6 @@
   M.EndExpanded);
 }
   }
-  return OS.str();
+  OS.flush();
+  return Dump;
 }
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp

[PATCH] D115106: [clang-tidy] Fix `readability-static-accessed-through-instance` false negative for static methods

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D115106#3172949 , @simon.giesecke 
wrote:

> Thanks a lot for addressing this issue! I am just trying it on our codebase.
>
>> The problem actually has nothing to do with the out-of-line definition being 
>> inline; the problem is that hasDeclaration() of the memberExpr() will match 
>> the out-of-line definition, which obviously isn't marked static, so 
>> isStaticStorageClass() won't match.
>
> Hm, an out-of-line definition *cannot* have the `static` keyword. I wonder if 
> it's actually a bug (in the AST? or just the matcher?) that 
> `isStaticStorageClass` doesn't match here? I guess that other checks that use 
> `isStaticStorageClass` might be affected by this too?

`isStaticStorageClass()` calls through to `FunctionDecl::getStorageClass()` 
which reports the storage as written on that declaration in source. So this 
isn't a bug in the AST, it's a more of a misunderstanding of whether we're 
querying the storage duration/linkage for the method or whether we're querying 
whether it was written with the static keyword.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6017
+/// cxxMethodDecl(isStatic()) matches A::foo() but not A::bar()
+AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
+

I would prefer we not expose this AST matcher as a public one. There's 
sufficient confusion around "as written in source" vs "computed" vs "linkage" 
in this area that I think we should be careful when introducing matchers around 
storage classes and storage durations. Then again, the water is already pretty 
muddy here, so maybe this ship sailed...

Other potential solutions to this issue would be to expose an AST matcher to 
traverse to the canonical decl or only matches the canonical decl, then we 
could use that to wrap the existing call to `isStaticStorageClass()`. (Of 
course, we could make this matcher local to just that source file, as well.)

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115106

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

[PATCH] D115250: switched to emulated TLV on macOS before 10.7

[Kirill A. Korinsky via Phabricator via cfe-commits]

catap updated this revision to Diff 392865.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

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

https://reviews.llvm.org/D115250

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_codegen.cpp
  clang/test/OpenMP/sections_lastprivate_codegen.cpp
  clang/test/OpenMP/task_in_reduction_codegen.cpp
  clang/test/OpenMP/taskgroup_task_reduction_codegen.cpp
  clang/test/OpenMP/taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/taskloop_simd_in_reduction_codegen.cpp
  clang/test/Sema/darwin-tls.c
  clang/test/Sema/tls.c
  clang/test/SemaCXX/cxx11-thread-unsupported.cpp
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -819,7 +819,9 @@
 
   /// Tests whether the target uses emulated TLS as default.
   bool hasDefaultEmulatedTLS() const {
-return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment();
+return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment() ||
+   (isMacOSX() && !isMacOSXVersionLT(10, 4) &&
+isMacOSXVersionLT(10, 7));
   }
 
   /// Tests whether the target uses -data-sections as default.
Index: clang/test/SemaCXX/cxx11-thread-unsupported.cpp
===
--- clang/test/SemaCXX/cxx11-thread-unsupported.cpp
+++ clang/test/SemaCXX/cxx11-thread-unsupported.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-apple-macosx10.6 -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-apple-macosx10.3 -verify %s
 
 void f() {
   thread_local int x; // expected-error {{thread-local storage is not supported for the current target}}
Index: clang/test/Sema/tls.c
===
--- clang/test/Sema/tls.c
+++ clang/test/Sema/tls.c
@@ -5,9 +5,9 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s
 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -fsyntax-only %s
 
-// Darwin supports TLS since 10.7.
-// RUN: not %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -fsyntax-only %s
+// Darwin supports TLS since macOS 10.4.
+// RUN: not %clang_cc1 -triple x86_64-apple-darwin7 -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.4.0 -fsyntax-only %s
 
 // RUN: %clang_cc1 -triple x86_64-pc-win32 -fsyntax-only %s
 // RUN: %clang_cc1 -triple i386-pc-win32 -fsyntax-only %s
Index: clang/test/Sema/darwin-tls.c
===
--- clang/test/Sema/darwin-tls.c
+++ clang/test/Sema/darwin-tls.c
@@ -1,4 +1,5 @@
-// RUN: not %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.6 %s 2>&1 | FileCheck %s --check-prefix NO-TLS
+// RUN: not %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.3 %s 2>&1 | FileCheck %s --check-prefix NO-TLS
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.4 %s 2>&1 | FileCheck %s --check-prefix TLS
 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.7 %s 2>&1 | FileCheck %s --check-prefix TLS
 
 // RUN: not %clang_cc1 -fsyntax-only -triple arm64-apple-ios7.1 %s 2>&1 | FileCheck %s --check-prefix NO-TLS
Index: clang/test/OpenMP/taskloop_simd_in_reduction_codegen.cpp
===
--- clang/test/OpenMP/taskloop_simd_in_reduction_codegen.cpp
+++ clang/test/OpenMP/taskloop_simd_in_reduction_codegen.cpp
@@ -1,11 +1,11 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK1
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK2
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin7 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK1
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin7 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin7 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK2
 
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK3

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

[Michael Benfield via Phabricator via cfe-commits]

mbenfield added a comment.

@aaron.ballman Look to you like this is ready to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

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

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:2116
+
+  // __builtin_reduce_xor restricts the element type to vector of integers type
+  // only.

nit: the element type will be restricted to `int`, so maybe say something like 
`only supports vectors of integers`>



Comment at: clang/test/Sema/builtins-reduction-math.c:50
+  i = __builtin_reduce_xor(i);
+  // expected-error@-1 {{1st argument must be a vector of integers type (was 
'int')}}
+

`vector of integers type` sounds a bit off. Not sure if that's much better, but 
how about dropping the `type` from the end?


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

https://reviews.llvm.org/D115231

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

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Minor suggestion in the docs, but otherwise LGTM.




Comment at: llvm/docs/Coroutines.rst:1382
+The `coro.end` intrinsic in the normal path wouldn't do this since the 
coroutine
+should already be marked as done by final suspend.
+

How about:

  In the unwind path (when the argument is `true`), `coro.end` will mark the 
coroutine as done, making it undefined behavior to resume the coroutine again 
and causing `llvm.coro.done` to return `true`.  This is not necessary in the 
normal path because the coroutine will already be marked as done by the final 
suspend.



Comment at: llvm/docs/Coroutines.rst:1395
+|| Landingpad  | mark coroutine as done | mark coroutine done  
 |
+++-++---+
 

ChuanqiXu wrote:
> Maybe I need to explain why it would work in start (ramp) function. 
> Generally, the ramp function is very small. But in case the initial_suspend 
> of the coroutine wouldn't always suspend, the start function could be as 
> large as the original function. In this case, it should remain the original 
> semantics.
That makes sense.  Honestly, it feels very strange to me that there are any 
differences between the ramp function and the resume/destroy functions in the 
handling of `coro.end`.



Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:304
+Shape.FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume)));
+Builder.CreateStore(NullPtr, GepIndex);
 if (!InResume)

ChuanqiXu wrote:
> rjmccall wrote:
> > We need to do this store elsewhere, right, like during final suspends?  Can 
> > we make a common function for it?
> I am not sure if I understand your point. I think the place where the 
> `coro.end` lives should be the right place. The common function is made.
Yes, thank you, this is all I wanted.


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

https://reviews.llvm.org/D115219

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

[PATCH] D115222: [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay, that's fine, I have no objection to removing it.  LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115222

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

[PATCH] D113638: [xray] Add support for hexagon architecture

[Krzysztof Parzyszek via Phabricator via cfe-commits]

kparzysz accepted this revision.
kparzysz added a comment.
This revision is now accepted and ready to land.

LGTM.  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113638

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

[PATCH] D115320: Avoid setting tbaa information on return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

In D115320#3178682 , 
@jeroen.dobbelaere wrote:

> Do you have a testcase ?

Thank you for the review. Please take a look at the new diff.


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

https://reviews.llvm.org/D115320

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

[PATCH] D115320: Avoid setting tbaa information on return type of call to inline assember

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 392840.
schittir added a subscriber: mikerice.
schittir added a comment.

Add test case
Fix formatting


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

https://reviews.llvm.org/D115320

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/avoidTBAAonASMstore.cpp


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes 
-fasm-blocks %s -emit-llvm -o - | FileCheck %s
+float _Complex foo(float _Complex z) {
+// CHECK-LABEL: define{{.*}} i64 @_Z3fooCf
+   unsigned short ControlWord;
+   __asm { fnstcw word ptr[ControlWord] };
+// CHECK: store i64 %2, i64* %1, align 4
+// CHECK-NOT: !tbaa
+   return z;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,8 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() { return TBAAAccessInfo(); }
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2504,6 +2504,13 @@
 BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+AlignmentSource Source = AlignmentSource::Type) {
+return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+CGM.returnNullTBAA());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2454,7 +2454,7 @@
 const ABIArgInfo  = CurFnInfo->getReturnInfo();
 if (RetAI.isDirect() || RetAI.isExtend()) {
   // Make a fake lvalue for the return value slot.
-  LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+  LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
   CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
   *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
   ResultRegDests, AsmString, S.getNumOutputs());


Index: clang/test/CodeGen/avoidTBAAonASMstore.cpp
===
--- /dev/null
+++ clang/test/CodeGen/avoidTBAAonASMstore.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck %s
+float _Complex foo(float _Complex z) {
+// CHECK-LABEL: define{{.*}} i64 @_Z3fooCf
+	unsigned short ControlWord;
+	__asm { fnstcw word ptr[ControlWord] };
+// CHECK: store i64 %2, i64* %1, align 4
+// CHECK-NOT: !tbaa
+	return z;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,9 @@
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  /// returnNullTBAA - Return empty TBAA constructor
+  TBAAAccessInfo returnNullTBAA();
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -935,6 +935,8 @@
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::returnNullTBAA() { return TBAAAccessInfo(); }
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if 

[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

[Konstantin Pyzhov via Phabricator via cfe-commits]

kpyzhov added inline comments.



Comment at: clang/test/CodeGenHIP/amdgpu_hostcall.cpp:2-6
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -emit-llvm 
-fcuda-is-device -DFN_HOSTCALL \
+// RUN:   -o - %s | FileCheck --enable-var-scope %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -emit-llvm 
-fcuda-is-device -DFN_PRINTF \
+// RUN:   -o - %s | FileCheck --enable-var-scope %s

dfukalov wrote:
> Am I right we don't actually need two runs here, the test may be executed 
> with one run, removed `#ifdefs` and, possible, multiplied `CHECK:` lines?
> I would suggest to use the llvm/utils/update_cc_test_checks.py script in such 
> tests.
Well, it may be executed with one run, but in that case we won't be able to 
catch an error if one of the functions is broken, because the 2nd one will set 
the module flag.
Why do you think I should use the script for this test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 392835.
vtjnash added a comment.

  fix cmake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext | FileCheck %s
+// CHECK: Enabled checks:
+// CHECK-NEXT:mytest
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest");
+  }
+};
+} // namespace
+
+namespace clang {
+namespace tidy {
+
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,6 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  LLVM_ENABLE_PLUGINS
   )
 
 configure_lit_site_cfg(
@@ -78,6 +79,22 @@
   endif()
 endforeach()
 
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+
+if(TARGET CTTestTidyModule)
+list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule)
+target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+endif()
+endif()
+
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
Index: clang-tools-extra/docs/clang-tidy/index.rst

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

[Michael Benfield via Phabricator via cfe-commits]

mbenfield updated this revision to Diff 392830.
mbenfield added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-diagnose-as-builtin.c

Index: clang/test/Sema/attr-diagnose-as-builtin.c
===
--- /dev/null
+++ clang/test/Sema/attr-diagnose-as-builtin.c
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -Wfortify-source -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -Wfortify-source -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) int x; // expected-warning {{'diagnose_as_builtin' attribute only applies to functions}}
+
+void *test_memcpy(const void *src, size_t c, void *dst) __attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) {
+  return __builtin_memcpy(dst, src, c);
+}
+
+void call_test_memcpy() {
+  char bufferA[10];
+  char bufferB[11];
+  test_memcpy(bufferB, 10, bufferA);
+  test_memcpy(bufferB, 11, bufferA); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+void failure_function_doesnt_exist() __attribute__((diagnose_as_builtin(__function_that_doesnt_exist))) {} // expected-error {{use of undeclared identifier '__function_that_doesnt_exist'}}
+
+void not_a_builtin() {}
+
+void failure_not_a_builtin() __attribute__((diagnose_as_builtin(not_a_builtin))) {} // expected-error {{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_too_many_parameters(void *dst, const void *src, size_t count, size_t nothing) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3, 4))) {} // expected-error {{'diagnose_as_builtin' attribute references function '__builtin_memcpy', which takes exactly 3 arguments}}
+
+void failure_too_few_parameters(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2))) {} // expected-error{{'diagnose_as_builtin' attribute references function '__builtin_memcpy', which takes exactly 3 arguments}}
+
+void failure_not_an_integer(void *dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, "abc", 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 2 to be an integer constant}}
+
+void failure_not_a_builtin2() __attribute__((diagnose_as_builtin("abc"))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_parameter_index_bounds(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute references parameter 3, but the function 'failure_parameter_index_bounds' has only 2 parameters}}
+
+void failure_parameter_types(double dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute parameter types do not match: parameter 1 of function 'failure_parameter_types' has type 'double', but parameter 1 of function '__builtin_memcpy' has type 'void *'}}
+
+void to_redeclare(void *dst, const void *src, size_t count);
+
+void use_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // We shouldn't get an error yet.
+  to_redeclare(dst, src, 10);
+}
+
+void to_redeclare(void *dst, const void *src, size_t count) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void error_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // Now we get an error.
+  to_redeclare(dst, src, 10); // expected-warning {{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}}
+}
+
+// Make sure this works even with extra qualifiers and the pass_object_size attribute.
+void *memcpy2(void *const dst __attribute__((pass_object_size(0))), const void *src, size_t copy_amount) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void call_memcpy2() {
+  char buf1[10];
+  char buf2[11];
+  memcpy2(buf1, buf2, 11); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+// We require the types to be identical, modulo canonicalization and qualifiers.
+// Maybe this could be relaxed if it proves too restrictive.
+void failure_type(void *dest, char val, size_t n) __attribute__((diagnose_as_builtin(__builtin_memset, 1, 2, 3))) {} // expected-error {{'diagnose_as_builtin' attribute parameter types do not match: parameter 2 of function 'failure_type' has type 'char', but parameter 2 of function '__builtin_memset' has type 

[PATCH] D115250: switched to emulated TLV on macOS before 10.7

[Kirill A. Korinsky via Phabricator via cfe-commits]

catap updated this revision to Diff 392828.

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

https://reviews.llvm.org/D115250

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/Sema/darwin-tls.c
  clang/test/Sema/tls.c
  clang/test/SemaCXX/cxx11-thread-unsupported.cpp
  llvm/include/llvm/ADT/Triple.h


Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -819,7 +819,8 @@
 
   /// Tests whether the target uses emulated TLS as default.
   bool hasDefaultEmulatedTLS() const {
-return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment();
+return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment() ||
+   (isMacOSX() && !isMacOSXVersionLT(10, 4) && isMacOSXVersionLT(10, 
7));
   }
 
   /// Tests whether the target uses -data-sections as default.
Index: clang/test/SemaCXX/cxx11-thread-unsupported.cpp
===
--- clang/test/SemaCXX/cxx11-thread-unsupported.cpp
+++ clang/test/SemaCXX/cxx11-thread-unsupported.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-apple-macosx10.6 -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-apple-macosx10.3 -verify %s
 
 void f() {
   thread_local int x; // expected-error {{thread-local storage is not 
supported for the current target}}
Index: clang/test/Sema/tls.c
===
--- clang/test/Sema/tls.c
+++ clang/test/Sema/tls.c
@@ -5,9 +5,9 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s
 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -fsyntax-only %s
 
-// Darwin supports TLS since 10.7.
-// RUN: not %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -fsyntax-only %s
+// Darwin supports TLS since macOS 10.4.
+// RUN: not %clang_cc1 -triple x86_64-apple-darwin7 -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.4.0 -fsyntax-only %s
 
 // RUN: %clang_cc1 -triple x86_64-pc-win32 -fsyntax-only %s
 // RUN: %clang_cc1 -triple i386-pc-win32 -fsyntax-only %s
Index: clang/test/Sema/darwin-tls.c
===
--- clang/test/Sema/darwin-tls.c
+++ clang/test/Sema/darwin-tls.c
@@ -1,4 +1,5 @@
-// RUN: not %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.6 %s 2>&1 | 
FileCheck %s --check-prefix NO-TLS
+// RUN: not %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.3 %s 2>&1 | 
FileCheck %s --check-prefix NO-TLS
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.4 %s 2>&1 | 
FileCheck %s --check-prefix TLS
 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.7 %s 2>&1 | 
FileCheck %s --check-prefix TLS
 
 // RUN: not %clang_cc1 -fsyntax-only -triple arm64-apple-ios7.1 %s 2>&1 | 
FileCheck %s --check-prefix NO-TLS
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2580,7 +2580,9 @@
   const char *Name = "__cxa_atexit";
   if (TLS) {
 const llvm::Triple  = CGF.getTarget().getTriple();
-Name = T.isOSDarwin() ?  "_tlv_atexit" : "__cxa_thread_atexit";
+Name = (T.isOSDarwin() && !(T.isMacOSX() && T.isMacOSXVersionLT(10, 7)))
+   ? "_tlv_atexit"
+   : "__cxa_thread_atexit";
   }
 
   // We're assuming that the destructor function is something we can
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -91,7 +91,7 @@
 this->TLSSupported = false;
 
 if (Triple.isMacOSX())
-  this->TLSSupported = !Triple.isMacOSXVersionLT(10, 7);
+  this->TLSSupported = !Triple.isMacOSXVersionLT(10, 4);
 else if (Triple.isiOS()) {
   // 64-bit iOS supported it from 8 onwards, 32-bit device from 9 onwards,
   // 32-bit simulator from 10 onwards.


Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -819,7 +819,8 @@
 
   /// Tests whether the target uses emulated TLS as default.
   bool hasDefaultEmulatedTLS() const {
-return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment();
+return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment() ||
+   (isMacOSX() && !isMacOSXVersionLT(10, 4) && isMacOSXVersionLT(10, 7));
   }
 
   /// Tests whether the target uses -data-sections as default.
Index: clang/test/SemaCXX/cxx11-thread-unsupported.cpp
===
--- clang/test/SemaCXX/cxx11-thread-unsupported.cpp
+++ 

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the fix and your patience with getting it landed! I've commit on 
your behalf in 5d3b8956e83484ff7234dda5e939abbdc9bd2c88 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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

[clang-tools-extra] 5d3b895 - Don't offer partial fix-its for `modernize-pass-by-value`

[Aaron Ballman via cfe-commits]

Author: Adrian Vogelsgesang
Date: 2021-12-08T13:31:30-05:00
New Revision: 5d3b8956e83484ff7234dda5e939abbdc9bd2c88

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

LOG: Don't offer partial fix-its for `modernize-pass-by-value`

This commit improves the fix-its of modernize-pass-by-value by
no longer proposing partial fixes. In the presence of using/typedef,
we failed to rewrite the function signature but still adjusted the
function body. This led to incorrect, partial fix-its. Instead, the
check now simply doesn't offer any fixes at all in such a situation.

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
index 3caaa3c876ab3..34079c65b68cb 100644
--- a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -190,24 +190,35 @@ void PassByValueCheck::check(const 
MatchFinder::MatchResult ) {
 
   auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use 
std::move");
 
-  // Iterate over all declarations of the constructor.
-  for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
-auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
-auto RefTL = ParamTL.getAs();
-
-// Do not replace if it is already a value, skip.
-if (RefTL.isNull())
-  continue;
-
-TypeLoc ValueTL = RefTL.getPointeeLoc();
-auto TypeRange = CharSourceRange::getTokenRange(ParmDecl->getBeginLoc(),
-ParamTL.getEndLoc());
-std::string ValueStr = Lexer::getSourceText(CharSourceRange::getTokenRange(
-ValueTL.getSourceRange()),
-SM, getLangOpts())
-   .str();
-ValueStr += ' ';
-Diag << FixItHint::CreateReplacement(TypeRange, ValueStr);
+  // If we received a `const&` type, we need to rewrite the function
+  // declarations.
+  if (ParamDecl->getType()->isLValueReferenceType()) {
+// Check if we can succesfully rewrite all declarations of the constructor.
+for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
+  TypeLoc ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
+  ReferenceTypeLoc RefTL = ParamTL.getAs();
+  if (RefTL.isNull()) {
+// We cannot rewrite this instance. The type is probably hidden behind
+// some `typedef`. Do not offer a fix-it in this case.
+return;
+  }
+}
+// Rewrite all declarations.
+for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
+  TypeLoc ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
+  ReferenceTypeLoc RefTL = ParamTL.getAs();
+
+  TypeLoc ValueTL = RefTL.getPointeeLoc();
+  CharSourceRange TypeRange = CharSourceRange::getTokenRange(
+  ParmDecl->getBeginLoc(), ParamTL.getEndLoc());
+  std::string ValueStr =
+  Lexer::getSourceText(
+  CharSourceRange::getTokenRange(ValueTL.getSourceRange()), SM,
+  getLangOpts())
+  .str();
+  ValueStr += ' ';
+  Diag << FixItHint::CreateReplacement(TypeRange, ValueStr);
+}
   }
 
   // Use std::move in the initialization list.

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
index 45f51a902cc36..28e4014c43d36 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -231,5 +231,18 @@ struct T {
 
 struct U {
   U(const POD ) : M(M) {}
+  // CHECK-FIXES: U(const POD ) : M(M) {}
   POD M;
 };
+
+// The rewrite can't look through `typedefs` and `using`.
+// Test that we don't partially rewrite one decl without rewriting the other.
+using MovableConstRef = const Movable &;
+struct V {
+  V(MovableConstRef M);
+  // CHECK-FIXES: V(MovableConstRef M);
+  Movable M;
+};
+V::V(const Movable ) : M(M) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
+// CHECK-FIXES: V::V(const Movable ) : M(M) {}



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

[PATCH] D115118: [clang-tidy] Assume that `noexcept` functions won't throw anything in `bugprone-exception-escape` check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:201-202
+  case EST_BasicNoexcept:
+  case EST_DependentNoexcept: // Let's be optimistic here (necessary e.g.
+  // for variant assignment; see PR#52254)
+  case EST_NoexceptTrue:

Hmmm, I'm not certain I agree that we should be optimistic. If the exception 
specification is unknown and we want to be conservative, we can't determine 
that it might not throw, we can only determine that it might throw.

So this might be the correct behavior for this check but the incorrect behavior 
for other checks.

One possible approach is to not give an answer until the noexcept specification 
is resolved (e.g., return an `Optional` where `None` means "I can't 
answer the question", and callers have to react to that), another would be to 
pass in an argument specifying whether the caller wants to be conservative or 
aggressive in this case (that's how we handle "does this expression have side 
effects" in Clang).



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-exception-escape.cpp:311
+  est_noexcept_true();
+  est_dependent_noexcept>();
+}

Do we still issue the diagnostic if you remove this instantiation?

Can you add a second instantiation: 
`est_dependent_noexcept>();` to show that it does not 
diagnose?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115118

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

It seems like your test file passes even before your patch. I've just checked 
it. My last pull from the baseline was on Nov 15.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added a comment.

> What name and email address would you like me to use for patch attribution?

Adrian Vogelsgesang, avogelsges...@salesforce.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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

[PATCH] D115043: [clang][deps] Use MemoryBuffer in minimizing FS

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith 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/D115043/new/

https://reviews.llvm.org/D115043

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

[PATCH] D115331: [llvm] Add null-termination capability to SmallVectorMemoryBuffer

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM, except I'd prefer the `#include`s be changed separately since that 
cleanup seems unrelated to this change.




Comment at: lldb/unittests/Expression/CppModuleConfigurationTest.cpp:14
 #include "lldb/Host/HostInfo.h"
-#include "llvm/Support/SmallVectorMemoryBuffer.h"
 

Please clean up the includes in a prep commit (no need for review IMO) rather 
than a drive-by.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115331

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

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112648#3165302 , @avogelsgesang 
wrote:

> @aaron.ballman can you please commit this on my behalf? I don't have commit 
> access

OMG, I'm sorry for missing this note! Yes, I'm happy to commit on your behalf. 
What name and email address would you like me to use for patch attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 392821.
ASDenysPetrov added a comment.

Improved dynamic type recognition. Provided `-fstrict-aliasing` compiler flag 
dependency.
Still has issues with some cases (see it at the bottom of the test file). WIP.


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

https://reviews.llvm.org/D114718

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
@@ -0,0 +1,524 @@
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+
+template 
+void clang_analyzer_dump(T x);
+void clang_analyzer_eval(int);
+
+namespace std {
+enum class byte : unsigned char {};
+enum class OtherByte : unsigned char {};
+}; // namespace std
+enum class EnumInt : int {};
+
+union UnionUchar {
+  unsigned char x;
+  char y;
+};
+union UnionInt {
+  int x;
+};
+class Class {};
+class ClassInt {
+public:
+  int x;
+};
+class Base {
+public:
+  int x;
+};
+struct AnotherBase {
+  int y;
+};
+class Derived : public AnotherBase, public Base {
+  int z;
+};
+class MostDerived : public Derived {
+  int w;
+};
+
+using AliasedStdByte = std::byte;
+using AliasedChar = char;
+using AliasedSChar = const signed char;
+using AliasedInt = int;
+using AliasedUInt = const unsigned int;
+using AliasedULong = unsigned long;
+using AliasedBase = Base;
+using AliasedAnotherBase = AnotherBase;
+
+namespace ns1 {
+
+void int_casts() {
+  using MyInt = int;
+  MyInt x = {};
+  // int to records
+  {
+auto *ptr = (Class *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (ClassInt *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (UnionUchar *)
+ptr->x = 42; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (UnionInt *)
+ptr->x = 42; // expected-warning{{Undefined behavior}}
+  }
+  // int to records
+  {
+auto *ptr = (std::byte *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (std::OtherByte *)
+auto y = *ptr; // expected-warning{{Undefined behavior. Attempting to access the stored value of type}}
+  }
+  {
+auto *ptr = (EnumInt *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  // int to scalars
+  {
+auto *ptr = (const char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (unsigned char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed char *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (const volatile unsigned int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed int *)
+*ptr = 24; // no-warning
+  }
+  {
+auto *ptr = (long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (float *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  // int to aliases
+  {
+auto *ptr = (AliasedStdByte *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (AliasedChar *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (AliasedSChar *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (AliasedULong *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (AliasedInt *)
+auto y = *ptr; // 

[PATCH] D115283: [AMDGPU] Set "amdgpu_hostcall" module flag if an AMDGPU function has calls to device lib functions that use hostcalls.

[Konstantin Pyzhov via Phabricator via cfe-commits]

kpyzhov added a comment.

In D115283#3179651 , @yaxunl wrote:

> One drawback of this approach is that it does not work for LLVM modules 
> generated from assembly or programmatically e.g. Tensorflow XLA.
>
> Another drawback is that if `__ockl_call_host_function` or 
> `__ockl_fprintf_stderr_begin` are eliminated by optimizer, the module flag is 
> still kept. This could happen if users use printf in assert.
>
> Is there a way to detect use of hostcall later in LLVM IR not by calling of 
> these functions?

Two other possible solutions that come to my mind:

1. Make a separate pass that would check if there is an instance of the 
ockl_hostcall_internal() function in the module and set the module flag if so.

2. Add a new "amdgpu_hostcall" function attribute. The 
"ockl_hostcall_internal()" function should be declared with 
__attribute__(("amdgpu_hostcall")) in the device libs. Then, in the Code Gen 
pass, we just set the "hidden_hostcall" kernel arg attribute if any function 
with "amdgpu_hostcall" is present. I think this is the best solution since we 
don't rely on the particular function name, and it would work ideally with any 
optimizations.

Please let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115283

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

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

Thanks!




Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:35
+TypeErasedDataflowAnalysis ) {
+  TypeErasedDataflowAnalysisState State = {Analysis.typeErasedInitialElement(),
+   InitEnv};

sgatev wrote:
> xazax.hun wrote:
> > I did not catch this earlier, but I wonder if we should pass the block in 
> > to `typeErasedInitialElement`. There are some analysis where the initial 
> > element might be different for certain nodes. E.g. here is the algorithms 
> > for computing dominators from wikipedia:
> > ```
> >  // dominator of the start node is the start itself
> >  Dom(n0) = {n0}
> >  // for all other nodes, set all nodes as the dominators
> >  for each n in N - {n0}
> >  Dom(n) = N;
> >  // iteratively eliminate nodes that are not dominators
> >  while changes in any Dom(n)
> >  for each n in N - {n0}:
> >  Dom(n) = {n} union with intersection over Dom(p) for all p in 
> > pred(n)
> > ```
> > 
> > Here the initial analysis state for the entry node differs from the initial 
> > state for the rest of the nodes. 
> Good point, though, I'm not sure if this algorithm is the best choice for 
> such an analysis. It seems that in that case we don't need to evaluate the 
> statements in the basic blocks, right? Perhaps we can generalize the 
> algorithm or offer a separate one for more efficient implementation of such 
> analyses. I added a FIXME and will consider this a bit more.
Dominators is one example, but there might be other analyses where the initial 
state differs :)
Since this change is really easy to make, I'm ok deferring this until we see 
the actual need.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115235

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

Re: TCE target nonconforming definition of long long and intmax_t

[Pekka Jääskeläinen via cfe-commits]

Hi Aaron,

Indeed the 32b TCE target is not fully compliant in this aspect;
its 64b emulation support is not complete, therefore we advertise
only these 32b limits. We have an in-progress 64b target where the
limits are 64b, but it's not upstreamed yet.

Yes, TCE target is still maintained, but it's a special target since
it has target-specific backend phases off-tree for online retargeting 
and pecularities of the TTA scheduling. Thus, I'd just leave it as

is for the 32b target and we'll upstream the 64b stub as soon as
it stabilizes a bit.

BR,
Pekka

On 7.12.2021 16.59, Aaron Ballman wrote:

Hello! I was digging around in stdint.h to do some implementation work
on C2x and I noticed that the TCE target seems to be nonconforming.

In C17, the implementation limits for intmax_t and uintmax_t are
specified by 7.20.2.5 as:

— minimum value of greatest-width signed integer type
INTMAX_MIN -(2^63 - 1)
— maximum value of greatest-width signed integer type
INTMAX_MAX 2^63 - 1
— maximum value of greatest-width unsigned integer type
UINTMAX_MAX 2^64 - 1

Similarly, the implementation limits for long long and unsigned long
long are specified by 5.2.4.2.1p1:

minimum value for an object of type long long int
LLONG_MIN -9223372036854775807 // -(2^63 - 1)
— maximum value for an object of type long long int
LLONG_MAX +9223372036854775807 // 2^63 - 1
— maximum value for an object of type unsigned long long int
ULLONG_MAX 18446744073709551615 // 2^64 - 1

However, the TCE target appears to set these to 32-bit limits, not
64-bit limits:

https://github.com/llvm/llvm-project/blob/2ab513cd3e0648806db7ed1f8170ad4a3d4e7749/clang/lib/Basic/Targets/TCE.h#L61

Is this target still being maintained? If so, what should be done
here? (I can file a bug report about this once we have a bug database
that can accept new content so we don't lose track of this.)

Thanks!

~Aaron



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