[PATCH] D82442: [Coroutines] Warning if the return type of coroutine_handle::address is not void*

2020-07-05 Thread JunMa via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8849831d55a2: [Coroutines] Warning if return type of 
coroutine_handle::address is not void* (authored by ChuanqiXu, committed by 
junparser).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D82442?vs=273926=275589#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82442

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp

Index: clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -verify %s -stdlib=libc++ -std=c++1z -fcoroutines-ts -fsyntax-only
+
+namespace std::experimental {
+template 
+struct coroutine_handle;
+
+template <>
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+  void *address() const;
+};
+
+template 
+struct coroutine_handle : public coroutine_handle<> {
+};
+
+template 
+struct void_t_imp {
+  using type = void;
+};
+template 
+using void_t = typename void_t_imp::type;
+
+template 
+struct traits_sfinae_base {};
+
+template 
+struct traits_sfinae_base> {
+  using promise_type = typename T::promise_type;
+};
+
+template 
+struct coroutine_traits : public traits_sfinae_base {};
+} // namespace std::experimental
+
+struct suspend_never {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+struct task {
+  struct promise_type {
+auto initial_suspend() { return suspend_never{}; }
+auto final_suspend() noexcept { return suspend_never{}; }
+auto get_return_object() { return task{}; }
+static void unhandled_exception() {}
+void return_void() {}
+  };
+};
+
+namespace std::experimental {
+template <>
+struct coroutine_handle : public coroutine_handle<> {
+  coroutine_handle *address() const; // expected-warning {{return type of 'coroutine_handle<>::address should be 'void*'}}
+};
+} // namespace std::experimental
+
+struct awaitable {
+  bool await_ready();
+
+  std::experimental::coroutine_handle
+  await_suspend(std::experimental::coroutine_handle<> handle);
+  void await_resume();
+} a;
+
+task f() {
+  co_await a;
+}
+
+int main() {
+  f();
+  return 0;
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -391,7 +391,13 @@
 return nullptr;
 
   Expr *JustAddress = AddressExpr.get();
-  // FIXME: Check that the type of AddressExpr is void*
+
+  // Check that the type of AddressExpr is void*
+  if (!JustAddress->getType().getTypePtr()->isVoidPointerType())
+S.Diag(cast(JustAddress)->getCalleeDecl()->getLocation(),
+   diag::warn_coroutine_handle_address_invalid_return_type)
+<< JustAddress->getType();
+
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
   JustAddress);
 }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1751,6 +1751,7 @@
   ~InlinedRegionBodyRAII() { CGF.AllocaInsertPt = OldAllocaIP; }
 };
   };
+
 private:
   /// CXXThisDecl - When generating code for a C++ member function,
   /// this will hold the implicit 'this' declaration.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10527,6 +10527,9 @@
 def note_await_ready_no_bool_conversion : Note<
   "return type of 'await_ready' is required to be contextually convertible to 'bool'"
 >;
+def warn_coroutine_handle_address_invalid_return_type : Warning <
+  "return type of 'coroutine_handle<>::address should be 'void*' (have %0) in order to get capability with existing async C API.">,
+  InGroup;
 def err_coroutine_promise_final_suspend_requires_nothrow : Error<
   "the expression 'co_await __promise.final_suspend()' is required to be non-throwing"
 >;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 8849831 - [Coroutines] Warning if return type of coroutine_handle::address is not void*

2020-07-05 Thread Jun Ma via cfe-commits

Author: Chuanqi Xu
Date: 2020-07-06T13:46:01+08:00
New Revision: 8849831d55a203eca1069a0e11877ab7e7e0ac57

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

LOG: [Coroutines] Warning if return type of coroutine_handle::address is not 
void*

User can own a version of coroutine_handle::address() whose return type is not
void* by using template specialization for coroutine_handle<> for some
promise_type.

In this case, the codes may violate the capability with existing async C APIs
that accepted a void* data parameter which was then passed back to the
user-provided callback.

Patch by ChuanqiXu

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

Added: 
clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Sema/SemaCoroutine.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f0921337f312..5b94aa8c4325 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10527,6 +10527,9 @@ def err_await_suspend_invalid_return_type : Error<
 def note_await_ready_no_bool_conversion : Note<
   "return type of 'await_ready' is required to be contextually convertible to 
'bool'"
 >;
+def warn_coroutine_handle_address_invalid_return_type : Warning <
+  "return type of 'coroutine_handle<>::address should be 'void*' (have %0) in 
order to get capability with existing async C API.">,
+  InGroup;
 def err_coroutine_promise_final_suspend_requires_nothrow : Error<
   "the expression 'co_await __promise.final_suspend()' is required to be 
non-throwing"
 >;

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h 
b/clang/lib/CodeGen/CodeGenFunction.h
index 6b2538a677e5..b1841d646643 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1751,6 +1751,7 @@ class CodeGenFunction : public CodeGenTypeCache {
   ~InlinedRegionBodyRAII() { CGF.AllocaInsertPt = OldAllocaIP; }
 };
   };
+
 private:
   /// CXXThisDecl - When generating code for a C++ member function,
   /// this will hold the implicit 'this' declaration.

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 70b8fd282056..992cccac6405 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -391,7 +391,13 @@ static Expr *maybeTailCall(Sema , QualType RetType, Expr 
*E,
 return nullptr;
 
   Expr *JustAddress = AddressExpr.get();
-  // FIXME: Check that the type of AddressExpr is void*
+
+  // Check that the type of AddressExpr is void*
+  if (!JustAddress->getType().getTypePtr()->isVoidPointerType())
+S.Diag(cast(JustAddress)->getCalleeDecl()->getLocation(),
+   diag::warn_coroutine_handle_address_invalid_return_type)
+<< JustAddress->getType();
+
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
   JustAddress);
 }

diff  --git a/clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp 
b/clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp
new file mode 100644
index ..a95138365234
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -verify %s -stdlib=libc++ -std=c++1z -fcoroutines-ts 
-fsyntax-only
+
+namespace std::experimental {
+template 
+struct coroutine_handle;
+
+template <>
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+  void *address() const;
+};
+
+template 
+struct coroutine_handle : public coroutine_handle<> {
+};
+
+template 
+struct void_t_imp {
+  using type = void;
+};
+template 
+using void_t = typename void_t_imp::type;
+
+template 
+struct traits_sfinae_base {};
+
+template 
+struct traits_sfinae_base> {
+  using promise_type = typename T::promise_type;
+};
+
+template 
+struct coroutine_traits : public traits_sfinae_base {};
+} // namespace std::experimental
+
+struct suspend_never {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+struct task {
+  struct promise_type {
+auto initial_suspend() { return suspend_never{}; }
+auto final_suspend() noexcept { return suspend_never{}; }
+auto get_return_object() { return task{}; }
+static void unhandled_exception() {}
+void return_void() {}
+  };
+};
+
+namespace std::experimental {
+template <>
+struct coroutine_handle : public coroutine_handle<> {
+  coroutine_handle *address() const; // expected-warning 
{{return type of 'coroutine_handle<>::address should be 'void*'}}

[PATCH] D83071: Add support for options with two flags for controlling the same field.

2020-07-05 Thread Daniel Grumberg via Phabricator via cfe-commits
dang updated this revision to Diff 275586.
dang added a comment.

Revert accidental namespace removal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83071

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -156,6 +156,7 @@
   : MarshallingInfo {
   code NormalizerRetTy = ty;
   code Normalizer = "normalizeSimpleFlag";
+  code Denormalizer = "denormalizeSimpleFlag";
 }
 
 class MarshallingInfoBitfieldFlag : MarshallingInfoFlag {
@@ -164,6 +165,20 @@
   code ValueExtractor = "(extractMaskValue)";
 }
 
+class MarshallingInfoBooleanTrueFlag
+  : MarshallingInfoFlag {
+  bit ShouldAlwaysEmit = 1;
+  code Normalizer ="normalizeBooleanTrueFlag";
+  code Denormalizer = "denormalizeBooleanFlag";
+}
+
+class MarshallingInfoBooleanFalseFlag
+  : MarshallingInfoFlag {
+  bit ShouldAlwaysEmit = 1;
+  code Normalizer ="normalizeBooleanFalseFlag";
+  code Denormalizer = "denormalizeBooleanFlag";
+}
+
 // Mixins for additional marshalling attributes.
 
 class IsNegative {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -138,6 +138,13 @@
   return !Args.hasArg(Opt);
 }
 
+void denormalizeSimpleFlag(SmallVectorImpl ,
+   const char *Spelling,
+   CompilerInvocation::StringAllocator SA,
+   unsigned TableIndex, unsigned Value) {
+  Args.push_back(Spelling);
+}
+
 template 
 static llvm::Optional
 normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList ,
@@ -147,6 +154,33 @@
   return None;
 }
 
+template 
+static Optional
+normalizeBooleanTrueFlag(OptSpecifier PosOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+  if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+  return Args.hasFlag(PosOpt, NegOpt);
+}
+
+template 
+static Optional
+normalizeBooleanFalseFlag(OptSpecifier NegOpt, unsigned TableIndex,
+  const ArgList , DiagnosticsEngine ) {
+  if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+  return Args.hasFlag(PosOpt, NegOpt);
+}
+
+template 
+static void denormalizeBooleanFlag(SmallVectorImpl ,
+   const char *Spelling,
+   CompilerInvocation::StringAllocator SA,
+   unsigned TableIndex, unsigned Value) {
+  if (Value == IsPositive)
+Args.push_back(Spelling);
+}
+
 static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
 unsigned TableIndex,
 const ArgList ,
@@ -169,12 +203,14 @@
 }
 
 static void denormalizeSimpleEnum(SmallVectorImpl ,
+  const char *Spelling,
   CompilerInvocation::StringAllocator SA,
   unsigned TableIndex, unsigned Value) {
   assert(TableIndex < SimpleEnumValueTablesSize);
   const SimpleEnumValueTable  = SimpleEnumValueTables[TableIndex];
   for (int I = 0, E = Table.Size; I != E; ++I) {
 if (Value == Table.Table[I].Value) {
+  Args.push_back(Spelling);
   Args.push_back(Table.Table[I].Name);
   return;
 }
@@ -185,8 +221,10 @@
 }
 
 static void denormalizeString(SmallVectorImpl ,
+  const char *Spelling,
   CompilerInvocation::StringAllocator SA,
   unsigned TableIndex, const std::string ) {
+  Args.push_back(Spelling);
   Args.push_back(SA(Value));
 }
 
@@ -782,10 +820,6 @@
 }
   }
 
-  Opts.ExperimentalNewPassManager = Args.hasFlag(
-  OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
-  /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
-
   Opts.DebugPassManager =
   Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
/* Default */ false);
@@ -3895,13 +3929,7 @@
 TYPE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)\
   if (((FLAGS)::CC1Option) &&  \
   (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {\
-if (Option::KIND##Class == Option::FlagClass) {\
-  Args.push_back(SPELLING);\
-}  \
-if (Option::KIND##Class == Option::SeparateClass) {\
-  

Re: [PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Eric Christopher via cfe-commits
On Sun, Jul 5, 2020 at 8:47 PM Arthur Eubanks via Phabricator <
revi...@reviews.llvm.org> wrote:

> aeubanks added a comment.
>
> In D83013#2132070 , @echristo
> wrote:
>
> > Adding Chandler and Alina here as well.
> >
> > In general, I don't think that this is such a great idea. Being able to
> have this sort of thing work more reliably is one of the reasons for the
> new pass manager. I think I'd like to see this split out into an old versus
> new pass manager pass to avoid the difficulty of cleaning this up after we
> finish migrating llvm to the new pass manager. This also seems to add some
> technical debt around options and other enablement which is also less than
> ideal. Is this compelling to add right now versus finishing work migrating
> llvm completely to the new pass manager and removing the old one? From
> speaking with Alina I think that work should be done in a short while.
> >
> > Thanks.
> >
> > -eric
>
>
> I don't think we're that close yet, probably at least a couple months out,
> there are lots of loose ends to be tied up. I'll make a post soon in
> llvm-dev (maybe first we can sync up again) about what I think needs to be
> done before the NPM switch.
>
>
Honestly at this point that is a couple of months :) I think we may be able
to speed things up as well.

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


[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D83013#2132070 , @echristo wrote:

> Adding Chandler and Alina here as well.
>
> In general, I don't think that this is such a great idea. Being able to have 
> this sort of thing work more reliably is one of the reasons for the new pass 
> manager. I think I'd like to see this split out into an old versus new pass 
> manager pass to avoid the difficulty of cleaning this up after we finish 
> migrating llvm to the new pass manager. This also seems to add some technical 
> debt around options and other enablement which is also less than ideal. Is 
> this compelling to add right now versus finishing work migrating llvm 
> completely to the new pass manager and removing the old one? From speaking 
> with Alina I think that work should be done in a short while.
>
> Thanks.
>
> -eric


I don't think we're that close yet, probably at least a couple months out, 
there are lots of loose ends to be tied up. I'll make a post soon in llvm-dev 
(maybe first we can sync up again) about what I think needs to be done before 
the NPM switch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013



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


[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-07-05 Thread Johel Ernesto Guerrero Peña via Phabricator via cfe-commits
JohelEGP added a comment.

Another weird formatting when direct-initializing a variable with a 
non-parenthesized requires-expression:

  constexpr void test_width()
  {
  bool invalid_rep{!requires {typename jge::width;
  }
  }
  ;
  }

vs

  constexpr void test_width()
  {
  bool invalid_rep{!(requires { typename jge::width; })};
  bool invalid_rep = !(requires { typename jge::width; });
  bool invalid_rep = !requires
  {
  typename jge::width;
  };
  }


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

https://reviews.llvm.org/D79773



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


[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added reviewers: chandlerc, asbirlea.
echristo added a comment.

Adding Chandler and Alina here as well.

In general, I don't think that this is such a great idea. Being able to have 
this sort of thing work more reliably is one of the reasons for the new pass 
manager. I think I'd like to see this split out into an old versus new pass 
manager pass to avoid the difficulty of cleaning this up after we finish 
migrating llvm to the new pass manager. This also seems to add some technical 
debt around options and other enablement which is also less than ideal. Is this 
compelling to add right now versus finishing work migrating llvm completely to 
the new pass manager and removing the old one? From speaking with Alina I think 
that work should be done in a short while.

Thanks.

-eric


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013



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


[PATCH] D82547: [Debugify] Expose debugify (original mode) as CC1 option

2020-07-05 Thread Vedant Kumar via Phabricator via cfe-commits
vsk requested changes to this revision.
vsk added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:855
 
+class ClangCustomPassManager : public legacy::PassManager {
+public:

Please factor out OptCustomPassManager from opt and generalize it so it can be 
used by both opt and clang. That should help ensure that extensions and bug 
fixes are only made to one custom 'debugify' pass manager.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:893
+
+  void enableDebugifyEachOriginal() { DebugifyEachOriginalEnabled = true; }
+

I don't think the discussion from 'RFC: Introduce LLVM DI Checker utility' is 
complete, and I'd ask that you split off changes for 'original mode' from this 
patch until there's some consensus about what that mode should look like.

There are open questions about to what extent a new mode is needed (e.g., it 
may be that the interesting questions compiler developers need to answer about 
debug info loss are simpler to determine some other way (which is not to say 
that that's true -- just that we haven't explored the space much yet)). Or what 
its output should look like.


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

https://reviews.llvm.org/D82547



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


[PATCH] D83176: [OpenMPIRBuilder][Fix] Move llvm::omp::types to OpenMPIRBuilder.

2020-07-05 Thread Hongtao Yu via Phabricator via cfe-commits
hoyFB added a comment.

Thanks for the quick turnaround! I confirm the change fixes our build break.




Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:313
   {
\
 SmallVector ArgsTypes({__VA_ARGS__});   
\
 Function *F = M.getFunction(_Name);
\

sstefan1 wrote:
> I wasn't sure how to handle `__VA_ARGS__` here, since we would need 
> `OMPBuilder` in front of every type. 
> That is why helper macros above exist. The problem with this is that this 
> creates some unused variables in `OpenMPOpt`.
> 
> Not sure if `-Wno-unused-variable` would be a good thing to do temporarily? 
> Is there another way to handle `__VA_ARGS__` here?
Could it be possible to use `OMPBuilder. _Name`, `OMPBuilder. _ReturnType` 
everywhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83176



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


[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: llvm/tools/opt/opt.cpp:281
 
+static cl::opt EnableCallGraphProfile(
+"enable-call-graph-profile", cl::init(true), cl::Hidden,

zhizhouy wrote:
> MaskRay wrote:
> > zhizhouy wrote:
> > > MaskRay wrote:
> > > > If there is no strong need for tuning this, please delete the option 
> > > > and PassManagerBuilder::CallGraphProfile
> > > > 
> > > > -
> > > > 
> > > > I know that `-enable-npm-call-graph-profile` exists, but it seems like 
> > > > a temporary workaround for me. @zhizhouy @void (D62627) Is the option 
> > > > still used?
> > > Does GNU assembler recognize .cgprofile section now?
> > > 
> > > I think we should keep this option as long as there is still usage of 
> > > other than integrated assembler.
> > > Does GNU assembler recognize .cgprofile section now?
> > 
> > I don't think it will ever support this section.
> > 
> > > I think we should keep this option as long as there is still usage of 
> > > other than integrated assembler.
> > 
> > Can you give a link about the use case?
> I have a link of the effort to migrate Linux kernel to integrated assembler 
> in ChromeOS: https://bugs.chromium.org/p/chromium/issues/detail?id=1020923
> 
> I think they are only to migrate newer versions thus the old linux kernels 
> are still using GNU assembler, and all kernels are built with LLVM now.
If we have to have an option, we should make both legacy pm and new pm use the 
same option. We should not create two cl::opt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013



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


[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D83178#2132034 , @njames93 wrote:

> We should also disable any header-guard related checks. Currently with this 
> patch llvm-header-guard will fire on every header file as no macros get 
> marked as being used for header guards by the pre processor. I'm guessing 
> this is preamble related.


That's a good point, shouldn't stop us landing this fix but we should do it 
carefully.

I have a patch https://reviews.llvm.org/D78038 lying around I could clean up 
that I suspect fixes this. (I hadn't bothered so far as I hadn't seen any 
really terrible consequences, but I think this qualifies)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178



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


[PATCH] D82739: Improve heuristic resolution of dependent types in TargetFinder

2020-07-05 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added reviewers: sammccall, kadircet, hokein.
nridge added a comment.

Adding some reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82739



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


[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

We should also disable any header-guard related checks. Currently with this 
patch llvm-header-guard will fire on every header file as no macros get marked 
as being used for header guards by the pre processor. I'm guessing this is 
preamble related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178



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


[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:347
   // FIXME: the PP callbacks skip the entire preamble.
   // Checks that want to see #includes in the main file do not see them.
   Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);

Not directly related to this, but surely this comment is redundant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178



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


[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Zhizhou Yang via Phabricator via cfe-commits
zhizhouy added inline comments.



Comment at: llvm/tools/opt/opt.cpp:281
 
+static cl::opt EnableCallGraphProfile(
+"enable-call-graph-profile", cl::init(true), cl::Hidden,

MaskRay wrote:
> zhizhouy wrote:
> > MaskRay wrote:
> > > If there is no strong need for tuning this, please delete the option and 
> > > PassManagerBuilder::CallGraphProfile
> > > 
> > > -
> > > 
> > > I know that `-enable-npm-call-graph-profile` exists, but it seems like a 
> > > temporary workaround for me. @zhizhouy @void (D62627) Is the option still 
> > > used?
> > Does GNU assembler recognize .cgprofile section now?
> > 
> > I think we should keep this option as long as there is still usage of other 
> > than integrated assembler.
> > Does GNU assembler recognize .cgprofile section now?
> 
> I don't think it will ever support this section.
> 
> > I think we should keep this option as long as there is still usage of other 
> > than integrated assembler.
> 
> Can you give a link about the use case?
I have a link of the effort to migrate Linux kernel to integrated assembler in 
ChromeOS: https://bugs.chromium.org/p/chromium/issues/detail?id=1020923

I think they are only to migrate newer versions thus the old linux kernels are 
still using GNU assembler, and all kernels are built with LLVM now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013



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


[PATCH] D83183: [clang] Rework how and when APValues are dumped

2020-07-05 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno created this revision.
riccibruno added reviewers: aaron.ballman, steveire, ilya-biryukov, sammccall, 
martong.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, rnkovacs.

Currently `APValue`s are dumped as a single string. This becomes quickly 
completely
unreadable since `APValue` is a tree-like structure. Even a simple example is 
not pretty:

  struct S { int arr[4]; float f; };
  constexpr S s = { .arr = {1,2}, .f = 3.1415f };
  // Struct  fields: Array: Int: 1, Int: 2, 2 x Int: 0, Float: 3.141500e+00

With this patch this becomes:

  -Struct
   |-field: Array size=4
   | |-elements: Int 1, Int 2
   | `-filler: 2 x Int 0
   `-field: Float 3.141500e+00

Additionally `APValue`s are currently only dumped as part of visiting a 
`ConstantExpr`.
This patch also dump the value of the initializer (if any) of constexpr 
variable declarations:

  constexpr int foo(int a, int b) { return a + b - 42; }
  constexpr int a = 1, b = 2;
  constexpr int c = foo(a, b) > 0 ? foo(a, b) : foo(b, a);
  // VarDecl 0x6218aec8  col:17 c 'const int' constexpr cinit
  // |-value: Int -39
  // `-ConditionalOperator 0x6218b4d0  'int'
  // 

Do the above by moving the dump functions to `TextNodeDumper` which already has
the machinery to display trees. The cases `APValue::LValue`, 
`APValue::MemberPointer`
and `APValue::AddrLabelDiff` are left as they were before (unimplemented).

We try to display multiple elements on the same line if they are considered to 
be "simple".
This is to avoid wasting large amounts of vertical space in an example like:

  constexpr int arr[8] = {0,1,2,3,4,5,6,7};
  // VarDecl 0x6218bb78  col:17 arr 'int const[8]' constexpr 
cinit
  // |-value: Array size=8
  // | |-elements: Int 0, Int 1, Int 2, Int 3
  // | `-elements: Int 4, Int 5, Int 6, Int 7


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83183

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-arithmetic.cpp
  clang/test/AST/ast-dump-APValue-array.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-APValue-vector.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/pr43983.cpp
  clang/test/Import/switch-stmt/test.cpp
  clang/test/Tooling/clang-check-ast-dump.cpp

Index: clang/test/Tooling/clang-check-ast-dump.cpp
===
--- clang/test/Tooling/clang-check-ast-dump.cpp
+++ clang/test/Tooling/clang-check-ast-dump.cpp
@@ -33,6 +33,7 @@
 // CHECK-ATTR-NEXT: FieldDecl{{.*}}n
 // CHECK-ATTR-NEXT:   AlignedAttr
 // CHECK-ATTR-NEXT: ConstantExpr
+// CHECK-ATTR-NEXT:   value: Int 2
 // CHECK-ATTR-NEXT:   BinaryOperator
 //
 // RUN: clang-check -ast-dump -ast-dump-filter test_namespace::AfterNullNode "%s" -- 2>&1 | FileCheck -check-prefix CHECK-AFTER-NULL %s
Index: clang/test/Import/switch-stmt/test.cpp
===
--- clang/test/Import/switch-stmt/test.cpp
+++ clang/test/Import/switch-stmt/test.cpp
@@ -5,20 +5,26 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 4
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
@@ -30,16 +36,20 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
Index: 

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: llvm/tools/opt/opt.cpp:281
 
+static cl::opt EnableCallGraphProfile(
+"enable-call-graph-profile", cl::init(true), cl::Hidden,

zhizhouy wrote:
> MaskRay wrote:
> > If there is no strong need for tuning this, please delete the option and 
> > PassManagerBuilder::CallGraphProfile
> > 
> > -
> > 
> > I know that `-enable-npm-call-graph-profile` exists, but it seems like a 
> > temporary workaround for me. @zhizhouy @void (D62627) Is the option still 
> > used?
> Does GNU assembler recognize .cgprofile section now?
> 
> I think we should keep this option as long as there is still usage of other 
> than integrated assembler.
> Does GNU assembler recognize .cgprofile section now?

I don't think it will ever support this section.

> I think we should keep this option as long as there is still usage of other 
> than integrated assembler.

Can you give a link about the use case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013



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


[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

In D83178#2131948 , @kadircet wrote:

> In D83178#2131914 , @sammccall wrote:
>
> > > can emit diagnostics at this callback (as mentioned in the comments).
> >
> > I'm a bit puzzled about this. The case that the code handles and the 
> > comment refers to is if a check installs PPCallbacks and overrides 
> > EndSourceFile. And I believe the existing code is ok for this case: we 
> > don't detach those callbacks.
>
>
> right, we invoke the callback but we replace the diagnostic consumer client 
> with `Clang->getDiagnostics().setClient(new IgnoreDiagnostics);` before 
> invoking the callback. Hence whenever the clang-tidy check emits the 
> diagnostics it doesn't get handled by `StoreDiags`, instead it's received by 
> `IgnoreDiagnostics`.


Yeah, hmm... How did this ever work! Obviously I never wrote enough tests here, 
but that line comes very close to fixing a real issue that I feel sure I 
manually tested.

Maybe this originally worked and there was another change at some point that 
broke it.

>> It didn't occur to me that EOF could be observed through the 
>> DiagnosticConsumer instead, and this patch does look like a reasonable fix 
>> for that. However ClangTidyDiagnosticConsumer doesn't override EndSourceFile 
>> nor even the destructor, so it doesn't seem to actually happen.

(This would possibly be relevant if ClangTidyDiagnosticConsumer were used, in 
clangd, oops)

>> (We have a test with a fake clang tidy check IIRC, I wonder if we can test 
>> this there)
> 
> I was being lazy :( as that fake tidy check is not really re-usable, so we 
> either extend the test, duplicate the logic or refactor it out. I wanted to 
> go with the last option as (IIRC) we already have 2 tests with fake 
> clang-tidy checkers.
>  Will do that though!

Great to pay down the tech debt here, though the bar of "at least understanding 
why we're doing it" is met (I was just being slow). And it's not like this 
change adds any complexity.
So if the trade-off between test complexity vs brittleness vs 
likely-to-catch-a-bug feels poor that's ok.

Another option is to write a test using a real check that (today, and likely in 
the future) triggers diagnostics on EOF. I think we use this technique to 
smoke-test clang-tidy already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178



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


[PATCH] D81514: [Fuchsia] Set projects and runtimes in the cache file

2020-07-05 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa378c0449507: [Fuchsia] Set projects and runtimes in the 
cache file (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D81514?vs=269692=275579#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81514

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake

Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -4,6 +4,9 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
+
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
 set(LLVM_ENABLE_TERMINFO OFF CACHE BOOL "")
@@ -55,14 +58,16 @@
 if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
   set(target "${BOOTSTRAP_CMAKE_CXX_COMPILER_TARGET}")
   if(STAGE2_LINUX_${target}_SYSROOT)
+set(LLVM_BUILTIN_TARGETS "${target}" CACHE STRING "")
 set(BUILTINS_${target}_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
 set(BUILTINS_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
 set(BUILTINS_${target}_CMAKE_SYSROOT ${STAGE2_LINUX_${target}_SYSROOT} CACHE STRING "")
-set(LLVM_BUILTIN_TARGETS "${target}" CACHE STRING "")
 
+set(LLVM_RUNTIME_TARGETS "${target}" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_SYSROOT ${STAGE2_LINUX_${target}_SYSROOT} CACHE STRING "")
+set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
@@ -76,10 +81,9 @@
 set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
-set(LLVM_RUNTIME_TARGETS "${target}" CACHE STRING "")
   endif()
 endif()
 
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -4,6 +4,9 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
+
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 if(NOT APPLE)
   set(LLVM_ENABLE_LLD ON CACHE BOOL "")
@@ -44,6 +47,7 @@
 
   set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "")
   set(COMPILER_RT_ENABLE_WATCHOS OFF CACHE BOOL "")
+  set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
@@ -82,7 +86,7 @@
 set(RUNTIMES_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
-set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
@@ -95,9 +99,10 @@
 set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
 # Use .build-id link.
 list(APPEND RUNTIME_BUILD_ID_LINK "${target}")
@@ -142,7 +147,7 @@
 

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D83178#2131976 , @sammccall wrote:

> Maybe this originally worked and there was another change at some point that 
> broke it.


Initial commit looks pretty broken at first glance though: 
https://github.com/llvm/llvm-project/commit/98ae975187c75ad17a1fdc3e82a844e9ae3f5c84#diff-90ae535abf6a5bdd201b472a160e3e09

I have no excuses :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178



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


[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Zhizhou Yang via Phabricator via cfe-commits
zhizhouy added inline comments.



Comment at: llvm/tools/opt/opt.cpp:281
 
+static cl::opt EnableCallGraphProfile(
+"enable-call-graph-profile", cl::init(true), cl::Hidden,

MaskRay wrote:
> If there is no strong need for tuning this, please delete the option and 
> PassManagerBuilder::CallGraphProfile
> 
> -
> 
> I know that `-enable-npm-call-graph-profile` exists, but it seems like a 
> temporary workaround for me. @zhizhouy @void (D62627) Is the option still 
> used?
Does GNU assembler recognize .cgprofile section now?

I think we should keep this option as long as there is still usage of other 
than integrated assembler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013



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


[clang] a378c04 - [Fuchsia] Set projects and runtimes in the cache file

2020-07-05 Thread Petr Hosek via cfe-commits

Author: Petr Hosek
Date: 2020-07-05T14:48:52-07:00
New Revision: a378c0449507e00e96534ff9ce9034e185425182

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

LOG: [Fuchsia] Set projects and runtimes in the cache file

We make assumptions about what projects and runtimes are enabled
when configuring our toolchain build, so we should enable those in
the cache file as well rather than relying on those being set
externally.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake
clang/cmake/caches/Fuchsia.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 4ef404b8aaef..9ecf8f300e75 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -4,6 +4,9 @@ set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64;RISCV CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING 
"")
+
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 if(NOT APPLE)
   set(LLVM_ENABLE_LLD ON CACHE BOOL "")
@@ -44,6 +47,7 @@ if(APPLE)
 
   set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "")
   set(COMPILER_RT_ENABLE_WATCHOS OFF CACHE BOOL "")
+  set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
@@ -82,7 +86,7 @@ foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unkn
 set(RUNTIMES_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
-set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
@@ -95,9 +99,10 @@ foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unkn
 set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
 # Use .build-id link.
 list(APPEND RUNTIME_BUILD_ID_LINK "${target}")
@@ -142,7 +147,7 @@ if(FUCHSIA_SDK)
 set(RUNTIMES_${target}-unknown-fuchsia_CMAKE_MODULE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(RUNTIMES_${target}-unknown-fuchsia_CMAKE_EXE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(RUNTIMES_${target}-unknown-fuchsia_CMAKE_SYSROOT 
${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
-set(RUNTIMES_${target}-unknown-fuchsia_LLVM_ENABLE_ASSERTIONS ON CACHE 
BOOL "")
+set(RUNTIMES_${target}-unknown-fuchsia_COMPILER_RT_USE_BUILTINS_LIBRARY ON 
CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_USE_COMPILER_RT ON CACHE 
BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_HERMETIC_STATIC_LIBRARY 
ON CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_INSTALL_STATIC_LIBRARY 
OFF CACHE BOOL "")
@@ -157,6 +162,8 @@ if(FUCHSIA_SDK)
 set(RUNTIMES_${target}-unknown-fuchsia_LIBCXX_HERMETIC_STATIC_LIBRARY ON 
CACHE BOOL "")
 
set(RUNTIMES_${target}-unknown-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY
 OFF CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING 
"")
+set(RUNTIMES_${target}-unknown-fuchsia_LLVM_ENABLE_ASSERTIONS ON CACHE 
BOOL "")
+set(RUNTIMES_${target}-unknown-fuchsia_LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 set(RUNTIMES_${target}-unknown-fuchsia+asan_LLVM_BUILD_COMPILER_RT OFF 
CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia+asan_LLVM_USE_SANITIZER "Address" 
CACHE STRING "")

diff  --git a/clang/cmake/caches/Fuchsia.cmake 
b/clang/cmake/caches/Fuchsia.cmake
index 2a65de2039c2..bb22a00fa5c7 100644
--- a/clang/cmake/caches/Fuchsia.cmake
+++ 

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

In D83178#2131914 , @sammccall wrote:

> > can emit diagnostics at this callback (as mentioned in the comments).
>
> I'm a bit puzzled about this. The case that the code handles and the comment 
> refers to is if a check installs PPCallbacks and overrides EndSourceFile. And 
> I believe the existing code is ok for this case: we don't detach those 
> callbacks.


right, we invoke the callback but we replace the diagnostic consumer client 
with `Clang->getDiagnostics().setClient(new IgnoreDiagnostics);` before 
invoking the callback. Hence whenever the clang-tidy check emits the 
diagnostics it doesn't get handled by `StoreDiags`, instead it's received by 
`IgnoreDiagnostics`.

> It didn't occur to me that EOF could be observed through the 
> DiagnosticConsumer instead, and this patch does look like a reasonable fix 
> for that. However ClangTidyDiagnosticConsumer doesn't override EndSourceFile 
> nor even the destructor, so it doesn't seem to actually happen.
> 
> LLVM-include-order is hooking PPCallbacks::EndOfMainFile, i.e. the first case 
> and the one I thought we were already handling, and I wouldn't expect 
> changing the relative order of EOF and resetting the DC to matter for that.

as explained above issue isn't about diagsconsumer (or 
`PPCallbacks::EndOfMainFile` which is invoked by `PP.EndSourceFile()`). It is 
just about having the no-op diagnostics client registered when 
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp#L151
 is hit.

> So I believe you that it fixes something but I'd feel better having some idea 
> of how :-)
>  (We have a test with a fake clang tidy check IIRC, I wonder if we can test 
> this there)

I was being lazy :( as that fake tidy check is not really re-usable, so we 
either extend the test, duplicate the logic or refactor it out. I wanted to go 
with the last option as (IIRC) we already have 2 tests with fake clang-tidy 
checkers.
Will do that though!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178



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


[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

> can emit diagnostics at this callback (as mentioned in the comments).

I'm a bit puzzled about this. The case that the code handles and the comment 
refers to is if a check installs PPCallbacks and overrides EndSourceFile. And I 
believe the existing code is ok for this case: we don't detach those callbacks.

It didn't occur to me that EOF could be observed through the DiagnosticConsumer 
instead, and this patch does look like a reasonable fix for that. However 
ClangTidyDiagnosticConsumer doesn't override EndSourceFile nor even the 
destructor, so it doesn't seem to actually happen.

LLVM-include-order is hooking PPCallbacks::EndOfMainFile, i.e. the first case 
and the one I thought we were already handling, and I wouldn't expect changing 
the relative order of EOF and resetting the DC to matter for that.

So I believe you that it fixes something but I'd feel better having some idea 
of how :-)
(We have a test with a fake clang tidy check IIRC, I wonder if we can test this 
there)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178



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


[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added subscribers: zhizhouy, void.
MaskRay added inline comments.



Comment at: llvm/tools/opt/opt.cpp:281
 
+static cl::opt EnableCallGraphProfile(
+"enable-call-graph-profile", cl::init(true), cl::Hidden,

If there is no strong need for tuning this, please delete the option and 
PassManagerBuilder::CallGraphProfile

-

I know that `-enable-npm-call-graph-profile` exists, but it seems like a 
temporary workaround for me. @zhizhouy @void (D62627) Is the option still used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013



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


[PATCH] D82425: [SemaCXX] Fix false positive of -Wuninitialized-const-reference in empty function body.

2020-07-05 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu marked an inline comment as done.
zequanwu added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:435
   if ((*I)->getType().isConstQualified())
-classify((*I), ConstRefUse);
+if (!hasTrivialBody(CE))
+  classify((*I), ConstRefUse);

nick wrote:
> zequanwu wrote:
> > aaron.ballman wrote:
> > > This can be hoisted out of the loop so that we don't have to check the 
> > > same thing on every argument.
> > The `DeclRefExpr` needs to be set to `Ignore` like `VisitCastExpr` does. 
> > Otherwise, it maybe classified to `Init` by `isTrackedVar` in 
> > `ClassifyRefs::get`.
> Could not the empty body check be done in `reportConstRefUse`, after 
> `isUninitialized`?
No, `reportConstRefUse` doesn't know if the called function has trivial body or 
not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82425



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


[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

2020-07-05 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 275558.
zequanwu added a comment.

Enable CGProfilePass for opt with LPM by default, like opt with NPM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83013

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/IPO.h
  llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h
  llvm/include/llvm/Transforms/Instrumentation/CGProfile.h
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Instrumentation/CGProfile.cpp
  llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
  llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
  llvm/test/Instrumentation/cgprofile.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -278,6 +278,10 @@
 cl::desc("Specify time trace file destination"),
 cl::value_desc("filename"));
 
+static cl::opt EnableCallGraphProfile(
+"enable-call-graph-profile", cl::init(true), cl::Hidden,
+cl::desc("Enable call graph profile pass (default = on)"));
+
 static cl::opt RemarksWithHotness(
 "pass-remarks-with-hotness",
 cl::desc("With PGO, include profile count in optimization remarks"),
@@ -414,6 +418,8 @@
 
   Builder.SLPVectorize = OptLevel > 1 && SizeLevel < 2;
 
+  Builder.CallGraphProfile = EnableCallGraphProfile;
+
   if (TM)
 TM->adjustPassManager(Builder);
 
Index: llvm/test/Other/opt-Os-pipeline.ll
===
--- llvm/test/Other/opt-Os-pipeline.ll
+++ llvm/test/Other/opt-Os-pipeline.ll
@@ -266,6 +266,13 @@
 ; CHECK-NEXT: Strip Unused Function Prototypes
 ; CHECK-NEXT: Dead Global Elimination
 ; CHECK-NEXT: Merge Duplicate Global Constants
+; CHECK-NEXT: Call Graph Profile
+; CHECK-NEXT:   FunctionPass Manager
+; CHECK-NEXT: Dominator Tree Construction
+; CHECK-NEXT: Natural Loop Information
+; CHECK-NEXT: Post-Dominator Tree Construction
+; CHECK-NEXT: Branch Probability Analysis
+; CHECK-NEXT: Block Frequency Analysis
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   Dominator Tree Construction
 ; CHECK-NEXT:   Natural Loop Information
Index: llvm/test/Other/opt-O3-pipeline.ll
===
--- llvm/test/Other/opt-O3-pipeline.ll
+++ llvm/test/Other/opt-O3-pipeline.ll
@@ -285,6 +285,13 @@
 ; CHECK-NEXT: Strip Unused Function Prototypes
 ; CHECK-NEXT: Dead Global Elimination
 ; CHECK-NEXT: Merge Duplicate Global Constants
+; CHECK-NEXT: Call Graph Profile
+; CHECK-NEXT:   FunctionPass Manager
+; CHECK-NEXT: Dominator Tree Construction
+; CHECK-NEXT: Natural Loop Information
+; CHECK-NEXT: Post-Dominator Tree Construction
+; CHECK-NEXT: Branch Probability Analysis
+; CHECK-NEXT: Block Frequency Analysis
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   Dominator Tree Construction
 ; CHECK-NEXT:   Natural Loop Information
Index: llvm/test/Other/opt-O2-pipeline.ll
===
--- llvm/test/Other/opt-O2-pipeline.ll
+++ llvm/test/Other/opt-O2-pipeline.ll
@@ -280,6 +280,13 @@
 ; CHECK-NEXT: Strip Unused Function Prototypes
 ; CHECK-NEXT: Dead Global Elimination
 ; CHECK-NEXT: Merge Duplicate Global Constants
+; CHECK-NEXT: Call Graph Profile
+; CHECK-NEXT:   FunctionPass Manager
+; CHECK-NEXT: Dominator Tree Construction
+; CHECK-NEXT: Natural Loop Information
+; CHECK-NEXT: Post-Dominator Tree Construction
+; CHECK-NEXT: Branch Probability Analysis
+; CHECK-NEXT: Block Frequency Analysis
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   Dominator Tree Construction
 ; CHECK-NEXT:   Natural Loop Information
Index: llvm/test/Instrumentation/cgprofile.ll
===
--- llvm/test/Instrumentation/cgprofile.ll
+++ llvm/test/Instrumentation/cgprofile.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -passes cg-profile -S | FileCheck %s
+; RUN: opt < %s -cg-profile -S | FileCheck %s
 
 declare void @b()
 
Index: llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
@@ -276,6 +276,13 @@
 ; GCN-O1-NEXT:   Warn about non-applied transformations
 ; GCN-O1-NEXT:   Alignment from assumptions
 ; GCN-O1-NEXT: Strip Unused Function Prototypes
+; GCN-O1-NEXT: Call Graph Profile
+; GCN-O1-NEXT:   FunctionPass Manager
+; GCN-O1-NEXT: 

[PATCH] D77598: Integral template argument suffix and cast printing

2020-07-05 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/test/SemaTemplate/matrix-type.cpp:77
 void use_matrix_3(matrix ) {}
-// expected-note@-1 {{candidate template ignored: deduced type 'matrix<[...], 
5UL - 2, 5UL>' of 1st parameter does not match adjusted type 'matrix<[...], 5, 
5>' of argument [with T = unsigned int, R = 5]}}
+// expected-note@-1 {{candidate template ignored: deduced type 'matrix<[...], 
5UL - 2, 5UL>' of 1st parameter does not match adjusted type 'matrix<[...], 5, 
5>' of argument [with T = unsigned int, R = 5UL]}}
 

Re clarifying Richard's comment: My initial reaction is that every diff in this 
file is unwanted. The old output was unambiguous, despite lacking suffixes.

However, consider the following silly error message from Clang trunk. This is 
what we hope to //avoid// after this patch, I assume:
```
// https://godbolt.org/z/6nGmkE
template struct S {};
template<> struct S<1> { using type = int; };
S<1L>::type t;

error: no type named 'type' in 'S<1>'; did you mean 'S<1>::type'?
```



Comment at: clang/test/SemaTemplate/temp_arg_nontype.cpp:280
+  template <> struct enable_if_unsigned_int<1> { typedef int type; }; // 
expected-note{{'enable_if_unsigned_int<1>::type' declared here}}
+  void test_unsigned_int() { enable_if_unsigned_int<2>::type i; } // 
expected-error{{enable_if_unsigned_int<2U>'; did you mean 
'enable_if_unsigned_int<1>::type'?}}
+

Re clarifying Richard's comment: I believe that Richard wants this to produce 
`enable_if_unsigned_int<2>'; did you mean` instead of 
`enable_if_unsigned_int<2U>'; did you mean`.
It looks to me like Clang is producing the shorter version when it prints out 
the corrected type `enable_if_unsigned_int<1>`, but not when it prints out the 
faulty type `enable_if_unsigned_int<2U>`. So maybe you addressed his comment in 
one place but not the other?


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

https://reviews.llvm.org/D77598



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


[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Some clang-tidy checkers, e.g. llvm-include-order can emit diagnostics
at this callback (as mentioned in the comments).

Clangd was resetting diag consumer to IgnoreDiags before sending EOF, hence we
were unable to emit diagnostics for such checkers.

This patch changes the order of that reset and preprocosser event to make sure
we emit that diag.

Fixes https://github.com/clangd/clangd/issues/314.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83178

Files:
  clang-tools-extra/clangd/ParsedAST.cpp


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -428,15 +428,15 @@
 CTFinder.matchAST(Clang->getASTContext());
   }
 
+  // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
+  // However Action->EndSourceFile() would destroy the ASTContext!
+  // So just inform the preprocessor of EOF, while keeping everything alive.
+  Clang->getPreprocessor().EndSourceFile();
   // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
   // has a longer lifetime.
   Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
   // CompilerInstance won't run this callback, do it directly.
   ASTDiags.EndSourceFile();
-  // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
-  // However Action->EndSourceFile() would destroy the ASTContext!
-  // So just inform the preprocessor of EOF, while keeping everything alive.
-  Clang->getPreprocessor().EndSourceFile();
 
   std::vector Diags = CompilerInvocationDiags;
   // Add diagnostics from the preamble, if any.


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -428,15 +428,15 @@
 CTFinder.matchAST(Clang->getASTContext());
   }
 
+  // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
+  // However Action->EndSourceFile() would destroy the ASTContext!
+  // So just inform the preprocessor of EOF, while keeping everything alive.
+  Clang->getPreprocessor().EndSourceFile();
   // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
   // has a longer lifetime.
   Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
   // CompilerInstance won't run this callback, do it directly.
   ASTDiags.EndSourceFile();
-  // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
-  // However Action->EndSourceFile() would destroy the ASTContext!
-  // So just inform the preprocessor of EOF, while keeping everything alive.
-  Clang->getPreprocessor().EndSourceFile();
 
   std::vector Diags = CompilerInvocationDiags;
   // Add diagnostics from the preamble, if any.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] b0b5162 - [Driver] Pass -gno-column-info instead of -dwarf-column-info

2020-07-05 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-07-05T11:50:38-07:00
New Revision: b0b5162fc23c55906a461366f8acef2431d951c5

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

LOG: [Driver] Pass -gno-column-info instead of -dwarf-column-info

Making -g[no-]column-info opt out reduces the length of a typical CC1 command 
line.
Additionally, in a non-debug compile, we won't see -dwarf-column-info.

Added: 


Modified: 
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/code-coverage.c
clang/test/CodeGen/linetable-endscope.c
clang/test/CodeGen/opt-record-MIR.c
clang/test/CodeGen/opt-record.c
clang/test/CodeGenCXX/PR20038.cpp
clang/test/CodeGenCXX/debug-info-inheriting-constructor.cpp
clang/test/CodeGenCXX/debug-info-inlined.cpp
clang/test/CodeGenCXX/debug-info-lambda.cpp
clang/test/CodeGenCXX/debug-info-line-if.cpp
clang/test/CodeGenCXX/debug-info-member-call.cpp
clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
clang/test/CodeGenCXX/debug-info-scope.cpp
clang/test/CodeGenCXX/linetable-cleanup.cpp
clang/test/CodeGenCXX/linetable-eh.cpp
clang/test/CodeGenCXX/linetable-fnbegin.cpp
clang/test/CodeGenCXX/lpad-linetable.cpp
clang/test/CodeGenObjC/arc-linetable-autorelease.m
clang/test/CodeGenObjC/arc-linetable.m
clang/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
clang/test/CodeGenOpenCL/func-call-dbg-loc.cl
clang/test/Driver/cl-options.c
clang/test/Driver/codeview-column-info.c
clang/test/Driver/debug-options.c
clang/test/Frontend/optimization-remark-line-directive.c
clang/test/OpenMP/for_codegen.cpp
clang/test/OpenMP/parallel_codegen.cpp
clang/test/OpenMP/parallel_for_codegen.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/CC1Options.td 
b/clang/include/clang/Driver/CC1Options.td
index 75f8aa610d04..5a7077ce9e46 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -237,8 +237,6 @@ def disable_O0_optnone : Flag<["-"], "disable-O0-optnone">,
   HelpText<"Disable adding the optnone attribute to functions at O0">;
 def disable_red_zone : Flag<["-"], "disable-red-zone">,
   HelpText<"Do not emit code that uses the red zone.">;
-def dwarf_column_info : Flag<["-"], "dwarf-column-info">,
-  HelpText<"Turn on column location information.">;
 def dwarf_ext_refs : Flag<["-"], "dwarf-ext-refs">,
   HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index e636ee62d202..50d18343f7d4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2030,7 +2030,7 @@ def : Flag<["-"], "gno-record-gcc-switches">, 
Alias;
 def gstrict_dwarf : Flag<["-"], "gstrict-dwarf">, Group;
 def gno_strict_dwarf : Flag<["-"], "gno-strict-dwarf">, Group;
 def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, 
Flags<[CoreOption]>;
-def gno_column_info : Flag<["-"], "gno-column-info">, Group, 
Flags<[CoreOption]>;
+def gno_column_info : Flag<["-"], "gno-column-info">, Group, 
Flags<[CoreOption, CC1Option]>;
 def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
 def gsplit_dwarf_EQ : Joined<["-"], "gsplit-dwarf=">, Group,
   HelpText<"Set DWARF fission mode to either 'split' or 'single'">,

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a0d638e7366d..ed365c608387 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3748,10 +3748,9 @@ static void RenderDebugOptions(const ToolChain , 
const Driver ,
   // not to include any column info.
   if (const Arg *A = Args.getLastArg(options::OPT_gcolumn_info))
 (void)checkDebugInfoOption(A, Args, D, TC);
-  if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/!EmitCodeView &&
-   DebuggerTuning != llvm::DebuggerKind::SCE))
-CmdArgs.push_back("-dwarf-column-info");
+  if (!Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
+!EmitCodeView && DebuggerTuning != 
llvm::DebuggerKind::SCE))
+CmdArgs.push_back("-gno-column-info");
 
   // FIXME: Move backend command line options to the module.
   // If -gline-tables-only or -gline-directives-only is the last option it 
wins.

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 821e40c5ea8d..f58854cd9e08 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ 

[PATCH] D77598: Integral template argument suffix and cast printing

2020-07-05 Thread Pratyush Das via Phabricator via cfe-commits
reikdas added a comment.

In D77598#2111606 , @rsmith wrote:

> In D77598#2064193 , @rsmith wrote:
>
> > In D77598#1990197 , @rsmith wrote:
> >
> > > Is is feasible to check whether the corresponding template parameter 
> > > either has a deduced type or is a parameter of a function template? If 
> > > not, we don't need to clarify the type of the template argument and could 
> > > leave off the suffix to shorten the printed argument.
> >
> >
> > Ping on this comment. If possible, I'd prefer for us to not produce 
> > suffixes and casts in the case where there's no need to clarify the type of 
> > the template argument. I would imagine we can pass in a flag into the 
> > printing routine to indicate whether it needs to produce an expression with 
> > the right type, or merely one convertible to the right type.
>
>
> This comment has not been addressed.


@rsmith Can you elaborate a little on what you meant in your comment? I thought 
you meant that we should not print the suffixes and casts for the type if it 
was deduced. Did you instead mean that we should //only// print the cast and 
the suffix if the type is a deduced type?


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

https://reviews.llvm.org/D77598



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


[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

2020-07-05 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 275550.
Tyker added a comment.

fixed the issue with multiple align in a single assume.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71739

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/align_value.cpp
  clang/test/CodeGen/alloc-align-attr.c
  clang/test/CodeGen/assume-aligned-and-alloc-align-attributes.c
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align.c
  clang/test/CodeGen/builtin-assume-aligned.c
  
clang/test/CodeGen/catch-alignment-assumption-attribute-align_value-on-lvalue.cpp
  
clang/test/CodeGen/catch-alignment-assumption-attribute-align_value-on-paramvar.cpp
  
clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp
  
clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function.cpp
  
clang/test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function-two-params.cpp
  
clang/test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params-variable.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-two-params.cpp
  clang/test/CodeGen/catch-alignment-assumption-openmp.cpp
  clang/test/CodeGen/non-power-of-2-alignment-assumptions.c
  clang/test/OpenMP/simd_codegen.cpp
  clang/test/OpenMP/simd_metadata.c
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h
  llvm/lib/Analysis/AssumeBundleQueries.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
  llvm/test/Transforms/AlignmentFromAssumptions/simple.ll
  llvm/test/Transforms/AlignmentFromAssumptions/simple32.ll
  llvm/test/Transforms/Inline/align.ll
  llvm/test/Transforms/InstCombine/assume.ll
  llvm/test/Transforms/PhaseOrdering/inlining-alignment-assumptions.ll
  llvm/test/Verifier/assume-bundles.ll
  llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp

Index: llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
===
--- llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
+++ llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
@@ -546,3 +546,41 @@
   ASSERT_EQ(AR[0].Index, 1u);
   ASSERT_EQ(AR[0].Assume, &*First);
 }
+
+TEST(AssumeQueryAPI, Alignment) {
+  LLVMContext C;
+  SMDiagnostic Err;
+  std::unique_ptr Mod = parseAssemblyString(
+  "declare void @llvm.assume(i1)\n"
+  "define void @test(i32* %P, i32* %P1, i32* %P2, i32 %I3, i1 %B) {\n"
+  "call void @llvm.assume(i1 true) [\"align\"(i32* %P, i32 8, i32 %I3)]\n"
+  "call void @llvm.assume(i1 true) [\"align\"(i32* %P1, i32 %I3, i32 "
+  "%I3)]\n"
+  "call void @llvm.assume(i1 true) [\"align\"(i32* %P2, i32 16, i32 8)]\n"
+  "ret void\n}\n",
+  Err, C);
+  if (!Mod)
+Err.print("AssumeQueryAPI", errs());
+
+  Function *F = Mod->getFunction("test");
+  BasicBlock::iterator Start = F->begin()->begin();
+  IntrinsicInst *II;
+  RetainedKnowledge RK;
+  II = cast(&*Start);
+  RK = getKnowledgeFromBundle(*II, II->bundle_op_info_begin()[0]);
+  ASSERT_EQ(RK.AttrKind, Attribute::Alignment);
+  ASSERT_EQ(RK.WasOn, F->getArg(0));
+  ASSERT_EQ(RK.ArgValue, 1u);
+  Start++;
+  II = cast(&*Start);
+  RK = getKnowledgeFromBundle(*II, II->bundle_op_info_begin()[0]);
+  ASSERT_EQ(RK.AttrKind, Attribute::Alignment);
+  ASSERT_EQ(RK.WasOn, F->getArg(1));
+  ASSERT_EQ(RK.ArgValue, 1u);
+  Start++;
+  II = cast(&*Start);
+  RK = getKnowledgeFromBundle(*II, II->bundle_op_info_begin()[0]);
+  ASSERT_EQ(RK.AttrKind, Attribute::Alignment);
+  ASSERT_EQ(RK.WasOn, F->getArg(2));
+  ASSERT_EQ(RK.ArgValue, 8u);
+}
Index: llvm/test/Verifier/assume-bundles.ll
===
--- llvm/test/Verifier/assume-bundles.ll
+++ llvm/test/Verifier/assume-bundles.ll
@@ -1,3 +1,4 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: not opt -verify < %s 2>&1 | FileCheck %s
 
 declare void @llvm.assume(i1)
@@ -6,14 +7,21 @@
 ; CHECK: tags must be valid attribute names
   call void @llvm.assume(i1 true) ["adazdazd"()]
 ; CHECK: the second argument should be a constant integral value
-  call void @llvm.assume(i1 true) ["align"(i32* %P, i32 %P1)]
+  call void @llvm.assume(i1 true) ["dereferenceable"(i32* %P, i32 %P1)]
 ; CHECK: to many arguments
-  call void @llvm.assume(i1 true) ["align"(i32* %P, i32 8, i32 8)]
+  call void @llvm.assume(i1 true) ["dereferenceable"(i32* %P, i32 8, i32 8)]
 ; CHECK: this attribute should have 2 arguments
-  call void @llvm.assume(i1 

[clang-tools-extra] edba286 - [clangd] Fix stack-use-after-scope

2020-07-05 Thread Benjamin Kramer via cfe-commits

Author: Benjamin Kramer
Date: 2020-07-05T17:09:08+02:00
New Revision: edba2864a7a86a97276c555d02276712e45d60fc

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

LOG: [clangd] Fix stack-use-after-scope

Found by asan.

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 6ac2f67d55b3..5d99104dadaf 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -750,10 +750,10 @@ Context ClangdServer::createProcessingContext(PathRef 
File) const {
 return Context::current().clone();
 
   config::Params Params;
+  llvm::SmallString<256> PosixPath;
   if (!File.empty()) {
 assert(llvm::sys::path::is_absolute(File));
-llvm::SmallString<256> PosixPath = File;
-llvm::sys::path::native(PosixPath, llvm::sys::path::Style::posix);
+llvm::sys::path::native(File, PosixPath, llvm::sys::path::Style::posix);
 Params.Path = PosixPath.str();
   }
 



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


[PATCH] D83176: [OpenMPIRBuilder][Fix] Move llvm::omp::types to OpenMPIRBuilder.

2020-07-05 Thread Stefan Stipanovic via Phabricator via cfe-commits
sstefan1 updated this revision to Diff 275549.
sstefan1 added a comment.

small fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83176

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -19,7 +19,6 @@
 
 using namespace llvm;
 using namespace omp;
-using namespace types;
 
 namespace {
 
@@ -50,7 +49,6 @@
   void TearDown() override {
 BB = nullptr;
 M.reset();
-uninitializeTypes();
   }
 
   LLVMContext Ctx;
Index: llvm/lib/Transforms/IPO/OpenMPOpt.cpp
===
--- llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -29,7 +29,6 @@
 
 using namespace llvm;
 using namespace omp;
-using namespace types;
 
 #define DEBUG_TYPE "openmp-opt"
 
@@ -224,11 +223,11 @@
   ICV.InitValue = nullptr; \
   break;   \
 case ICV_ZERO: \
-  ICV.InitValue =  \
-  ConstantInt::get(Type::getInt32Ty(Int32->getContext()), 0);  \
+  ICV.InitValue = ConstantInt::get(\
+  Type::getInt32Ty(OMPBuilder.Int32->getContext()), 0);\
   break;   \
 case ICV_FALSE:\
-  ICV.InitValue = ConstantInt::getFalse(Int1->getContext());   \
+  ICV.InitValue = ConstantInt::getFalse(OMPBuilder.Int1->getContext());\
   break;   \
 case ICV_LAST: \
   break;   \
@@ -293,11 +292,26 @@
 
 Module  = *((*ModuleSlice.begin())->getParent());
 
+// Helper macros for handling __VA_ARGS__ in OMP_RTL
+#define OMP_TYPE(VarName, ...) Type *VarName = OMPBuilder.VarName;
+
+#define OMP_ARRAY_TYPE(VarName, ...)   \
+  ArrayType *VarName##Ty = OMPBuilder.VarName##Ty; \
+  PointerType *VarName##PtrTy = OMPBuilder.VarName##PtrTy;
+
+#define OMP_FUNCTION_TYPE(VarName, ...)\
+  FunctionType *VarName = OMPBuilder.VarName;  \
+  PointerType *VarName##Ptr = OMPBuilder.VarName##Ptr;
+
+#define OMP_STRUCT_TYPE(VarName, ...)  \
+  StructType *VarName = OMPBuilder.VarName;\
+  PointerType *VarName##Ptr = OMPBuilder.VarName##Ptr;
+
 #define OMP_RTL(_Enum, _Name, _IsVarArg, _ReturnType, ...) \
   {\
 SmallVector ArgsTypes({__VA_ARGS__});   \
 Function *F = M.getFunction(_Name);\
-if (declMatchesRTFTypes(F, _ReturnType, ArgsTypes)) {  \
+if (declMatchesRTFTypes(F, OMPBuilder._ReturnType, ArgsTypes)) {   \
   auto  = RFIs[_Enum]; \
   RFI.Kind = _Enum;\
   RFI.Name = _Name;\
@@ -553,11 +567,11 @@
"Unexpected replacement value!");
 
 // TODO: Use dominance to find a good position instead.
-auto CanBeMoved = [](CallBase ) {
+auto CanBeMoved = [this](CallBase ) {
   unsigned NumArgs = CB.getNumArgOperands();
   if (NumArgs == 0)
 return true;
-  if (CB.getArgOperand(0)->getType() != IdentPtr)
+  if (CB.getArgOperand(0)->getType() != OMPInfoCache.OMPBuilder.IdentPtr)
 return false;
   for (unsigned u = 1; u < NumArgs; ++u)
 if (isa(CB.getArgOperand(u)))
@@ -592,7 +606,7 @@
 // existing and used by one of the calls, or created 

[PATCH] D83176: [OpenMPIRBuilder][Fix] Move llvm::omp::types to OpenMPIRBuilder.

2020-07-05 Thread Stefan Stipanovic via Phabricator via cfe-commits
sstefan1 marked an inline comment as done.
sstefan1 added a subscriber: hoyFB.
sstefan1 added a comment.

Since this is not a small change, I think it would be good if @hoyFB could test 
if this resolves the issue.

There is one small issue though:




Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:313
   {
\
 SmallVector ArgsTypes({__VA_ARGS__});   
\
 Function *F = M.getFunction(_Name);
\

I wasn't sure how to handle `__VA_ARGS__` here, since we would need 
`OMPBuilder` in front of every type. 
That is why helper macros above exist. The problem with this is that this 
creates some unused variables in `OpenMPOpt`.

Not sure if `-Wno-unused-variable` would be a good thing to do temporarily? Is 
there another way to handle `__VA_ARGS__` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83176



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


[PATCH] D83176: [OpenMPIRBuilder][Fix] Move llvm::omp::types to OpenMPIRBuilder.

2020-07-05 Thread Stefan Stipanovic via Phabricator via cfe-commits
sstefan1 created this revision.
sstefan1 added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, aaron.ballman, dexonsmith, 
guansong, hiraditya, yaxunl.
Herald added projects: clang, LLVM.

D82193  exposed a problem with global type 
definitions in
`OMPConstants.h`. This causes a race when running in thinLTO mode.
Types now live inside of OpenMPIRBuilder to prevent this from happening.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83176

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -19,7 +19,6 @@
 
 using namespace llvm;
 using namespace omp;
-using namespace types;
 
 namespace {
 
@@ -50,7 +49,6 @@
   void TearDown() override {
 BB = nullptr;
 M.reset();
-uninitializeTypes();
   }
 
   LLVMContext Ctx;
Index: llvm/lib/Transforms/IPO/OpenMPOpt.cpp
===
--- llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -29,7 +29,6 @@
 
 using namespace llvm;
 using namespace omp;
-using namespace types;
 
 #define DEBUG_TYPE "openmp-opt"
 
@@ -224,11 +223,11 @@
   ICV.InitValue = nullptr; \
   break;   \
 case ICV_ZERO: \
-  ICV.InitValue =  \
-  ConstantInt::get(Type::getInt32Ty(Int32->getContext()), 0);  \
+  ICV.InitValue = ConstantInt::get(\
+  Type::getInt32Ty(OMPBuilder.Int32->getContext()), 0);\
   break;   \
 case ICV_FALSE:\
-  ICV.InitValue = ConstantInt::getFalse(Int1->getContext());   \
+  ICV.InitValue = ConstantInt::getFalse(OMPBuilder.Int1->getContext());\
   break;   \
 case ICV_LAST: \
   break;   \
@@ -293,11 +292,27 @@
 
 Module  = *((*ModuleSlice.begin())->getParent());
 
+// Helper macros for handling __VA_ARGS__ in OMP_RTL
+#define OMP_TYPE(VarName, ...) Type *VarName = OMPBuilder.VarName;
+
+#define OMP_ARRAY_TYPE(VarName, ...)   \
+  ArrayType *VarName##Ty = OMPBuilder.VarName##Ty; \
+  PointerType *VarName##PtrTy = OMPBuilder.VarName##PtrTy;
+
+#define OMP_FUNCTION_TYPE(VarName, ...)\
+  FunctionType *VarName = OMPBuilder.VarName;  \
+  PointerType *VarName##Ptr = OMPBuilder.VarName##Ptr;
+
+#define OMP_STRUCT_TYPE(VarName, ...)  \
+  StructType *VarName = OMPBuilder.VarName;\
+  PointerType *VarName##Ptr = OMPBuilder.VarName##Ptr;
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+
 #define OMP_RTL(_Enum, _Name, _IsVarArg, _ReturnType, ...) \
   {\
 SmallVector ArgsTypes({__VA_ARGS__});   \
 Function *F = M.getFunction(_Name);\
-if (declMatchesRTFTypes(F, _ReturnType, ArgsTypes)) {  \
+if (declMatchesRTFTypes(F, OMPBuilder._ReturnType, ArgsTypes)) {   \
   auto  = RFIs[_Enum]; \
   RFI.Kind = _Enum;\
   RFI.Name = _Name;\
@@ -553,11 +568,11 @@
"Unexpected replacement value!");
 
 // TODO: Use dominance to find a good position instead.
-auto CanBeMoved = [](CallBase ) {
+auto CanBeMoved = [this](CallBase ) {
   unsigned NumArgs = CB.getNumArgOperands();
   if (NumArgs == 

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

2020-07-05 Thread Thorsten via Phabricator via cfe-commits
tschuett added a comment.

You could try:

  clangxx_host -Xclang -fdump-record-layouts %p/Inputs/layout.cpp 


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

https://reviews.llvm.org/D83008



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


[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

2020-07-05 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

Thanks for tracking this down, this is a really nasty bug...

The fix itself is obviously fine, but I think I'm out of the loop regarding the 
testing strategy. We talked about adding a Clang test for this with the help of 
this layout overwrite JSON file. I assume that extending this to cover virtual 
bases turned out to be more complicated than expected? FWIW, I'm not 
necessarily the biggest fan of this Clang test option so I would be fine if we 
leave it as-is.

I think having an LLDB test is a good idea, but it's not clear why it's a Shell 
test. If I understand correctly this test requires running on arm64e (so, a 
remote test target), but from what I remember all the remote platform support 
is only in dotest.py? Also pretty much all other expression evaluation tests 
and the utilities for that are in the Python/API test infrastructure, so it's a 
bit out of place.

Also I think the test can be in general much simpler than an arm64-specific 
test. We get all base class offsets wrong in LLDB, so we can just write a 
simple test where you change the layout of some structs in a way that it 
doesn't fit the default layout. E.g., just putting `alignas` on a base class 
and then reading fields should be enough to trigger the same bug.


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

https://reviews.llvm.org/D83008



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


[clang-tools-extra] e8158bf - [NFC] Clean up braces and anon namespace

2020-07-05 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-07-05T11:16:40+01:00
New Revision: e8158bf0e770e7a4cf73df91552af1e156a3ea17

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

LOG: [NFC] Clean up braces and anon namespace

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
index 98ca34eb620c..9ad6fb737ec9 100644
--- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,7 +19,6 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
-namespace {
 static const char ReturnStr[] = "return";
 static const char ContinueStr[] = "continue";
 static const char BreakStr[] = "break";
@@ -28,44 +27,40 @@ static const char WarningMessage[] = "do not use 'else' 
after '%0'";
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 static const char WarnOnConditionVariablesStr[] = "WarnOnConditionVariables";
 
-const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+static const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
   if (!Node)
 return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
-if (DeclRef->getDecl()->getID() == DeclIdentifier) {
+if (DeclRef->getDecl()->getID() == DeclIdentifier)
   return DeclRef;
-}
   } else {
 for (const Stmt *ChildNode : Node->children()) {
-  if (const DeclRefExpr *Result = findUsage(ChildNode, DeclIdentifier)) {
+  if (const DeclRefExpr *Result = findUsage(ChildNode, DeclIdentifier))
 return Result;
-  }
 }
   }
   return nullptr;
 }
 
-const DeclRefExpr *
+static const DeclRefExpr *
 findUsageRange(const Stmt *Node,
-   const llvm::iterator_range ) {
+   const llvm::ArrayRef ) {
   if (!Node)
 return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
-if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
+if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID()))
   return DeclRef;
-}
   } else {
 for (const Stmt *ChildNode : Node->children()) {
   if (const DeclRefExpr *Result =
-  findUsageRange(ChildNode, DeclIdentifiers)) {
+  findUsageRange(ChildNode, DeclIdentifiers))
 return Result;
-  }
 }
   }
   return nullptr;
 }
 
-const DeclRefExpr *checkInitDeclUsageInElse(const IfStmt *If) {
+static const DeclRefExpr *checkInitDeclUsageInElse(const IfStmt *If) {
   const auto *InitDeclStmt = dyn_cast_or_null(If->getInit());
   if (!InitDeclStmt)
 return nullptr;
@@ -82,25 +77,23 @@ const DeclRefExpr *checkInitDeclUsageInElse(const IfStmt 
*If) {
   return findUsageRange(If->getElse(), DeclIdentifiers);
 }
 
-const DeclRefExpr *checkConditionVarUsageInElse(const IfStmt *If) {
-  const VarDecl *CondVar = If->getConditionVariable();
-  return CondVar != nullptr ? findUsage(If->getElse(), CondVar->getID())
-: nullptr;
+static const DeclRefExpr *checkConditionVarUsageInElse(const IfStmt *If) {
+  if (const VarDecl *CondVar = If->getConditionVariable())
+return findUsage(If->getElse(), CondVar->getID());
+  return nullptr;
 }
 
-bool containsDeclInScope(const Stmt *Node) {
-  if (isa(Node)) {
+static bool containsDeclInScope(const Stmt *Node) {
+  if (isa(Node))
 return true;
-  }
-  if (const auto *Compound = dyn_cast(Node)) {
+  if (const auto *Compound = dyn_cast(Node))
 return llvm::any_of(Compound->body(), [](const Stmt *SubNode) {
   return isa(SubNode);
 });
-  }
   return false;
 }
 
-void removeElseAndBrackets(DiagnosticBuilder , ASTContext ,
+static void removeElseAndBrackets(DiagnosticBuilder , ASTContext ,
const Stmt *Else, SourceLocation ElseLoc) {
   auto Remap = [&](SourceLocation Loc) {
 return Context.getSourceManager().getExpansionLoc(Loc);
@@ -134,7 +127,6 @@ void removeElseAndBrackets(DiagnosticBuilder , 
ASTContext ,
 SourceRange(ElseExpandedLoc, EndLoc), Repl);
   }
 }
-} // namespace
 
 ElseAfterReturnCheck::ElseAfterReturnCheck(StringRef Name,
ClangTidyContext *Context)



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


[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

2020-07-05 Thread Vaibhav Garg via Phabricator via cfe-commits
gargvaibhav64 created this revision.
gargvaibhav64 added reviewers: rsmith, v.g.vassilev.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This commit attaches teaches ASTDeclReader::attachPreviousDecl to successfully 
merge two Decl's when one contains an inheritable attribute like the 
MSInheritanceAttr. Usually, attributes that are needed to present along the 
redeclaration chain are attached during ASTReading from 
ASTDeclReader::attachPreviousDecl, but no such thing is done for inheritable 
attributes. Currently, only the logic for merging MSInheritanceAttr is provided.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83174

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/inherit-attribute/a.h
  clang/test/Modules/Inputs/inherit-attribute/b.h
  clang/test/Modules/Inputs/inherit-attribute/c.h
  clang/test/Modules/Inputs/inherit-attribute/module.modulemap
  clang/test/Modules/inherit-attribute.cpp

Index: clang/test/Modules/inherit-attribute.cpp
===
--- /dev/null
+++ clang/test/Modules/inherit-attribute.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -ast-dump -I%S/Inputs/inherit-attribute -fmodules-cache-path=%t -fimplicit-module-maps -verify %s -fmodules-local-submodule-visibility
+
+#include "b.h"
+#include "c.h"
+
+class Foo;
+
+Foo f;
+// expected-no-diagnostics
\ No newline at end of file
Index: clang/test/Modules/Inputs/inherit-attribute/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/module.modulemap
@@ -0,0 +1,3 @@
+module "b" { header "b.h" }
+
+module "c" { header "c.h" }
\ No newline at end of file
Index: clang/test/Modules/Inputs/inherit-attribute/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/c.h
@@ -0,0 +1,7 @@
+#include "a.h"
+
+class Foo;
+class C {
+public:
+  C();
+};
\ No newline at end of file
Index: clang/test/Modules/Inputs/inherit-attribute/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/b.h
@@ -0,0 +1,12 @@
+#include "a.h"
+
+class Foo;
+
+void bar() {
+  ::step;
+}
+
+class B {
+public:
+  B();
+};
\ No newline at end of file
Index: clang/test/Modules/Inputs/inherit-attribute/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/a.h
@@ -0,0 +1,10 @@
+#ifndef FOO
+#define FOO
+
+class Foo {
+public:
+  void step(int v);
+  Foo();
+};
+
+#endif
\ No newline at end of file
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -281,6 +281,8 @@
 static Decl *getMostRecentDeclImpl(...);
 static Decl *getMostRecentDecl(Decl *D);
 
+static void mergeInheritableAttributes(Decl *D, Decl *Previous);
+
 template 
 static void attachPreviousDeclImpl(ASTReader ,
Redeclarable *D, Decl *Previous,
@@ -3531,6 +3533,18 @@
   return ASTDeclReader::getMostRecentDecl(D->getCanonicalDecl());
 }
 
+void ASTDeclReader::mergeInheritableAttributes(Decl *D, Decl *Previous) {
+  if (Previous->hasAttr() &&
+  !D->hasAttr()) {
+Attr *IA = Previous->getAttr();
+D->addAttr(IA);
+  } else if (!Previous->hasAttr() &&
+ D->hasAttr()) {
+Attr *IA = D->getAttr();
+Previous->addAttr(IA);
+  }
+}
+
 template
 void ASTDeclReader::attachPreviousDeclImpl(ASTReader ,
Redeclarable *D,
@@ -3689,6 +3703,10 @@
   if (auto *TD = dyn_cast(D))
 inheritDefaultTemplateArguments(Reader.getContext(),
 cast(Previous), TD);
+
+  // If any of the declaration in the chain contains an Inheritable attribute,
+  // it needs to be added to all the declarations in the redeclarable chain.
+  mergeInheritableAttributes(D, Previous);
 }
 
 template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D83173: [VE] Correct stack alignment

2020-07-05 Thread Kazushi Marukawa via Phabricator via cfe-commits
kaz7 created this revision.
kaz7 added reviewers: simoll, k-ishizaka.
kaz7 added projects: LLVM, VE.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added a project: clang.

Change stack alignment from 64 bits to 128 bits to follow ABI correctly.
And add a regression test for datalayout.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83173

Files:
  clang/lib/Basic/Targets/VE.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/VE/VETargetMachine.cpp


Index: llvm/lib/Target/VE/VETargetMachine.cpp
===
--- llvm/lib/Target/VE/VETargetMachine.cpp
+++ llvm/lib/Target/VE/VETargetMachine.cpp
@@ -41,8 +41,8 @@
   // VE supports 32 bit and 64 bits integer on registers
   Ret += "-n32:64";
 
-  // Stack alignment is 64 bits
-  Ret += "-S64";
+  // Stack alignment is 128 bits
+  Ret += "-S128";
 
   return Ret;
 }
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -250,3 +250,7 @@
 // RUN: %clang_cc1 -triple bpfeb -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=BPFEB
 // BPFEB: target datalayout = "E-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+
+// RUN: %clang_cc1 -triple ve -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=VE
+// VE: target datalayout = "e-m:e-i64:64-n32:64-S128"
Index: clang/lib/Basic/Targets/VE.h
===
--- clang/lib/Basic/Targets/VE.h
+++ clang/lib/Basic/Targets/VE.h
@@ -45,7 +45,7 @@
 WCharType = UnsignedInt;
 WIntType = UnsignedInt;
 UseZeroLengthBitfieldAlignment = true;
-resetDataLayout("e-m:e-i64:64-n32:64-S64");
+resetDataLayout("e-m:e-i64:64-n32:64-S128");
   }
 
   void getTargetDefines(const LangOptions ,


Index: llvm/lib/Target/VE/VETargetMachine.cpp
===
--- llvm/lib/Target/VE/VETargetMachine.cpp
+++ llvm/lib/Target/VE/VETargetMachine.cpp
@@ -41,8 +41,8 @@
   // VE supports 32 bit and 64 bits integer on registers
   Ret += "-n32:64";
 
-  // Stack alignment is 64 bits
-  Ret += "-S64";
+  // Stack alignment is 128 bits
+  Ret += "-S128";
 
   return Ret;
 }
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -250,3 +250,7 @@
 // RUN: %clang_cc1 -triple bpfeb -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=BPFEB
 // BPFEB: target datalayout = "E-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+
+// RUN: %clang_cc1 -triple ve -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=VE
+// VE: target datalayout = "e-m:e-i64:64-n32:64-S128"
Index: clang/lib/Basic/Targets/VE.h
===
--- clang/lib/Basic/Targets/VE.h
+++ clang/lib/Basic/Targets/VE.h
@@ -45,7 +45,7 @@
 WCharType = UnsignedInt;
 WIntType = UnsignedInt;
 UseZeroLengthBitfieldAlignment = true;
-resetDataLayout("e-m:e-i64:64-n32:64-S64");
+resetDataLayout("e-m:e-i64:64-n32:64-S128");
   }
 
   void getTargetDefines(const LangOptions ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] aed6a1b - Add tests for clang -fno-zero-initialized-in-bss and llc -nozero-initialized-in-bss

2020-07-05 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-07-04T23:26:57-07:00
New Revision: aed6a1b137dc17426e3da8b85e1f9966c8229c05

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

LOG: Add tests for clang -fno-zero-initialized-in-bss and llc 
-nozero-initialized-in-bss

And rename the CC1 option.

Added: 
clang/test/Driver/fzero-initialized-in-bss.c
llvm/test/CodeGen/X86/zero-initialized-in-bss.ll

Modified: 
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/CC1Options.td 
b/clang/include/clang/Driver/CC1Options.td
index 8729512454c3..75f8aa610d04 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -307,8 +307,6 @@ def mlimit_float_precision : Separate<["-"], 
"mlimit-float-precision">,
   HelpText<"Limit float precision to the given value">;
 def split_stacks : Flag<["-"], "split-stacks">,
   HelpText<"Try to use a split stack if possible.">;
-def mno_zero_initialized_in_bss : Flag<["-"], "mno-zero-initialized-in-bss">,
-  HelpText<"Do not put zero initialized data in the BSS">;
 def mregparm : Separate<["-"], "mregparm">,
   HelpText<"Limit the number of registers available for integer arguments">;
 def msmall_data_limit : Separate<["-"], "msmall-data-limit">,

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d030468514c3..e636ee62d202 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1589,7 +1589,6 @@ def fno_unwind_tables : Flag<["-"], "fno-unwind-tables">, 
Group;
 def fno_verbose_asm : Flag<["-"], "fno-verbose-asm">, Group, 
Flags<[CC1Option]>;
 def fno_working_directory : Flag<["-"], "fno-working-directory">, 
Group;
 def fno_wrapv : Flag<["-"], "fno-wrapv">, Group;
-def fno_zero_initialized_in_bss : Flag<["-"], "fno-zero-initialized-in-bss">, 
Group;
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group, Flags<[CC1Option]>,
   HelpText<"Synthesize retain and release calls for Objective-C pointers">;
 def fno_objc_arc : Flag<["-"], "fno-objc-arc">, Group;
@@ -1926,7 +1925,7 @@ def fwrapv : Flag<["-"], "fwrapv">, Group, 
Flags<[CC1Option]>,
   HelpText<"Treat signed integer overflow as two's complement">;
 def fwritable_strings : Flag<["-"], "fwritable-strings">, Group, 
Flags<[CC1Option]>,
   HelpText<"Store string literals as writable data">;
-def fzero_initialized_in_bss : Flag<["-"], "fzero-initialized-in-bss">, 
Group;
+defm zero_initialized_in_bss : OptOutFFlag<"zero-initialized-in-bss", "", 
"Don't place zero initialized data in BSS">;
 defm function_sections : OptInFFlag<"function-sections", "Place each function 
in its own section">;
 def fbasic_block_sections_EQ : Joined<["-"], "fbasic-block-sections=">, 
Group,
   Flags<[CC1Option, CC1AsOption]>,

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index e6dd6ce0a95e..a0d638e7366d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4556,8 +4556,8 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
   CmdArgs.push_back(FPKeepKindStr);
 
   if (!Args.hasFlag(options::OPT_fzero_initialized_in_bss,
-options::OPT_fno_zero_initialized_in_bss))
-CmdArgs.push_back("-mno-zero-initialized-in-bss");
+options::OPT_fno_zero_initialized_in_bss, true))
+CmdArgs.push_back("-fno-zero-initialized-in-bss");
 
   bool OFastEnabled = isOptimizationLevelFast(Args);
   // If -Ofast is the optimization level, then -fstrict-aliasing should be

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index e12931a5a1b4..821e40c5ea8d 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -942,7 +942,7 @@ static bool ParseCodeGenArgs(CodeGenOptions , ArgList 
, InputKind IK,
   Opts.StrictFloatCastOverflow =
   !Args.hasArg(OPT_fno_strict_float_cast_overflow);
 
-  Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
+  Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_fno_zero_initialized_in_bss);
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, 
Diags);
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.SmallDataLimit =

diff  --git a/clang/test/Driver/fzero-initialized-in-bss.c 
b/clang/test/Driver/fzero-initialized-in-bss.c
new file mode 100644
index ..34a1dc9788cd
--- /dev/null
+++ b/clang/test/Driver/fzero-initialized-in-bss.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### %s -c -fzero-initialized-in-bss 2>&1 | FileCheck %s