[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

[Alexandre Rames via Phabricator via cfe-commits]

arames added a comment.

In D102943#2775099 , @dexonsmith 
wrote:

> In D102943#2774732 , @jroelofs 
> wrote:
>
>> why do module hashes need to be stable when cross-compiling?
>
> IIUC, the use case is cross-compiling when building the modules, which are 
> then sent to the target to use when compiling other things.

Yes, that's right.
In particular, we are talking about the hash used for for (prebuilt) implicit 
modules paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

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

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

[Alexandre Rames via Phabricator via cfe-commits]

arames added a comment.

In D102943#2775131 , @dexonsmith 
wrote:

> In D102943#2775115 , @pcc wrote:
>
>> Isn't the bug here that module hashing is using `hash_code`? So shouldn't 
>> the correct fix be to use a specific hashing algorithm for module hashes?
>
> Yeah, I tend to agree. I thought modules already used MD5, but maybe just for 
> the AST signature.

For reference, the issue I am trying to fix stems from the hash computed as 
part of CompilerInvocation::getModuleHash() 
.

Reading the doc around `llvm::hash_code`, my first reaction was indeed that 
modules may have to use a different hash.
I thought the current proposal was something to consider, because:

1. Practically, everything is done on `uint64_t`, so I would expect switching 
from `size_t` to `uint64_t` to have little to no impact on non-64bit targets.
2. The current modules code kind of already assumes `hash_code` is stable over 
different executions. Plain simple use-case with implicit modules would be 
severely deficient if the hash was not stable across executions. To be fair, 
the assumption is different from the one I am trying to fix of 32bit vs 64bit.

Alternatively, we could introduce something along the lines of 
`llvm::stable_hash_code` (other name suggestions welcome).
We can probably restructure the code with very few changes. (Really, it looks 
like we only need a configurable storage type, and allow skipping the execution 
seed part.)

Would you think this is a better approach ? Or do you have something else to 
suggest ?

Cheers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

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

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

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

dexonsmith added a subscriber: jansvoboda11.
dexonsmith added a comment.

In D102943#2775115 , @pcc wrote:

> Isn't the bug here that module hashing is using `hash_code`? So shouldn't the 
> correct fix be to use a specific hashing algorithm for module hashes?

Yeah, I tend to agree. I thought modules already used MD5, but maybe just for 
the AST signature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

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

[PATCH] D102633: [clang-scan-deps] Improvements to thread usage

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

dexonsmith added a subscriber: jansvoboda11.
dexonsmith added a comment.

In D102633#2773578 , @aganea wrote:

> In D102633#2769762 , @arphaman 
> wrote:
>
>> It might be good for @aganea to take a look as well.
>
> Thanks! I actually work with @saudi, I already took a look at the patch 
> before uploading. However I'm stil torn about running one of the workers on 
> the main thread. I fear that we could have random errors because of the stack 
> size of the "main" thread could be different from the stack size of the 
> "satellite" threads. There's 99.99% chance that this won't happen, but I'd 
> prefer that behavior to be explicit.
>
> We could have:
>
>   -j0: use all hardware threads
>   -j1: don't use multi-threading, run all the tasks on the main thread
>   -jN: use the specified number of threads
>
> The rationale is that we're using clang-scan-deps as-a-DLL in Fastbuild, to 
> extract the dependencies. Since Fastbuild already has its own thread pool 
> management, we call into clang-scan-deps with -j1 from different Fastbuild 
> threads (but keeping the `DependencyScanningService` alive between calls). It 
> would great if each call to clang-scan-deps wouldn't create a extra new 
> thread.
>
> Perhaps any of you would like to comment? +@mehdi_amini

Seems like if you want/need a DLL you should be using a library interface not 
the tool interface. If you can't / don't want to use the C++ API in 
clang/lib/Tooling, then we should push out a C API. Maybe you can confirm that 
https://github.com/apple/llvm-project/blob/apple/main/clang/include/clang-c/Dependencies.h
 in libclang will work for your needs? It landed on the fork just because it 
was experimental and libclang has a stable API promise, but I think the names 
of the functions make that clear enough -- e.g., 
`clang_experimental_DependencyScannerWorker_create_v0` seems well-enough named 
that no one expects permanent compatibility -- and it seems pretty awkward to 
be using the tool as a DLL to work around not having this in tree. Then your 
FastBuild can directly manage the service and the workers. WDYT?

@arphaman / @Bigcheese / @jansvoboda11 , thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102633

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

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Isn't the bug here that module hashing is using `hash_code`? So shouldn't the 
correct fix be to use a specific hashing algorithm for module hashes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

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

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

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

dexonsmith added a comment.

In D102943#2774732 , @jroelofs wrote:

> why do module hashes need to be stable when cross-compiling?

IIUC, the use case is cross-compiling when building the modules, which are then 
sent to the target to use when compiling other things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

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

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich updated this revision to Diff 347152.
vlovich added a comment.

Regen documentation & clang-format test cases.


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

https://reviews.llvm.org/D102706

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17871,6 +17871,7 @@
" aaa;\n"
"});",
getLLVMStyleWithColumns(60));
+
   verifyFormat("SomeFunction({[&] {\n"
"// comment\n"
"  },\n"
@@ -18423,6 +18424,117 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(100);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = "
+"std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) "
+"mutable { result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = "
+   "std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }).foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("std::sort(v.begin(), v.end(),\n"
+   "  [](const auto , const auto "
+   ") {\n"
+   "  return someLongArgumentName.someMemberVariable < "
+   "someOtherLongArgumentName.someMemberVariable;\n"
+   "});",
+   Style);
+  verifyFormat("test() {\n"
+   "  (\n"
+   "  []() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  },\n"
+   "  foo, bar)\n"
+   "  .foo();\n"
+   "}",
+   Style);
+  verifyFormat("test() {\n"
+   "  ([]() -> {\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  })\n"
+   "  .foo()\n"
+   "  .bar();\n"
+   "}",
+   Style);
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then(\n"
+"  [this, , someObject = "
+"std::mv(s)](std::vector evaluated) mutable {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { "
+"result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, , someObject = "
+   "std::mv(s)](std::vector evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style));
+  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
+"  return promise.then([this, ] {\n"
+"return someObject.startAsyncAction().then(\n"
+"[this, ](AsyncActionResult result) mutable { "
+"result.processMore(); });\n"
+"  });\n"
+"}\n",
+format("SomeResult doSomething(SomeObject promise) {\n"
+   "  return promise.then([this, ] {\n"
+   "return someObject.startAsyncAction().then([this, "
+   "](AsyncActionResult result) 

[clang] f7788e1 - Revert "[NewPM] Only invalidate modified functions' analyses in CGSCC passes"

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2021-05-21T16:38:03-07:00
New Revision: f7788e1bff223a58292b8b1d0818dac63b713ead

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

LOG: Revert "[NewPM] Only invalidate modified functions' analyses in CGSCC 
passes"

This reverts commit d14d84af2f5ebb8ae2188ce6884a29a586dc0a40.

Causes unacceptable memory regressions.

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Analysis/CGSCCPassManager.cpp
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/lib/Transforms/IPO/Inliner.cpp
llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Removed: 
llvm/test/Transforms/Inline/analysis-invalidation.ll



diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 7f478fb05f50..e20f78191572 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -64,10 +64,18 @@
 ; CHECK-O: Running analysis: OuterAnalysisManagerProxy
 ; CHECK-O: Running pass: InlinerPass on (main)
 ; CHECK-O: Running pass: PostOrderFunctionAttrsPass on (main)
+; CHECK-O: Invalidating analysis: DominatorTreeAnalysis on main
+; CHECK-O: Invalidating analysis: BasicAA on main
+; CHECK-O: Invalidating analysis: AAManager on main
 ; CHECK-O3: Running pass: ArgumentPromotionPass on (main)
 ; CHECK-O: Running pass: SROA on main
+; These next two can appear in any order since they are accessed as parameters
+; on the same call to SROA::runImpl
+; CHECK-O-DAG: Running analysis: DominatorTreeAnalysis on main
 ; CHECK-O: Running pass: EarlyCSEPass on main
 ; CHECK-O: Running analysis: MemorySSAAnalysis on main
+; CHECK-O: Running analysis: AAManager on main
+; CHECK-O: Running analysis: BasicAA on main
 ; CHECK-O: Running pass: SpeculativeExecutionPass on main
 ; CHECK-O: Running pass: JumpThreadingPass on main
 ; CHECK-O: Running analysis: LazyValueAnalysis on main
@@ -105,6 +113,16 @@
 ; CHECK-O: Running pass: LCSSAPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running pass: InstCombinePass on main
+; CHECK-O: Invalidating analysis: DominatorTreeAnalysis on main
+; CHECK-O: Invalidating analysis: BasicAA on main
+; CHECK-O: Invalidating analysis: AAManager on main
+; CHECK-O: Invalidating analysis: MemorySSAAnalysis on main
+; CHECK-O: Invalidating analysis: LoopAnalysis on main
+; CHECK-O: Invalidating analysis: PhiValuesAnalysis on main
+; CHECK-O: Invalidating analysis: MemoryDependenceAnalysis on main
+; CHECK-O: Invalidating analysis: DemandedBitsAnalysis on main
+; CHECK-O: Invalidating analysis: PostDominatorTreeAnalysis on main
+; CHECK-O: Invalidating analysis: CallGraphAnalysis
 ; CHECK-O: Running pass: GlobalOptPass
 ; CHECK-O: Running pass: GlobalDCEPass
 ; CHECK-O: Running pass: EliminateAvailableExternallyPass
@@ -114,13 +132,21 @@
 ; CHECK-O: Running pass: Float2IntPass on main
 ; CHECK-O: Running pass: LowerConstantIntrinsicsPass on main
 ; CHECK-O: Running pass: LoopSimplifyPass on main
+; CHECK-O: Running analysis: LoopAnalysis on main
 ; CHECK-O: Running pass: LCSSAPass on main
+; CHECK-O: Running analysis: MemorySSAAnalysis on main
+; CHECK-O: Running analysis: AAManager on main
+; CHECK-O: Running analysis: BasicAA on main
+; CHECK-O: Running analysis: ScalarEvolutionAnalysis on main
+; CHECK-O: Running analysis: InnerAnalysisManagerProxy
 ; CHECK-O: Running pass: LoopRotatePass on Loop at depth 1 containing: %b
 ; CHECK-O: Running pass: LoopDistributePass on main
 ; CHECK-O: Running pass: InjectTLIMappings on main
 ; CHECK-O: Running pass: LoopVectorizePass on main
 ; CHECK-O: Running analysis: BlockFrequencyAnalysis on main
 ; CHECK-O: Running analysis: BranchProbabilityAnalysis on main
+; CHECK-O: Running analysis: PostDominatorTreeAnalysis on main
+; CHECK-O: Running analysis: DemandedBitsAnalysis on main
 ; CHECK-O: Running pass: LoopLoadEliminationPass on main
 ; CHECK-O: Running pass: InstCombinePass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main

diff  --git a/llvm/lib/Analysis/CGSCCPassManager.cpp 
b/llvm/lib/Analysis/CGSCCPassManager.cpp
index c01704162e96..8043cc7fc742 100644
--- a/llvm/lib/Analysis/CGSCCPassManager.cpp
+++ b/llvm/lib/Analysis/CGSCCPassManager.cpp
@@ -858,7 +858,7 @@ incorporateNewSCCRange(const SCCRangeT , 
LazyCallGraph ,
   // split-off SCCs.
   // We know however that this will preserve any FAM proxy so go ahead and mark
   // that.
-  auto PA = PreservedAnalyses::allInSet>();
+  PreservedAnalyses PA;
   PA.preserve();
   AM.invalidate(*OldC, PA);
 

diff  --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp 

[PATCH] D102924: [clang] NFC: Replace std::pair by a struct in InitHeaderSearch

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

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102924

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

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

In D102706#2774010 , @MyDeveloperDay 
wrote:

> Is ```Signature``` effectively as is?

Yes. I noted that it's the default behavior. Please let me know if I can 
clarify the documentation wording somehow (or if you have preferred wording you 
want me to use)


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

https://reviews.llvm.org/D102706

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

[PATCH] D102730: [clang-format] Support custom If macros

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:252
+- Option ``IfMacros`` has been added. This lets you define macros that get
+  formatted like conditionals much like ``ForEachMacros`` get stiled like
+  foreach loops.

MyDeveloperDay wrote:
> stiled? did you mean styled?
Whoops. Yes I did.



Comment at: clang/lib/Format/FormatTokenLexer.cpp:1019
   FormatTok->setType(it->second);
+  if (it->second == TT_IfMacro) {
+FormatTok->Tok.setKind(tok::kw_if);

MyDeveloperDay wrote:
> Is there any chance you could leave a comment here as to why this is needed, 
> I can't work it out?
Will do my best.  TLDR: The token is otherwise "unknown" & the conditional 
formatting logic depends on the lexer token value. This avoids having to 
rewrite all the existing code & seemed to make sense since we want this token 
treated like an `if`.

I don't of course know if this is The Best Way (tm). It's just the minimal 
change I figured out would get the formatting to work properly but happy to 
apply feedback from someone more intimately familiar with the internals of the 
formatter as I'm a complete novice here.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3025
   return false;
+if (Left.is(TT_IfMacro)) {
+  return false;

MyDeveloperDay wrote:
> I'll let you decide if you think we need another SBPO_XXX style?
I thought about it but I wasn't was really sure how to add it in a way that 
would make sense. Do you think people would want to apply consistent SBPO 
styling for IF & FOREACH macros or want fine-grained control? If the former, 
then I can just check the foreach macro & maybe rename it to 
`SBPO_ControlStatementsExceptMacros` (maintaining the old name for back 
compat). If the latter, then it would seem like we need a separate boolean that 
controls whether SBPO_ControlStatements would apply?

My gut is probably the "maintain consistency" option is fine for now so I've 
gone ahead & applied that change in the latest diff.


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

https://reviews.llvm.org/D102730

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

[PATCH] D102730: [clang-format] Support custom If macros

[Vitali Lovich via Phabricator via cfe-commits]

vlovich updated this revision to Diff 347150.
vlovich marked 3 inline comments as done.
vlovich added a comment.

Review response. Updated the options I changed via the dump_format_style.py 
script but didn't ingest all the other changes it generated.
Changed the SBPO option to have a more generic name that also applies to IF 
macros.
Fix typo in release notes.


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

https://reviews.llvm.org/D102730

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1335,7 +1335,7 @@
 
   FormatStyle Style = getLLVMStyle();
   Style.SpaceBeforeParens =
-  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  FormatStyle::SBPO_ControlStatementsExceptControlMacros;
   verifyFormat("void f() {\n"
"  foreach(Item *item, itemlist) {}\n"
"  Q_FOREACH(Item *item, itemlist) {}\n"
@@ -16957,6 +16957,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: ControlStatementsExceptControlMacros", SpaceBeforeParens,
+  FormatStyle::SBPO_ControlStatementsExceptControlMacros);
   CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
   FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
@@ -16964,6 +16966,8 @@
   FormatStyle::SBPO_Never);
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: ControlStatementsExceptForEachMacros", SpaceBeforeParens,
+  FormatStyle::SBPO_ControlStatementsExceptControlMacros);
 
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
@@ -17111,6 +17115,11 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.IfMacros.clear();
+  std::vector CustomIfs;
+  CustomIfs.push_back("MYIF");
+  CHECK_PARSE("IfMacros: [MYIF]", IfMacros, CustomIfs);
+
   Style.AttributeMacros.clear();
   CHECK_PARSE("BasedOnStyle: LLVM", AttributeMacros,
   std::vector{"__capability"});
@@ -19684,11 +19693,16 @@
 
 TEST_F(FormatTest, SpacesInConditionalStatement) {
   FormatStyle Spaces = getLLVMStyle();
+  Spaces.IfMacros.clear();
+  Spaces.IfMacros.push_back("MYIF");
   Spaces.SpacesInConditionalStatement = true;
   verifyFormat("for ( int i = 0; i; i++ )\n  continue;", Spaces);
   verifyFormat("if ( !a )\n  return;", Spaces);
   verifyFormat("if ( a )\n  return;", Spaces);
   verifyFormat("if constexpr ( a )\n  return;", Spaces);
+  verifyFormat("MYIF ( a )\n  return;", Spaces);
+  verifyFormat("MYIF ( a )\n  return;\nelse MYIF ( b )\n  return;", Spaces);
+  verifyFormat("MYIF ( a )\n  return;\nelse\n  return;", Spaces);
   verifyFormat("switch ( a )\ncase 1:\n  return;", Spaces);
   verifyFormat("while ( a )\n  return;", Spaces);
   verifyFormat("while ( (a && b) )\n  return;", Spaces);
@@ -19697,6 +19711,12 @@
   // Check that space on the left of "::" is inserted as expected at beginning
   // of condition.
   verifyFormat("while ( ::func() )\n  return;", Spaces);
+
+  // Check impact of ControlStatementsExceptControlMacros is honored.
+  Spaces.SpaceBeforeParens = FormatStyle::SBPO_ControlStatementsExceptControlMacros;
+  verifyFormat("MYIF( a )\n  return;", Spaces);
+  verifyFormat("MYIF( a )\n  return;\nelse MYIF( b )\n  return;", Spaces);
+  verifyFormat("MYIF( a )\n  return;\nelse\n  return;", Spaces);
 }
 
 TEST_F(FormatTest, AlternativeOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1363,7 +1363,7 @@
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
 if (!CurrentToken->isOneOf(
-TT_LambdaLSquare, TT_LambdaLBrace, TT_AttributeMacro,
+TT_LambdaLSquare, TT_LambdaLBrace, TT_AttributeMacro, TT_IfMacro,
 TT_ForEachMacro, TT_TypenameMacro, TT_FunctionLBrace,
 TT_ImplicitStringLiteral, TT_InlineASMBrace, TT_FatArrow,
 TT_LambdaArrow, TT_NamespaceMacro, TT_OverloadedOperator,
@@ -3019,9 +3019,13 @@
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
 if (Style.SpaceBeforeParens ==
-

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Headers/cuda_wrappers/complex:79
+#if _GLIBCXX_RELEASE == 11
+#define __failed_assertion __cuda_failed_assertion
+#endif

yaxunl wrote:
> May I ask where is __cuda_failed_assertion defined? Thanks.
The function is not defined anywhere. 

From https://bugs.llvm.org/show_bug.cgi?id=50383#c14
> Its sole purpose is to be a non-constexpr function that can be called during 
> constant 
> evaluation to cause a compilation error. It can't be called at runtime, 
> because 
> compilation will fail if control flow ever results in calling it. So no 
> definition is needed.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

tejohnson wrote:
> nickdesaulniers wrote:
> > tejohnson wrote:
> > > nickdesaulniers wrote:
> > > > tejohnson wrote:
> > > > > nickdesaulniers wrote:
> > > > > > tejohnson wrote:
> > > > > > > What's the effect of or reason for this change?
> > > > > > Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
> > > > > > `mstack_protector_guard_offset_EQ`, 
> > > > > > `mstack_protector_guard_reg_EQ`) 2 are strings 
> > > > > > (`mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ`).  
> > > > > > It was inconsistent that one could be unspecified, where as the 
> > > > > > other could be unspecified or `"none"` (but those 2 values were 
> > > > > > equivalent).
> > > > > > 
> > > > > > Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd 
> > > > > > need to check that `StackProtectorGuardReg != "none"` rather than 
> > > > > > `!StackProtectorGuardReg.empty()` below.
> > > > > > 
> > > > > > I can change it back, but I think the asymmetry between 
> > > > > > `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in 
> > > > > > D88631, and I missed that in code review.
> > > > > > 
> > > > > > I don't think there would be any other observers of such a change.
> > > > > I see. Does unspecified mean something like just 
> > > > > "-mstack-protector-guard-reg=" with nothing after the =? I didn't 
> > > > > realize that was supported.
> > > > It looks like we validate for that case in the front end already. 
> > > > Specifically, `RenderAnalyzerOptions` in 
> > > > clang/lib/Driver/ToolChains/Clang.cpp.
> > > > 
> > > > $ clang -mstack-protector-guard-reg= ...
> > > > clang-13: error: invalid value '' in 'mstack-protector-guard-reg='
> > > Does that mean that without the "none" handling there is no way to 
> > > disable? I.e. overriding an earlier value. Not sure how important this is.
> > Oh, that's a great point.  I guess I'm not really sure of the intention of 
> > `"none"` then, @xiangzhangllvm can you comment whether that was the 
> > intention?
> > 
> > A quick test in GCC shows that GCC does not accept the value `"none"` for 
> > either `-mstack-protector-guard=` or `-mstack-protector-guard-reg=`.
> > 
> > We could strive to support disabling the command line flag once 
> > respecified, but I'd rather do it for both of the above two flags, not just 
> > `-mstack-protector-guard-reg=`.
> Yeah and it doesn't look like gcc supports anything like -mno-stack-...
> So this seems fine the way you have changed it, unless @xiangzhangllvm has a 
> different opinion, but perhaps that could be resolved later and consistently 
> between all the options as you note.
Sure, I'm happy to follow up on this if I got it wrong here. In the meantime, 
this will help our CI go back green, so I've merged it. Thank you much for the 
review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG033138ea452f: [IR] make stack-protector-guard-* flags into 
module attrs (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector-guard.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
  llvm/test/CodeGen/X86/stack-protector-3.ll
  llvm/test/Linker/stack-protector-guard-module-attrs.ll

Index: llvm/test/Linker/stack-protector-guard-module-attrs.ll
===
--- /dev/null
+++ llvm/test/Linker/stack-protector-guard-module-attrs.ll
@@ -0,0 +1,77 @@
+; RUN: split-file %s %t
+; RUN: not llvm-link %t/a.ll %t/b.ll 2>&1 | FileCheck --check-prefix=CHECK-KIND %s
+; RUN: not llvm-link %t/c.ll %t/d.ll 2>&1 | FileCheck --check-prefix=CHECK-REG %s
+; RUN: not llvm-link %t/e.ll %t/f.ll 2>&1 | FileCheck --check-prefix=CHECK-OFFSET %s
+; RUN: llvm-link %t/g.ll %t/h.ll
+
+; CHECK-KIND: error: linking module flags 'stack-protector-guard': IDs have conflicting values
+; CHECK-REG: error: linking module flags 'stack-protector-guard-reg': IDs have conflicting values
+; CHECK-OFFSET: error: linking module flags 'stack-protector-guard-offset': IDs have conflicting values
+
+;--- a.ll
+; Test that different values of stack-protector-guard fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+;--- b.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard", !"global"}
+
+;--- c.ll
+; Test that different values of stack-protector-guard-reg fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+;--- d.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-reg", !"sp_el1"}
+
+;--- e.ll
+; Test that different values of stack-protector-guard-offset fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-offset", i32 257}
+;--- f.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-offset", i32 256}
+
+;--- g.ll
+; Test that the same values for the three module attributes succeed.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0, !1, !2}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+!1 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+!2 = !{i32 1, !"stack-protector-guard-offset", i32 257}
+;--- h.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0, !1, !2}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+!1 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+!2 = !{i32 1, !"stack-protector-guard-offset", i32 257}
Index: llvm/test/CodeGen/X86/stack-protector-3.ll
===
--- llvm/test/CodeGen/X86/stack-protector-3.ll
+++ llvm/test/CodeGen/X86/stack-protector-3.ll
@@ -1,10 +1,18 @@
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=tls -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=global -o - < %s | FileCheck --check-prefix=CHECK-GLOBAL %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=fs -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=gs -o - < %s | FileCheck --check-prefix=CHECK-GS %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=20 -o - < %s | FileCheck --check-prefix=CHECK-OFFSET %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=-20 -o - < %s | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: cat %t/main.ll %t/c.ll > %t/c2.ll
+; RUN: cat %t/main.ll %t/d.ll > %t/d2.ll
+; RUN: cat %t/main.ll %t/e.ll > %t/e2.ll
+; RUN: cat %t/main.ll %t/f.ll > 

[clang] 033138e - [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via cfe-commits]

Author: Nick Desaulniers
Date: 2021-05-21T15:53:30-07:00
New Revision: 033138ea452f5f493fb5095e5963419905ad12e1

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

LOG: [IR] make stack-protector-guard-* flags into module attrs

D88631 added initial support for:

- -mstack-protector-guard=
- -mstack-protector-guard-reg=
- -mstack-protector-guard-offset=

flags, and D100919 extended these to AArch64. Unfortunately, these flags
aren't retained for LTO. Make them module attributes rather than
TargetOptions.

Link: https://github.com/ClangBuiltLinux/linux/issues/1378

Reviewed By: tejohnson

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

Added: 
clang/test/CodeGen/stack-protector-guard.c
llvm/test/Linker/stack-protector-guard-module-attrs.ll

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CodeGenModule.cpp
llvm/include/llvm/CodeGen/CommandFlags.h
llvm/include/llvm/IR/Module.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/CommandFlags.cpp
llvm/lib/CodeGen/StackProtector.cpp
llvm/lib/IR/Module.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
llvm/test/CodeGen/X86/stack-protector-3.ll

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 128aa7251bb7..b8cdc5f057b2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3426,7 +3426,7 @@ def mstack_protector_guard_offset_EQ : Joined<["-"], 
"mstack-protector-guard-off
   MarshallingInfoInt, "INT_MAX", 
"int">;
 def mstack_protector_guard_reg_EQ : Joined<["-"], 
"mstack-protector-guard-reg=">, Group, Flags<[CC1Option]>,
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,
   Flags<[CC1Option]>, Group,
   MarshallingInfoFlag>;

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index c0b29a85504d..a2d219e92d6b 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -558,15 +558,6 @@ static bool initTargetOptions(DiagnosticsEngine ,
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
   Options.UniqueBasicBlockSectionNames =
   CodeGenOpts.UniqueBasicBlockSectionNames;
-  Options.StackProtectorGuard =
-  llvm::StringSwitch(
-  CodeGenOpts.StackProtectorGuard)
-  .Case("tls", llvm::StackProtectorGuards::TLS)
-  .Case("global", llvm::StackProtectorGuards::Global)
-  .Case("sysreg", llvm::StackProtectorGuards::SysReg)
-  .Default(llvm::StackProtectorGuards::None);
-  Options.StackProtectorGuardOffset = CodeGenOpts.StackProtectorGuardOffset;
-  Options.StackProtectorGuardReg = CodeGenOpts.StackProtectorGuardReg;
   Options.TLSSize = CodeGenOpts.TLSSize;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
   Options.ExplicitEmulatedTLS = CodeGenOpts.ExplicitEmulatedTLS;

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 8d1b2ea5d522..744ccf0fe59f 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -777,6 +777,15 @@ void CodeGenModule::Release() {
   if (!getCodeGenOpts().RecordCommandLine.empty())
 EmitCommandLineMetadata();
 
+  if (!getCodeGenOpts().StackProtectorGuard.empty())
+getModule().setStackProtectorGuard(getCodeGenOpts().StackProtectorGuard);
+  if (!getCodeGenOpts().StackProtectorGuardReg.empty())
+getModule().setStackProtectorGuardReg(
+getCodeGenOpts().StackProtectorGuardReg);
+  if (getCodeGenOpts().StackProtectorGuardOffset != INT_MAX)
+getModule().setStackProtectorGuardOffset(
+getCodeGenOpts().StackProtectorGuardOffset);
+
   getTargetCodeGenInfo().emitTargetMetadata(*this, MangledDeclNames);
 
   EmitBackendOptionsMetadata(getCodeGenOpts());

diff  --git a/clang/test/CodeGen/stack-protector-guard.c 
b/clang/test/CodeGen/stack-protector-guard.c
new file mode 100644
index ..a5483ba0f194
--- /dev/null
+++ b/clang/test/CodeGen/stack-protector-guard.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg \
+// RUN:-mstack-protector-guard-reg=sp_el0 \
+// RUN:-mstack-protector-guard-offset=1024 \
+// RUN:-emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck --check-prefix=CHECK-NONE %s
+void foo(int*);
+void bar(int x) {
+  int baz[x];
+  foo(baz);
+}
+
+// CHECK: !llvm.module.flags = 

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[Vitali Lovich via Phabricator via cfe-commits]

vlovich added a comment.

In D102706#2774018 , @MyDeveloperDay 
wrote:

> Could you clang-format the tests please so I can more easily read them.

Sure. I was just taking my queue from the rest of the file which appeared to 
intentionally not clang-format the test strings.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2790
 
+**LambdaBodyIndentation** (``LambdaBodyIndentationKind``)
+  What is the lambda body indented relative to (default is ``Signature``).

MyDeveloperDay wrote:
> Did you generate this by hand or using the clang/doc/tools/dump_style.py?
By hand. `dump_format_style` generates other changes (although I wasn't aware 
of it at the time). I'll regenerate just this section using that tool.


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

https://reviews.llvm.org/D102706

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

[PATCH] D102736: Fix tmp files being left on Windows builds.

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.



Comment at: llvm/lib/Support/Path.cpp:1237
   RenameEC = copy_file(TmpName, Name);
   setDeleteDisposition(H, true);
 }

amccarth wrote:
> I'm curious if this path has ever been exercised.
> 
> I see that rename_handle is implemented with MoveFileExW on Windows.  
> MoveFileExW docs say it will fail if you're trying to move a _directory_ to 
> another device, but that it can do copy+delete to move a _file_ to another 
> device.  (That might require adding MOVEFILE_COPY_ALLOWED to the flags passed 
> in the MoveFileExW in lib\Support\Windows\Path.inc near line 485.)
> 
> Anyway, it just seems like we're re-implementing functionality the OS calls 
> can already do for us.
> 
> https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexw
 according to https://reviews.llvm.org/D50048 we used to pass 
MOVEFILE_COPY_ALLOWED but then removed it because it calls `rename_internal` 
which fails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102736

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Headers/cuda_wrappers/complex:79
+#if _GLIBCXX_RELEASE == 11
+#define __failed_assertion __cuda_failed_assertion
+#endif

May I ask where is __cuda_failed_assertion defined? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D102936#2774686 , @jwakely wrote:

> You can't use `__GLIBCXX__` this way. It will be different for different 
> snapshots from the gcc-11 branch. Some distros are already shipping gcc-11 
> snapshots with later dates.
>
> I would just check RELEASE == 11. If `__failed_assertion` is present, you'll 
> rename it. If it's not present, nothing gets renamed but it works anyway.

Good point. I've updated the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 347133.
tra added a comment.

Check only _GLIBCXX_RELEASE


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

Files:
  clang/lib/Headers/cuda_wrappers/complex


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assertion")
+#if _GLIBCXX_RELEASE == 11
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assertion")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assertion")
+#if _GLIBCXX_RELEASE == 11
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assertion")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Teresa Johnson via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

nickdesaulniers wrote:
> tejohnson wrote:
> > nickdesaulniers wrote:
> > > tejohnson wrote:
> > > > nickdesaulniers wrote:
> > > > > tejohnson wrote:
> > > > > > What's the effect of or reason for this change?
> > > > > Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
> > > > > `mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 
> > > > > 2 are strings (`mstack_protector_guard_EQ` and 
> > > > > `mstack_protector_guard_reg_EQ`).  It was inconsistent that one could 
> > > > > be unspecified, where as the other could be unspecified or `"none"` 
> > > > > (but those 2 values were equivalent).
> > > > > 
> > > > > Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need 
> > > > > to check that `StackProtectorGuardReg != "none"` rather than 
> > > > > `!StackProtectorGuardReg.empty()` below.
> > > > > 
> > > > > I can change it back, but I think the asymmetry between 
> > > > > `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in 
> > > > > D88631, and I missed that in code review.
> > > > > 
> > > > > I don't think there would be any other observers of such a change.
> > > > I see. Does unspecified mean something like just 
> > > > "-mstack-protector-guard-reg=" with nothing after the =? I didn't 
> > > > realize that was supported.
> > > It looks like we validate for that case in the front end already. 
> > > Specifically, `RenderAnalyzerOptions` in 
> > > clang/lib/Driver/ToolChains/Clang.cpp.
> > > 
> > > $ clang -mstack-protector-guard-reg= ...
> > > clang-13: error: invalid value '' in 'mstack-protector-guard-reg='
> > Does that mean that without the "none" handling there is no way to disable? 
> > I.e. overriding an earlier value. Not sure how important this is.
> Oh, that's a great point.  I guess I'm not really sure of the intention of 
> `"none"` then, @xiangzhangllvm can you comment whether that was the intention?
> 
> A quick test in GCC shows that GCC does not accept the value `"none"` for 
> either `-mstack-protector-guard=` or `-mstack-protector-guard-reg=`.
> 
> We could strive to support disabling the command line flag once respecified, 
> but I'd rather do it for both of the above two flags, not just 
> `-mstack-protector-guard-reg=`.
Yeah and it doesn't look like gcc supports anything like -mno-stack-...
So this seems fine the way you have changed it, unless @xiangzhangllvm has a 
different opinion, but perhaps that could be resolved later and consistently 
between all the options as you note.



Comment at: llvm/test/LTO/AArch64/stack-protector-guard-module-attrs.ll:2-5
+; RUN: not llvm-link %t/a.ll %t/b.ll 2>&1 | FileCheck 
--check-prefix=CHECK-KIND %s
+; RUN: not llvm-link %t/c.ll %t/d.ll 2>&1 | FileCheck --check-prefix=CHECK-REG 
%s
+; RUN: not llvm-link %t/e.ll %t/f.ll 2>&1 | FileCheck 
--check-prefix=CHECK-OFFSET %s
+; RUN: llvm-link %t/g.ll %t/h.ll

nickdesaulniers wrote:
> I used `llvm-link` here, but please let me know if it would be preferable to 
> convert these to use `llvm-lto2` instead?
> 
> Also, is there any issue with this new dir, `llvm/test/LTO/AArch64/`? Do I 
> need to modify any lit cfg files so that the tests don't run for non-aarch64?
> 
> Also, I guess this test isn't really specific to aarch64; I don't do any 
> codegen and don't specify the target triple (though, these module attributes 
> only will be emitted from the front end for x86 and aarch64 at the moment; 
> perhaps riscv and ppc64le in the future).
Yeah I think you will want to add the aarch64 equivalent of 
llvm/test/LTO/X86/lit.local.cfg, even if not needed for this initial test.



Comment at: llvm/test/LTO/AArch64/stack-protector-guard-module-attrs.ll:2-5
+; RUN: not llvm-link %t/a.ll %t/b.ll 2>&1 | FileCheck 
--check-prefix=CHECK-KIND %s
+; RUN: not llvm-link %t/c.ll %t/d.ll 2>&1 | FileCheck --check-prefix=CHECK-REG 
%s
+; RUN: not llvm-link %t/e.ll %t/f.ll 2>&1 | FileCheck 
--check-prefix=CHECK-OFFSET %s
+; RUN: llvm-link %t/g.ll %t/h.ll

nickdesaulniers wrote:
> tejohnson wrote:
> > nickdesaulniers wrote:
> > > I used `llvm-link` here, but please let me know if it would be preferable 
> > > to convert these to use `llvm-lto2` instead?
> > > 
> > > Also, is there any issue with this new dir, `llvm/test/LTO/AArch64/`? Do 
> > > I need to modify any lit cfg files so that the tests don't run for 
> > > non-aarch64?
> > > 
> > > Also, I guess this test isn't really specific to aarch64; I don't do any 
> > > codegen and don't specify the target triple (though, these module 
> > > attributes only will be 

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

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

- moved module attr mismatch test to llvm/test/Linker/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector-guard.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
  llvm/test/CodeGen/X86/stack-protector-3.ll
  llvm/test/Linker/stack-protector-guard-module-attrs.ll

Index: llvm/test/Linker/stack-protector-guard-module-attrs.ll
===
--- /dev/null
+++ llvm/test/Linker/stack-protector-guard-module-attrs.ll
@@ -0,0 +1,77 @@
+; RUN: split-file %s %t
+; RUN: not llvm-link %t/a.ll %t/b.ll 2>&1 | FileCheck --check-prefix=CHECK-KIND %s
+; RUN: not llvm-link %t/c.ll %t/d.ll 2>&1 | FileCheck --check-prefix=CHECK-REG %s
+; RUN: not llvm-link %t/e.ll %t/f.ll 2>&1 | FileCheck --check-prefix=CHECK-OFFSET %s
+; RUN: llvm-link %t/g.ll %t/h.ll
+
+; CHECK-KIND: error: linking module flags 'stack-protector-guard': IDs have conflicting values
+; CHECK-REG: error: linking module flags 'stack-protector-guard-reg': IDs have conflicting values
+; CHECK-OFFSET: error: linking module flags 'stack-protector-guard-offset': IDs have conflicting values
+
+;--- a.ll
+; Test that different values of stack-protector-guard fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+;--- b.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard", !"global"}
+
+;--- c.ll
+; Test that different values of stack-protector-guard-reg fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+;--- d.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-reg", !"sp_el1"}
+
+;--- e.ll
+; Test that different values of stack-protector-guard-offset fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-offset", i32 257}
+;--- f.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-offset", i32 256}
+
+;--- g.ll
+; Test that the same values for the three module attributes succeed.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0, !1, !2}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+!1 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+!2 = !{i32 1, !"stack-protector-guard-offset", i32 257}
+;--- h.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0, !1, !2}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+!1 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+!2 = !{i32 1, !"stack-protector-guard-offset", i32 257}
Index: llvm/test/CodeGen/X86/stack-protector-3.ll
===
--- llvm/test/CodeGen/X86/stack-protector-3.ll
+++ llvm/test/CodeGen/X86/stack-protector-3.ll
@@ -1,10 +1,18 @@
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=tls -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=global -o - < %s | FileCheck --check-prefix=CHECK-GLOBAL %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=fs -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=gs -o - < %s | FileCheck --check-prefix=CHECK-GS %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=20 -o - < %s | FileCheck --check-prefix=CHECK-OFFSET %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=-20 -o - < %s | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: cat %t/main.ll %t/c.ll > %t/c2.ll
+; RUN: cat %t/main.ll %t/d.ll > %t/d2.ll
+; RUN: cat %t/main.ll %t/e.ll > %t/e2.ll
+; RUN: cat %t/main.ll %t/f.ll > %t/f2.ll
+; RUN: cat %t/main.ll %t/g.ll > %t/g2.ll
+; RUN: 

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/test/LTO/AArch64/stack-protector-guard-module-attrs.ll:2-5
+; RUN: not llvm-link %t/a.ll %t/b.ll 2>&1 | FileCheck 
--check-prefix=CHECK-KIND %s
+; RUN: not llvm-link %t/c.ll %t/d.ll 2>&1 | FileCheck --check-prefix=CHECK-REG 
%s
+; RUN: not llvm-link %t/e.ll %t/f.ll 2>&1 | FileCheck 
--check-prefix=CHECK-OFFSET %s
+; RUN: llvm-link %t/g.ll %t/h.ll

nickdesaulniers wrote:
> I used `llvm-link` here, but please let me know if it would be preferable to 
> convert these to use `llvm-lto2` instead?
> 
> Also, is there any issue with this new dir, `llvm/test/LTO/AArch64/`? Do I 
> need to modify any lit cfg files so that the tests don't run for non-aarch64?
> 
> Also, I guess this test isn't really specific to aarch64; I don't do any 
> codegen and don't specify the target triple (though, these module attributes 
> only will be emitted from the front end for x86 and aarch64 at the moment; 
> perhaps riscv and ppc64le in the future).
Perhaps I should move this test under llvm/test/tools/llvm-link/ or 
llvm/test/Linker/ ? llvm/test/Linker/ has lots of tests that invoke `llvm-link`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D100794: [HIP] Support overloaded math functions for hipRTC

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Headers/__clang_hip_cmath.h:20
 #include 
 #include 
 #include 

ashi1 wrote:
> we may not need to `#include` limits and/or type_traits if we replaced them 
> with `__hip::is_integral` and `__hip::is_arithmetic`.
right. but we may need to move them to HIP headers first if HIP headers need 
them.



Comment at: clang/lib/Headers/__clang_hip_cmath.h:587
 #endif
-#endif
+#endif // !defined(__HIPCC_RTC__)
 

ashi1 wrote:
> I don't think this `#endif` is for `!defined(__HIPCC_RTC__)`. It should be 
> for `_LIBCPP_BEGIN_NAMESPACE_STD`.
You are right. fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100794

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

[clang] 91dfd68 - [NFC][HIP] fix comments in __clang_hip_cmath.h

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-05-21T17:44:18-04:00
New Revision: 91dfd68e90156dab3cbf11c9ae677cf60b2df65c

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

LOG: [NFC][HIP] fix comments in __clang_hip_cmath.h

Added: 


Modified: 
clang/lib/Headers/__clang_hip_cmath.h

Removed: 




diff  --git a/clang/lib/Headers/__clang_hip_cmath.h 
b/clang/lib/Headers/__clang_hip_cmath.h
index 5d7b75ffdcc0f..b5d7c16ac5e41 100644
--- a/clang/lib/Headers/__clang_hip_cmath.h
+++ b/clang/lib/Headers/__clang_hip_cmath.h
@@ -583,8 +583,8 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 namespace std {
 #ifdef _GLIBCXX_BEGIN_NAMESPACE_VERSION
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
-#endif
-#endif // !defined(__HIPCC_RTC__)
+#endif // _GLIBCXX_BEGIN_NAMESPACE_VERSION
+#endif // _LIBCPP_BEGIN_NAMESPACE_STD
 
 // Pull the new overloads we defined above into namespace std.
 // using ::abs; - This may be considered for C++.
@@ -729,9 +729,9 @@ _LIBCPP_END_NAMESPACE_STD
 #else
 #ifdef _GLIBCXX_BEGIN_NAMESPACE_VERSION
 _GLIBCXX_END_NAMESPACE_VERSION
-#endif
+#endif // _GLIBCXX_BEGIN_NAMESPACE_VERSION
 } // namespace std
-#endif
+#endif // _LIBCPP_END_NAMESPACE_STD
 #endif // !defined(__HIPCC_RTC__)
 
 // Define device-side math functions from  on MSVC.



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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

tejohnson wrote:
> nickdesaulniers wrote:
> > tejohnson wrote:
> > > nickdesaulniers wrote:
> > > > tejohnson wrote:
> > > > > What's the effect of or reason for this change?
> > > > Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
> > > > `mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 2 
> > > > are strings (`mstack_protector_guard_EQ` and 
> > > > `mstack_protector_guard_reg_EQ`).  It was inconsistent that one could 
> > > > be unspecified, where as the other could be unspecified or `"none"` 
> > > > (but those 2 values were equivalent).
> > > > 
> > > > Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need to 
> > > > check that `StackProtectorGuardReg != "none"` rather than 
> > > > `!StackProtectorGuardReg.empty()` below.
> > > > 
> > > > I can change it back, but I think the asymmetry between 
> > > > `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in 
> > > > D88631, and I missed that in code review.
> > > > 
> > > > I don't think there would be any other observers of such a change.
> > > I see. Does unspecified mean something like just 
> > > "-mstack-protector-guard-reg=" with nothing after the =? I didn't realize 
> > > that was supported.
> > It looks like we validate for that case in the front end already. 
> > Specifically, `RenderAnalyzerOptions` in 
> > clang/lib/Driver/ToolChains/Clang.cpp.
> > 
> > $ clang -mstack-protector-guard-reg= ...
> > clang-13: error: invalid value '' in 'mstack-protector-guard-reg='
> Does that mean that without the "none" handling there is no way to disable? 
> I.e. overriding an earlier value. Not sure how important this is.
Oh, that's a great point.  I guess I'm not really sure of the intention of 
`"none"` then, @xiangzhangllvm can you comment whether that was the intention?

A quick test in GCC shows that GCC does not accept the value `"none"` for 
either `-mstack-protector-guard=` or `-mstack-protector-guard-reg=`.

We could strive to support disabling the command line flag once respecified, 
but I'd rather do it for both of the above two flags, not just 
`-mstack-protector-guard-reg=`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/test/LTO/AArch64/stack-protector-guard-module-attrs.ll:2-5
+; RUN: not llvm-link %t/a.ll %t/b.ll 2>&1 | FileCheck 
--check-prefix=CHECK-KIND %s
+; RUN: not llvm-link %t/c.ll %t/d.ll 2>&1 | FileCheck --check-prefix=CHECK-REG 
%s
+; RUN: not llvm-link %t/e.ll %t/f.ll 2>&1 | FileCheck 
--check-prefix=CHECK-OFFSET %s
+; RUN: llvm-link %t/g.ll %t/h.ll

I used `llvm-link` here, but please let me know if it would be preferable to 
convert these to use `llvm-lto2` instead?

Also, is there any issue with this new dir, `llvm/test/LTO/AArch64/`? Do I need 
to modify any lit cfg files so that the tests don't run for non-aarch64?

Also, I guess this test isn't really specific to aarch64; I don't do any 
codegen and don't specify the target triple (though, these module attributes 
only will be emitted from the front end for x86 and aarch64 at the moment; 
perhaps riscv and ppc64le in the future).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 347124.
nickdesaulniers added a comment.
Herald added a subscriber: steven_wu.

- add llvm/tests/LTO/AArch64/ and test case for module attr mismatch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector-guard.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
  llvm/test/CodeGen/X86/stack-protector-3.ll
  llvm/test/LTO/AArch64/stack-protector-guard-module-attrs.ll

Index: llvm/test/LTO/AArch64/stack-protector-guard-module-attrs.ll
===
--- /dev/null
+++ llvm/test/LTO/AArch64/stack-protector-guard-module-attrs.ll
@@ -0,0 +1,77 @@
+; RUN: split-file %s %t
+; RUN: not llvm-link %t/a.ll %t/b.ll 2>&1 | FileCheck --check-prefix=CHECK-KIND %s
+; RUN: not llvm-link %t/c.ll %t/d.ll 2>&1 | FileCheck --check-prefix=CHECK-REG %s
+; RUN: not llvm-link %t/e.ll %t/f.ll 2>&1 | FileCheck --check-prefix=CHECK-OFFSET %s
+; RUN: llvm-link %t/g.ll %t/h.ll
+
+; CHECK-KIND: error: linking module flags 'stack-protector-guard': IDs have conflicting values
+; CHECK-REG: error: linking module flags 'stack-protector-guard-reg': IDs have conflicting values
+; CHECK-OFFSET: error: linking module flags 'stack-protector-guard-offset': IDs have conflicting values
+
+;--- a.ll
+; Test that different values of stack-protector-guard fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+;--- b.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard", !"global"}
+
+;--- c.ll
+; Test that different values of stack-protector-guard-reg fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+;--- d.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-reg", !"sp_el1"}
+
+;--- e.ll
+; Test that different values of stack-protector-guard-offset fail.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-offset", i32 257}
+;--- f.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"stack-protector-guard-offset", i32 256}
+
+;--- g.ll
+; Test that the same values for the three module attributes succeed.
+define void @foo() sspstrong {
+  ret void
+}
+!llvm.module.flags = !{!0, !1, !2}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+!1 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+!2 = !{i32 1, !"stack-protector-guard-offset", i32 257}
+;--- h.ll
+declare void @foo() sspstrong
+define void @bar() sspstrong {
+  call void @foo()
+  ret void
+}
+!llvm.module.flags = !{!0, !1, !2}
+!0 = !{i32 1, !"stack-protector-guard", !"sysreg"}
+!1 = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+!2 = !{i32 1, !"stack-protector-guard-offset", i32 257}
Index: llvm/test/CodeGen/X86/stack-protector-3.ll
===
--- llvm/test/CodeGen/X86/stack-protector-3.ll
+++ llvm/test/CodeGen/X86/stack-protector-3.ll
@@ -1,10 +1,18 @@
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=tls -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=global -o - < %s | FileCheck --check-prefix=CHECK-GLOBAL %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=fs -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=gs -o - < %s | FileCheck --check-prefix=CHECK-GS %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=20 -o - < %s | FileCheck --check-prefix=CHECK-OFFSET %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=-20 -o - < %s | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: cat %t/main.ll %t/c.ll > %t/c2.ll
+; RUN: cat %t/main.ll %t/d.ll > %t/d2.ll
+; RUN: cat %t/main.ll %t/e.ll > %t/e2.ll
+; RUN: cat %t/main.ll 

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

why do module hashes need to be stable when cross-compiling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D101793#2774191 , @weiwang wrote:

> In D101793#2772717 , @yaxunl wrote:
>
>> In D101793#2772461 , @weiwang 
>> wrote:
>>
>>> Thanks for the approval!
>>>
>>> Just want to understand the list of "decls to check for deferred 
>>> diagnostics" better, where are these decls coming from? And why do they 
>>> need to be checked for warnings? I see decls from libc are in the list, but 
>>> I have no idea why are they selected.
>>
>> For offloading languages e.g. OpenMP/CUDA/HIP, there are apparent errors in 
>> functions shared between host and device. However, unless these functions 
>> are sure to be emitted on device or host, these errors should not be 
>> emitted. These errors are so called deferred error messages. The function 
>> decls which need to be checked are recorded. After AST is finalized, the AST 
>> of these functions are iterated. If a function is found sure to be emitted, 
>> the deferred error message in it are emitted.
>
> Thanks! So the `DeclsToCheckForDeferredDiags` contains the candidate decls to 
> be checked. The decls are selected because they would generate diags in the 
> context of offloading languages, but whether or not an diag will be emitted 
> is deferred till traversal of the AST is performed. I still don't quite 
> understand why would libc functions be in the candidate list? They look 
> simple enough (and I think they've been there forever).  For example 
> `__uint16_identity` 
> (https://code.woboq.org/userspace/glibc/bits/uintn-identity.h.html#32),
>
>   static inline __uint16_t __uint16_identity(__uint16_t __x) {
>   return __x;
>   }
>
> I don't see how this function would generate diag either on host or device.

A function may call another function which contains deferred diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

nickdesaulniers wrote:
> tejohnson wrote:
> > nickdesaulniers wrote:
> > > tejohnson wrote:
> > > > What's the effect of or reason for this change?
> > > Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
> > > `mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 2 
> > > are strings (`mstack_protector_guard_EQ` and 
> > > `mstack_protector_guard_reg_EQ`).  It was inconsistent that one could be 
> > > unspecified, where as the other could be unspecified or `"none"` (but 
> > > those 2 values were equivalent).
> > > 
> > > Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need to 
> > > check that `StackProtectorGuardReg != "none"` rather than 
> > > `!StackProtectorGuardReg.empty()` below.
> > > 
> > > I can change it back, but I think the asymmetry between 
> > > `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in 
> > > D88631, and I missed that in code review.
> > > 
> > > I don't think there would be any other observers of such a change.
> > I see. Does unspecified mean something like just 
> > "-mstack-protector-guard-reg=" with nothing after the =? I didn't realize 
> > that was supported.
> It looks like we validate for that case in the front end already. 
> Specifically, `RenderAnalyzerOptions` in 
> clang/lib/Driver/ToolChains/Clang.cpp.
> 
> $ clang -mstack-protector-guard-reg= ...
> clang-13: error: invalid value '' in 'mstack-protector-guard-reg='
Does that mean that without the "none" handling there is no way to disable? 
I.e. overriding an earlier value. Not sure how important this is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102812: [clang] Don't pass multiple backend options if mixing -mimplicit-it and -Wa,-mimplicit-it

[Martin Storsjö via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4468e5b89992: [clang] Dont pass multiple backend 
options if mixing -mimplicit-it and -Wa… (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102812

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/arm-target-as-mimplicit-it.s

Index: clang/test/Driver/arm-target-as-mimplicit-it.s
===
--- clang/test/Driver/arm-target-as-mimplicit-it.s
+++ clang/test/Driver/arm-target-as-mimplicit-it.s
@@ -10,22 +10,23 @@
 // RUN: %clang -target arm-linux-gnueabi -### -Xassembler -mimplicit-it=arm %s 2>&1 | FileCheck %s --check-prefix=ARM
 // RUN: %clang -target arm-linux-gnueabi -### -Xassembler -mimplicit-it=thumb %s 2>&1 | FileCheck %s --check-prefix=THUMB
 /// Test space separated -Wa,- arguments (latter wins).
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always -Wa,-mimplicit-it=always %s 2>&1 | FileCheck %s --check-prefix=ALWAYS
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=never -Wa,-mimplicit-it=always %s 2>&1 | FileCheck %s --check-prefix=ALWAYS
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always -Wa,-mimplicit-it=never %s 2>&1 | FileCheck %s --check-prefix=NEVER
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always -Wa,-mimplicit-it=arm %s 2>&1 | FileCheck %s --check-prefix=ARM
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always -Wa,-mimplicit-it=thumb %s 2>&1 | FileCheck %s --check-prefix=THUMB
 /// Test comma separated -Wa,- arguments (latter wins).
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always,-mimplicit-it=always %s 2>&1 | FileCheck %s --check-prefix=ALWAYS
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=never,-mimplicit-it=always %s 2>&1 | FileCheck %s --check-prefix=ALWAYS
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always,-mimplicit-it=never %s 2>&1 | FileCheck %s --check-prefix=NEVER
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always,-mimplicit-it=arm %s 2>&1 | FileCheck %s --check-prefix=ARM
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always,-mimplicit-it=thumb %s 2>&1 | FileCheck %s --check-prefix=THUMB
 
-/// Mix -implicit-it= (compiler) with -Wa,-mimplicit-it= (assembler), assembler
-/// takes priority. -mllvm -arm-implicit-it= will be repeated, with the
-/// assembler flag appearing last (latter wins).
-// RUN: %clang -target arm-linux-gnueabi -### -mimplicit-it=never -Wa,-mimplicit-it=always %S/Inputs/wildcard1.c 2>&1 | FileCheck %s --check-prefix=NEVER_ALWAYS
-// RUN: %clang -target arm-linux-gnueabi -### -mimplicit-it=always -Wa,-mimplicit-it=never %S/Inputs/wildcard1.c 2>&1 | FileCheck %s --check-prefix=ALWAYS_NEVER
-// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=never -mimplicit-it=always %S/Inputs/wildcard1.c 2>&1 | FileCheck %s --check-prefix=ALWAYS_NEVER
+/// Mix -implicit-it= (compiler) with -Wa,-mimplicit-it= (assembler), the
+/// last one set takes priority.
+// RUN: %clang -target arm-linux-gnueabi -### -mimplicit-it=always -Wa,-mimplicit-it=always %S/Inputs/wildcard1.c 2>&1 | FileCheck %s --check-prefix=ALWAYS
+// RUN: %clang -target arm-linux-gnueabi -### -mimplicit-it=never -Wa,-mimplicit-it=always %S/Inputs/wildcard1.c 2>&1 | FileCheck %s --check-prefix=ALWAYS
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=never -mimplicit-it=always %S/Inputs/wildcard1.c 2>&1 | FileCheck %s --check-prefix=ALWAYS
 
 /// Test invalid input.
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=foo %s 2>&1 | FileCheck %s --check-prefix=INVALID
@@ -34,11 +35,15 @@
 // RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always,-mimplicit-it=foo %s 2>&1 | FileCheck %s --check-prefix=INVALID
 
 
+/// Check that there isn't a second -arm-implicit-it before or after the one
+/// that was the indended match.
+// ALWAYS-NOT: "-arm-implicit-it={{.*}}"
 // ALWAYS: "-mllvm" "-arm-implicit-it=always"
+// ALWAYS-NOT: "-arm-implicit-it={{.*}}"
+// NEVER-NOT: "-arm-implicit-it={{.*}}"
 // NEVER: "-mllvm" "-arm-implicit-it=never"
+// NEVER-NOT: "-arm-implicit-it={{.*}}"
 // ARM: "-mllvm" "-arm-implicit-it=arm"
 // THUMB: "-mllvm" "-arm-implicit-it=thumb"
-// NEVER_ALWAYS: "-mllvm" "-arm-implicit-it=never" "-mllvm" "-arm-implicit-it=always"
-// ALWAYS_NEVER: "-mllvm" "-arm-implicit-it=always" "-mllvm" "-arm-implicit-it=never"
 // INVALID: error: unsupported argument '-mimplicit-it=foo' to option 'Wa,'
 // XINVALID: error: unsupported argument '-mimplicit-it=foo' to option 'Xassembler'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp

[clang] 4468e5b - [clang] Don't pass multiple backend options if mixing -mimplicit-it and -Wa,-mimplicit-it

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2021-05-22T00:05:31+03:00
New Revision: 4468e5b8999291cc84b78f33f207dcd0e58146bc

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

LOG: [clang] Don't pass multiple backend options if mixing -mimplicit-it and 
-Wa,-mimplicit-it

If multiple instances of the -arm-implicit-it option is passed to
the backend, it errors out.

Also fix cases where there are multiple -Wa,-mimplicit-it; the existing
tests indicate that the last one specified takes effect, while in
practice it passed double options, which didn't work as intended.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/arm-target-as-mimplicit-it.s

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index d30195b558816..e668bf09ad25a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2366,15 +2366,15 @@ void Clang::DumpCompilationDatabaseFragmentToDir(
   DumpCompilationDatabase(C, "", Target, Output, Input, Args);
 }
 
-static bool AddARMImplicitITArgs(const ArgList , ArgStringList ,
+static bool CheckARMImplicitITArg(StringRef Value) {
+  return Value == "always" || Value == "never" || Value == "arm" ||
+ Value == "thumb";
+}
+
+static void AddARMImplicitITArgs(const ArgList , ArgStringList ,
  StringRef Value) {
-  if (Value == "always" || Value == "never" || Value == "arm" ||
-  Value == "thumb") {
-CmdArgs.push_back("-mllvm");
-CmdArgs.push_back(Args.MakeArgString("-arm-implicit-it=" + Value));
-return true;
-  }
-  return false;
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back(Args.MakeArgString("-arm-implicit-it=" + Value));
 }
 
 static void CollectArgsForIntegratedAssembler(Compilation ,
@@ -2393,22 +2393,6 @@ static void 
CollectArgsForIntegratedAssembler(Compilation ,
DefaultIncrementalLinkerCompatible))
 CmdArgs.push_back("-mincremental-linker-compatible");
 
-  switch (C.getDefaultToolChain().getArch()) {
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-if (Arg *A = Args.getLastArg(options::OPT_mimplicit_it_EQ)) {
-  StringRef Value = A->getValue();
-  if (!AddARMImplicitITArgs(Args, CmdArgs, Value))
-D.Diag(diag::err_drv_unsupported_option_argument)
-<< A->getOption().getName() << Value;
-}
-break;
-  default:
-break;
-  }
-
   // If you add more args here, also add them to the block below that
   // starts with "// If CollectArgsForIntegratedAssembler() isn't called 
below".
 
@@ -2422,8 +2406,27 @@ static void 
CollectArgsForIntegratedAssembler(Compilation ,
   bool UseRelaxRelocations = C.getDefaultToolChain().useRelaxRelocations();
   bool UseNoExecStack = C.getDefaultToolChain().isNoExecStackDefault();
   const char *MipsTargetFeature = nullptr;
+  StringRef ImplicitIt;
   for (const Arg *A :
-   Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
+   Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler,
+ options::OPT_mimplicit_it_EQ)) {
+if (A->getOption().getID() == options::OPT_mimplicit_it_EQ) {
+  switch (C.getDefaultToolChain().getArch()) {
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+// Only store the value; the last value set takes effect.
+ImplicitIt = A->getValue();
+if (!CheckARMImplicitITArg(ImplicitIt))
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getOption().getName() << ImplicitIt;
+continue;
+  default:
+break;
+  }
+}
+
 A->claim();
 
 for (StringRef Value : A->getValues()) {
@@ -2444,9 +2447,12 @@ static void 
CollectArgsForIntegratedAssembler(Compilation ,
   case llvm::Triple::thumbeb:
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
-if (Value.startswith("-mimplicit-it=") &&
-AddARMImplicitITArgs(Args, CmdArgs, Value.split("=").second))
-  continue;
+if (Value.startswith("-mimplicit-it=")) {
+  // Only store the value; the last value set takes effect.
+  ImplicitIt = Value.split("=").second;
+  if (CheckARMImplicitITArg(ImplicitIt))
+continue;
+}
 if (Value == "-mthumb")
   // -mthumb has already been processed in ComputeLLVMTriple()
   // recognize but skip over here.
@@ -2576,6 +2582,8 @@ static void CollectArgsForIntegratedAssembler(Compilation 
,
   }
 }
   }
+  if (ImplicitIt.size())
+

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely requested changes to this revision.
jwakely added a comment.
This revision now requires changes to proceed.

You can't use `__GLIBCXX__` this way. It will be different for different 
snapshots from the gcc-11 branch.

I would just check RELEASE == 11. If `__failed_assertion` is present, you'll 
rename it. If it's not present, nothing gets renamed but it works anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers planned changes to this revision.
nickdesaulniers added a comment.

Will work on explicit LTO tests.




Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

tejohnson wrote:
> nickdesaulniers wrote:
> > tejohnson wrote:
> > > What's the effect of or reason for this change?
> > Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
> > `mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 2 are 
> > strings (`mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ`).  
> > It was inconsistent that one could be unspecified, where as the other could 
> > be unspecified or `"none"` (but those 2 values were equivalent).
> > 
> > Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need to 
> > check that `StackProtectorGuardReg != "none"` rather than 
> > `!StackProtectorGuardReg.empty()` below.
> > 
> > I can change it back, but I think the asymmetry between 
> > `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in D88631, 
> > and I missed that in code review.
> > 
> > I don't think there would be any other observers of such a change.
> I see. Does unspecified mean something like just 
> "-mstack-protector-guard-reg=" with nothing after the =? I didn't realize 
> that was supported.
It looks like we validate for that case in the front end already. Specifically, 
`RenderAnalyzerOptions` in clang/lib/Driver/ToolChains/Clang.cpp.

$ clang -mstack-protector-guard-reg= ...
clang-13: error: invalid value '' in 'mstack-protector-guard-reg='


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D102742#2774639 , @nickdesaulniers 
wrote:

> In D102742#2774562 , @tejohnson 
> wrote:
>
>> In D102742#2774185 , 
>> @nickdesaulniers wrote:
>>
>>> - upgrade module merge strategy to Error
>>
>> Probably want a test using llvm-link or llvm-lto to check this behavior 
>> (that alike flags are getting propagated as expected and that conflicting 
>> ones error)
>
> Not many tests under llvm/test/LTO or llvm/test/ThinLTO use `llvm-link`; is 
> there a more appropriate subdir for such a test for conflicting module 
> attributes?

Hmm, maybe in llvm/test/Linker? Or you could presumably use llvm-lto.




Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

nickdesaulniers wrote:
> tejohnson wrote:
> > What's the effect of or reason for this change?
> Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
> `mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 2 are 
> strings (`mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ`).  
> It was inconsistent that one could be unspecified, where as the other could 
> be unspecified or `"none"` (but those 2 values were equivalent).
> 
> Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need to check 
> that `StackProtectorGuardReg != "none"` rather than 
> `!StackProtectorGuardReg.empty()` below.
> 
> I can change it back, but I think the asymmetry between 
> `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in D88631, 
> and I missed that in code review.
> 
> I don't think there would be any other observers of such a change.
I see. Does unspecified mean something like just "-mstack-protector-guard-reg=" 
with nothing after the =? I didn't realize that was supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 347113.
nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added a comment.

- doxygen comments, test that module flags aren't emitted without flags


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector-guard.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
  llvm/test/CodeGen/X86/stack-protector-3.ll

Index: llvm/test/CodeGen/X86/stack-protector-3.ll
===
--- llvm/test/CodeGen/X86/stack-protector-3.ll
+++ llvm/test/CodeGen/X86/stack-protector-3.ll
@@ -1,10 +1,18 @@
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=tls -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=global -o - < %s | FileCheck --check-prefix=CHECK-GLOBAL %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=fs -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=gs -o - < %s | FileCheck --check-prefix=CHECK-GS %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=20 -o - < %s | FileCheck --check-prefix=CHECK-OFFSET %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=-20 -o - < %s | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: cat %t/main.ll %t/c.ll > %t/c2.ll
+; RUN: cat %t/main.ll %t/d.ll > %t/d2.ll
+; RUN: cat %t/main.ll %t/e.ll > %t/e2.ll
+; RUN: cat %t/main.ll %t/f.ll > %t/f2.ll
+; RUN: cat %t/main.ll %t/g.ll > %t/g2.ll
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/a2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/b2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/c2.ll | FileCheck --check-prefix=CHECK-GLOBAL %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/d2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/e2.ll | FileCheck --check-prefix=CHECK-GS %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/f2.ll | FileCheck --check-prefix=CHECK-OFFSET %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/g2.ll | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
 
 ; CHECK-TLS-FS-40:   movq%fs:40, %rax
 ; CHECK-TLS-FS-40:   movq%fs:40, %rax
@@ -47,6 +55,8 @@
 ; CHECK-GLOBAL-NEXT:  .cfi_def_cfa_offset 32
 ; CHECK-GLOBAL-NEXT:  callq   __stack_chk_fail
 
+;--- main.ll
+
 ; ModuleID = 't.c'
 @.str = private unnamed_addr constant [14 x i8] c"stackoverflow\00", align 1
 @a = dso_local local_unnamed_addr global i8* null, align 8
@@ -75,3 +85,23 @@
 attributes #0 = { nounwind sspreq uwtable writeonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #1 = { argmemonly nounwind willreturn }
 attributes #2 = { nounwind }
+
+;--- a.ll
+;--- b.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+;--- c.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard", !"global"}
+;--- d.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-reg", !"fs"}
+;--- e.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-reg", !"gs"}
+;--- f.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-offset", i32 20}
+;--- g.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-offset", i32 -20}
Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -1,46 +1,39 @@
-; RUN: llc %s --stack-protector-guard=sysreg \
-; RUN:   --stack-protector-guard-reg=sp_el0 \
-; RUN:   

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D102742#2774562 , @tejohnson wrote:

> In D102742#2774185 , 
> @nickdesaulniers wrote:
>
>> - upgrade module merge strategy to Error
>
> Probably want a test using llvm-link or llvm-lto to check this behavior (that 
> alike flags are getting propagated as expected and that conflicting ones 
> error)

Not many tests under llvm/test/LTO or llvm/test/ThinLTO use `llvm-link`; is 
there a more appropriate subdir for such a test for conflicting module 
attributes?




Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

tejohnson wrote:
> What's the effect of or reason for this change?
Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
`mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 2 are 
strings (`mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ`).  It 
was inconsistent that one could be unspecified, where as the other could be 
unspecified or `"none"` (but those 2 values were equivalent).

Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need to check 
that `StackProtectorGuardReg != "none"` rather than 
`!StackProtectorGuardReg.empty()` below.

I can change it back, but I think the asymmetry between 
`mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in D88631, and 
I missed that in code review.

I don't think there would be any other observers of such a change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102663: Bug 49633 - Added warning for static inline global and namespaced declarations for c++17+

[Serberoth via Phabricator via cfe-commits]

serberoth added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:7127
 << FixItHint::CreateRemoval(D.getDeclSpec().getInlineSpecLoc());
+} else if (SC == SC_Static && DC->isFileContext() &&
+   getLangOpts().CPlusPlus17) {

erichkeane wrote:
> First, what about this is C++17 specific?  The inline and static relationship 
> is older than C++17, right?
> 
> Also, are you sure that the warning/stuff in the 'else' isn't still valuable?
Perhaps I misunderstood something in the bug write-up, the component for the 
ticket is C++17.  Also there is the warning that `inline variables are a C++17 
extension` that appears when you use the inline keyword on a variable 
(regardless of the appearance of the static keyword).  I suppose that does not 
necessarily mean we cannot simply show both warnings, and maybe that is the 
right thing to do.  I felt that was not really necessary because of the other 
warning.

As for the other warnings in the else the one is the warning that I mentioned 
(which only applies when the C++17 standard is not applied, and the other is a 
C++14 compatibility warning which states:
"inline variables are incompatible with C++ standards before C++17"

You can see the messages for those diagnostic message here:
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticSemaKinds.td#L1467

Please let me know if I am still missing something with how this diagnostic was 
supposed to apply and where.  Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102663

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

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

[Alexandre Rames via Phabricator via cfe-commits]

arames created this revision.
Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman, hiraditya.
arames requested review of this revision.
Herald added projects: clang, LLVM, clang-tools-extra.
Herald added subscribers: cfe-commits, llvm-commits.

`size_t` varying across platforms can cause issues, for example mismatching
hashes when cross-compiling modules from a 64bit target to a 32bit target.

Although the comments note that `hash_code` values "are not stable to save or
persist", it is effectively used in such cases today, for example for implicit
module hashes.

Since hashing mechanisms in practice already use `uint64_t` for computations,
use `uint64_t` instead of `size_t` to store the value of `hash_code`.
This is similar to earlier changes in c0445098519defc4bd13624095fac92977c9cfe4.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102943

Files:
  clang-tools-extra/clangd/index/SymbolID.cpp
  clang/include/clang/AST/DataCollection.h
  clang/lib/Lex/HeaderSearch.cpp
  llvm/include/llvm/ADT/Hashing.h
  llvm/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
  llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp
  llvm/unittests/ADT/HashingTest.cpp

Index: llvm/unittests/ADT/HashingTest.cpp
===
--- llvm/unittests/ADT/HashingTest.cpp
+++ llvm/unittests/ADT/HashingTest.cpp
@@ -22,7 +22,7 @@
 
 // Helper for test code to print hash codes.
 void PrintTo(const hash_code , std::ostream *os) {
-  *os << static_cast(code);
+  *os << static_cast(code);
 }
 
 // Fake an object that is recognized as hashable data to test super large
@@ -134,7 +134,7 @@
 
 // Provide a dummy, hashable type designed for easy verification: its hash is
 // the same as its value.
-struct HashableDummy { size_t value; };
+struct HashableDummy { uint64_t value; };
 hash_code hash_value(HashableDummy dummy) { return dummy.value; }
 
 TEST(HashingTest, HashCombineRangeBasicTest) {
@@ -172,7 +172,7 @@
   EXPECT_NE(dummy_hash, arr4_hash);
   EXPECT_NE(arr1_hash, arr4_hash);
 
-  const size_t arr5[] = { 1, 2, 3 };
+  const uint64_t arr5[] = { 1, 2, 3 };
   const HashableDummy d_arr5[] = { {1}, {2}, {3} };
   hash_code arr5_hash = hash_combine_range(begin(arr5), end(arr5));
   hash_code d_arr5_hash = hash_combine_range(begin(d_arr5), end(d_arr5));
@@ -182,11 +182,11 @@
 TEST(HashingTest, HashCombineRangeLengthDiff) {
   // Test that as only the length varies, we compute different hash codes for
   // sequences.
-  std::map code_to_size;
+  std::map code_to_size;
   std::vector all_one_c(256, '\xff');
   for (unsigned Idx = 1, Size = all_one_c.size(); Idx < Size; ++Idx) {
 hash_code code = hash_combine_range(_one_c[0], _one_c[0] + Idx);
-std::map::iterator
+std::map::iterator
   I = code_to_size.insert(std::make_pair(code, Idx)).first;
 EXPECT_EQ(Idx, I->second);
   }
@@ -194,7 +194,7 @@
   std::vector all_zero_c(256, '\0');
   for (unsigned Idx = 1, Size = all_zero_c.size(); Idx < Size; ++Idx) {
 hash_code code = hash_combine_range(_zero_c[0], _zero_c[0] + Idx);
-std::map::iterator
+std::map::iterator
   I = code_to_size.insert(std::make_pair(code, Idx)).first;
 EXPECT_EQ(Idx, I->second);
   }
@@ -202,7 +202,7 @@
   std::vector all_one_int(512, -1);
   for (unsigned Idx = 1, Size = all_one_int.size(); Idx < Size; ++Idx) {
 hash_code code = hash_combine_range(_one_int[0], _one_int[0] + Idx);
-std::map::iterator
+std::map::iterator
   I = code_to_size.insert(std::make_pair(code, Idx)).first;
 EXPECT_EQ(Idx, I->second);
   }
@@ -210,7 +210,7 @@
   std::vector all_zero_int(512, 0);
   for (unsigned Idx = 1, Size = all_zero_int.size(); Idx < Size; ++Idx) {
 hash_code code = hash_combine_range(_zero_int[0], _zero_int[0] + Idx);
-std::map::iterator
+std::map::iterator
   I = code_to_size.insert(std::make_pair(code, Idx)).first;
 EXPECT_EQ(Idx, I->second);
   }
@@ -283,8 +283,7 @@
 fprintf(stderr, " { %-35s 0x%016llxULL },\n",
 member_str.c_str(), static_cast(hash));
 #endif
-EXPECT_EQ(static_cast(golden_data[i].hash),
-  static_cast(hash));
+EXPECT_EQ(golden_data[i].hash, static_cast(hash));
   }
 }
 
@@ -304,9 +303,9 @@
   // Hashing a sequence of heterogeneous types which *happen* to all produce the
   // same data for hashing produces the same as a range-based hash of the
   // fundamental values.
-  const size_t s1 = 1024, s2 = , s3 = 900;
+  const uint64_t s1 = 1024, s2 = , s3 = 900;
   const HashableDummy d1 = { 1024 }, d2 = {  }, d3 = { 900 };
-  const size_t arr2[] = { s1, s2, s3 };
+  const uint64_t arr2[] = { s1, s2, s3 };
   EXPECT_EQ(hash_combine_range(begin(arr2), end(arr2)),
 hash_combine(s1, s2, s3));
   EXPECT_EQ(hash_combine(s1, s2, s3), hash_combine(s1, s2, d3));
Index: llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp
===
--- 

[PATCH] D100276: [clang] p1099 3/5: using Enum::member

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM. An additional review here would be nice though.

> good, it appears the patch bot fail is fixed -- it seems unhappy with 
> non-tree-like dependency graphs, and tries to apply each reachable patch as 
> many times as it is reachable :(
> The data race failure seems entirely unconnected with this patch -- please 
> correct me if I'm wrong.

Indeed!




Comment at: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3.cpp:20
 class C {
+public:
   int g();

urnathan wrote:
> bruno wrote:
> > bruno wrote:
> > > > The change to 
> > > > clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3.cpp is to 
> > > > silence an uninteresting access error that the above change causes.
> > > 
> > > The fact that the access check changed for previous version of the 
> > > language (not necessarily related to p1099) indicates that this specific 
> > > change deserves its own patch.
> > How about this one?
> I didn;t describe this very well.  What happens with the reordering of the 
> checks of the named scopes is that we now diagnose that C::g is private and 
> then that we're reaching into a struct.  Before we just diagnosed the second 
> problem and bailed, never getting to the first.  The test is aiming to 
> discover that second problem is detected, and doesn't care about the access.  
> Hence make it public.
Oh I see, makes sense. Thanks for the explanation.


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

https://reviews.llvm.org/D100276

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D102742#2774185 , @nickdesaulniers 
wrote:

> - upgrade module merge strategy to Error

Probably want a test using llvm-link or llvm-lto to check this behavior (that 
alike flags are getting propagated as expected and that conflicting ones error)




Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString, [{"none"}]>;
+  MarshallingInfoString>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,

What's the effect of or reason for this change?



Comment at: clang/test/CodeGen/stack-protector-guard.c:4
+// RUN:-mstack-protector-guard-offset=1024 \
+// RUN:-emit-llvm %s -o - | FileCheck %s
+void foo(int*);

Perhaps add a check that the module flags not added without these options



Comment at: llvm/include/llvm/IR/Module.h:898
 
+  StringRef getStackProtectorGuard() const;
+  void setStackProtectorGuard(StringRef Kind);

add a doxygen comment like the ones for the other interfaces here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102863: [analyzer] RetainCountChecker: Disable tracking for references to OSMetaClass.

[Georgeta Igna via Phabricator via cfe-commits]

georgi_igna updated this revision to Diff 347107.
georgi_igna added a comment.

whitespace issue solved


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102863

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/os_object_base.h
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -721,6 +721,16 @@
   obj->release();
 }
 
+void test_osmetaclass_release(){
+const char *name = "no_name";
+const OSMetaClass *meta = OSMetaClass::copyMetaClassWithName(name);
+if (!meta) {
+return;
+} else {
+meta->releaseMetaClass();
+}
+}
+
 class SampleClass {
 public:
   OSObjectPtr field;
Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -68,6 +68,8 @@
 struct OSMetaClass : public OSMetaClassBase {
   virtual OSObject * alloc() const;
   static OSObject * allocClassWithName(const char * name);
+  static const OSMetaClass * copyMetaClassWithName(const char * name);
+  void releaseMetaClass() const;
   virtual ~OSMetaClass(){}
 };
 
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -146,14 +146,20 @@
   return !(match(SubclassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectSubclass(const Decl *D) {
-  return D && isSubclass(D, "OSMetaClassBase");
+static bool isExactClass(const Decl *D, StringRef ClassName) {
+  using namespace ast_matchers;
+  DeclarationMatcher sameClassM =
+  cxxRecordDecl(hasName(std::string(ClassName)));
+  return !(match(sameClassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectDynamicCast(StringRef S) {
-  return S == "safeMetaCast";
+static bool isOSObjectSubclass(const Decl *D) {
+  return D && isSubclass(D, "OSMetaClassBase") &&
+ !isExactClass(D, "OSMetaClass");
 }
 
+static bool isOSObjectDynamicCast(StringRef S) { return S == "safeMetaCast"; }
+
 static bool isOSObjectRequiredCast(StringRef S) {
   return S == "requiredMetaCast";
 }


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -721,6 +721,16 @@
   obj->release();
 }
 
+void test_osmetaclass_release(){
+const char *name = "no_name";
+const OSMetaClass *meta = OSMetaClass::copyMetaClassWithName(name);
+if (!meta) {
+return;
+} else {
+meta->releaseMetaClass();
+}
+}
+
 class SampleClass {
 public:
   OSObjectPtr field;
Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -68,6 +68,8 @@
 struct OSMetaClass : public OSMetaClassBase {
   virtual OSObject * alloc() const;
   static OSObject * allocClassWithName(const char * name);
+  static const OSMetaClass * copyMetaClassWithName(const char * name);
+  void releaseMetaClass() const;
   virtual ~OSMetaClass(){}
 };
 
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -146,14 +146,20 @@
   return !(match(SubclassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectSubclass(const Decl *D) {
-  return D && isSubclass(D, "OSMetaClassBase");
+static bool isExactClass(const Decl *D, StringRef ClassName) {
+  using namespace ast_matchers;
+  DeclarationMatcher sameClassM =
+  cxxRecordDecl(hasName(std::string(ClassName)));
+  return !(match(sameClassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectDynamicCast(StringRef S) {
-  return S == "safeMetaCast";
+static bool isOSObjectSubclass(const Decl *D) {
+  return D && isSubclass(D, "OSMetaClassBase") &&
+ !isExactClass(D, "OSMetaClass");
 }
 
+static bool isOSObjectDynamicCast(StringRef S) { return S == "safeMetaCast"; }
+
 static bool isOSObjectRequiredCast(StringRef S) {
   return S == "requiredMetaCast";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102863: [analyzer] RetainCountChecker: Disable tracking for references to OSMetaClass.

[Georgeta Igna via Phabricator via cfe-commits]

georgi_igna updated this revision to Diff 347104.
georgi_igna added a comment.

trailing whitespace solved in os_object_base.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102863

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/os_object_base.h
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -721,6 +721,16 @@
   obj->release();
 }
 
+void test_osmetaclass_release(){
+const char *name = "no_name";
+const OSMetaClass *meta = OSMetaClass::copyMetaClassWithName(name);
+if (!meta) {
+return;
+} else {
+meta->releaseMetaClass();
+}
+}
+
 class SampleClass {
 public:
   OSObjectPtr field;
Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -67,7 +67,13 @@
 
 struct OSMetaClass : public OSMetaClassBase {
   virtual OSObject * alloc() const;
+
   static OSObject * allocClassWithName(const char * name);
+
+  static const OSMetaClass * copyMetaClassWithName(const char * name);
+
+  void releaseMetaClass() const;
+
   virtual ~OSMetaClass(){}
 };
 
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -146,14 +146,20 @@
   return !(match(SubclassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectSubclass(const Decl *D) {
-  return D && isSubclass(D, "OSMetaClassBase");
+static bool isExactClass(const Decl *D, StringRef ClassName) {
+  using namespace ast_matchers;
+  DeclarationMatcher sameClassM =
+  cxxRecordDecl(hasName(std::string(ClassName)));
+  return !(match(sameClassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectDynamicCast(StringRef S) {
-  return S == "safeMetaCast";
+static bool isOSObjectSubclass(const Decl *D) {
+  return D && isSubclass(D, "OSMetaClassBase") &&
+ !isExactClass(D, "OSMetaClass");
 }
 
+static bool isOSObjectDynamicCast(StringRef S) { return S == "safeMetaCast"; }
+
 static bool isOSObjectRequiredCast(StringRef S) {
   return S == "requiredMetaCast";
 }


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -721,6 +721,16 @@
   obj->release();
 }
 
+void test_osmetaclass_release(){
+const char *name = "no_name";
+const OSMetaClass *meta = OSMetaClass::copyMetaClassWithName(name);
+if (!meta) {
+return;
+} else {
+meta->releaseMetaClass();
+}
+}
+
 class SampleClass {
 public:
   OSObjectPtr field;
Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -67,7 +67,13 @@
 
 struct OSMetaClass : public OSMetaClassBase {
   virtual OSObject * alloc() const;
+
   static OSObject * allocClassWithName(const char * name);
+
+  static const OSMetaClass * copyMetaClassWithName(const char * name);
+
+  void releaseMetaClass() const;
+
   virtual ~OSMetaClass(){}
 };
 
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -146,14 +146,20 @@
   return !(match(SubclassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectSubclass(const Decl *D) {
-  return D && isSubclass(D, "OSMetaClassBase");
+static bool isExactClass(const Decl *D, StringRef ClassName) {
+  using namespace ast_matchers;
+  DeclarationMatcher sameClassM =
+  cxxRecordDecl(hasName(std::string(ClassName)));
+  return !(match(sameClassM, *D, D->getASTContext()).empty());
 }
 
-static bool isOSObjectDynamicCast(StringRef S) {
-  return S == "safeMetaCast";
+static bool isOSObjectSubclass(const Decl *D) {
+  return D && isSubclass(D, "OSMetaClassBase") &&
+ !isExactClass(D, "OSMetaClass");
 }
 
+static bool isOSObjectDynamicCast(StringRef S) { return S == "safeMetaCast"; }
+
 static bool isOSObjectRequiredCast(StringRef S) {
   return S == "requiredMetaCast";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Joachim Meyer via Phabricator via cfe-commits]

fodinabor accepted this revision.
fodinabor added a comment.

LGTM :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102940: [OpenMP] Remove OpenMP CUDA Target Parallel compiler flag

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, tianshilei1992.
jhuber6 added a project: clang.
Herald added subscribers: dexonsmith, dang, guansong, yaxunl.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.

The changes introduced in D97680  turns this 
command line option into a no-op so
it can be removed entirely.

Depends on D97680 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102940

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3475,9 +3475,6 @@
   if (Opts.OpenMPCUDAMode)
 GenerateArg(Args, OPT_fopenmp_cuda_mode, SA);
 
-  if (Opts.OpenMPCUDATargetParallel)
-GenerateArg(Args, OPT_fopenmp_cuda_parallel_target_regions, SA);
-
   if (Opts.OpenMPCUDAForceFullRuntime)
 GenerateArg(Args, OPT_fopenmp_cuda_force_full_runtime, SA);
 
@@ -3905,12 +3902,6 @@
   Opts.OpenMPCUDAMode = Opts.OpenMPIsDevice && (T.isNVPTX() || T.isAMDGCN()) &&
 Args.hasArg(options::OPT_fopenmp_cuda_mode);
 
-  // Set CUDA support for parallel execution of target regions for OpenMP 
target
-  // NVPTX/AMDGCN if specified in options.
-  Opts.OpenMPCUDATargetParallel =
-  Opts.OpenMPIsDevice && (T.isNVPTX() || T.isAMDGCN()) &&
-  Args.hasArg(options::OPT_fopenmp_cuda_parallel_target_regions);
-
   // Set CUDA mode for OpenMP target NVPTX/AMDGCN if specified in options
   Opts.OpenMPCUDAForceFullRuntime =
   Opts.OpenMPIsDevice && (T.isNVPTX() || T.isAMDGCN()) &&
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5673,13 +5673,6 @@
options::OPT_fno_openmp_cuda_mode, /*Default=*/false))
 CmdArgs.push_back("-fopenmp-cuda-mode");
 
-  // When in OpenMP offloading mode with NVPTX target, forward
-  // cuda-parallel-target-regions flag
-  if (Args.hasFlag(options::OPT_fopenmp_cuda_parallel_target_regions,
-   options::OPT_fno_openmp_cuda_parallel_target_regions,
-   /*Default=*/true))
-CmdArgs.push_back("-fopenmp-cuda-parallel-target-regions");
-
   // When in OpenMP offloading mode with NVPTX target, check if full 
runtime
   // is required.
   if (Args.hasFlag(options::OPT_fopenmp_cuda_force_full_runtime,
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2340,12 +2340,6 @@
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",
   LangOpts<"OpenMPOptimisticCollapse">, DefaultFalse,
   PosFlag, NegFlag, 
BothFlags<[NoArgumentUnused, HelpHidden]>>;
-def fopenmp_cuda_parallel_target_regions : Flag<["-"], 
"fopenmp-cuda-parallel-target-regions">, Group,
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
-  HelpText<"Support parallel execution of target regions on Cuda-based 
devices.">;
-def fno_openmp_cuda_parallel_target_regions : Flag<["-"], 
"fno-openmp-cuda-parallel-target-regions">, Group,
-  Flags<[NoArgumentUnused, HelpHidden]>,
-  HelpText<"Support only serial execution of target regions on Cuda-based 
devices.">;
 def static_openmp: Flag<["-"], "static-openmp">,
   HelpText<"Use the static host OpenMP runtime while linking.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -237,7 +237,6 @@
 LANGOPT(OpenMPCUDABlocksPerSM  , 32, 0, "Number of blocks per SM for CUDA 
devices.")
 LANGOPT(OpenMPCUDAReductionBufNum , 32, 1024, "Number of the reduction records 
in the intermediate reduction buffer used for the teams reductions.")
 LANGOPT(OpenMPOptimisticCollapse  , 1, 0, "Use at most 32 bits to represent 
the collapsed loop nest counter.")
-LANGOPT(OpenMPCUDATargetParallel, 1, 0, "Support parallel execution of target 
region on Cuda-based devices.")
 LANGOPT(RenderScript  , 1, 0, "RenderScript")
 
 LANGOPT(CUDAIsDevice  , 1, 0, "compiling for CUDA device")


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3475,9 +3475,6 @@
   if (Opts.OpenMPCUDAMode)
 GenerateArg(Args, 

[PATCH] D97680: [OpenMP] Simplify GPU memory globalization

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 347097.
jhuber6 added a comment.

Fixing nits and splitting command line removal to a new patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97680

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/test/OpenMP/assumes_include_nvptx.cpp
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/target_parallel_debug_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/globalization_remarks.ll
  llvm/test/Transforms/PhaseOrdering/openmp-opt-module.ll

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Headers/cuda_wrappers/complex:77
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assert")
+#if _GLIBCXX_RELEASE == 11 && __GLIBCXX__ == 20210427

tra wrote:
> fodinabor wrote:
> > Not sure I understand what the push of `__failed_assert` is for..? can't 
> > find any reference to `__failed_assert` anywhere... did you mean to write 
> > `__failed_assertion`?
> It's a precaution -- we have no control over what may or may not be defined 
> outside of this header.
> 
That, and I've made a typo. Fixed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 347092.
tra edited the summary of this revision.
tra added a comment.

Fixed typo in push/pop macro name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

Files:
  clang/lib/Headers/cuda_wrappers/complex


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assertion")
+#if _GLIBCXX_RELEASE == 11 && __GLIBCXX__ == 20210427
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assertion")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assertion")
+#if _GLIBCXX_RELEASE == 11 && __GLIBCXX__ == 20210427
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assertion")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Headers/cuda_wrappers/complex:77
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assert")
+#if _GLIBCXX_RELEASE == 11 && __GLIBCXX__ == 20210427

fodinabor wrote:
> Not sure I understand what the push of `__failed_assert` is for..? can't find 
> any reference to `__failed_assert` anywhere... did you mean to write 
> `__failed_assertion`?
It's a precaution -- we have no control over what may or may not be defined 
outside of this header.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

Right, @teemperor, I will do the refactors once we figure out how to utilize 
`trackExpressionValue()` here (because that will eliminate quite a bit of code 
from `GetNoteVisitor`, so no point in refactoring those bits).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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

[PATCH] D102736: Fix tmp files being left on Windows builds.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 347090.
akhuang marked an inline comment as done.
akhuang added a comment.

Use TempFile on both linux and windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102736

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/Windows/Path.inc

Index: llvm/lib/Support/Windows/Path.inc
===
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -560,11 +560,6 @@
   return errc::permission_denied;
 }
 
-static std::error_code rename_fd(int FromFD, const Twine ) {
-  HANDLE FromHandle = reinterpret_cast(_get_osfhandle(FromFD));
-  return rename_handle(FromHandle, To);
-}
-
 std::error_code rename(const Twine , const Twine ) {
   // Convert to utf-16.
   SmallVector WideFrom;
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -1229,7 +1229,7 @@
   auto H = reinterpret_cast(_get_osfhandle(FD));
   std::error_code RenameEC = setDeleteDisposition(H, false);
   if (!RenameEC) {
-RenameEC = rename_fd(FD, Name);
+RenameEC = rename_handle(H, Name);
 // If rename failed because it's cross-device, copy instead
 if (RenameEC ==
   std::error_code(ERROR_NOT_SAME_DEVICE, std::system_category())) {
@@ -1261,7 +1261,6 @@
 return errorCodeToError(EC);
   }
   FD = -1;
-
   return errorCodeToError(RenameEC);
 }
 
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -704,29 +704,38 @@
 void CompilerInstance::clearOutputFiles(bool EraseFiles) {
   for (OutputFile  : OutputFiles) {
 if (EraseFiles) {
-  if (!OF.TempFilename.empty()) {
-llvm::sys::fs::remove(OF.TempFilename);
-continue;
+  if (OF.TempFile) {
+if (llvm::Error E = OF.TempFile->discard())
+  consumeError(std::move(E));
   }
   if (!OF.Filename.empty())
 llvm::sys::fs::remove(OF.Filename);
   continue;
 }
 
-if (OF.TempFilename.empty())
+if (!OF.TempFile)
+  continue;
+
+if (OF.TempFile->TmpName.empty()) {
+  if (llvm::Error E = OF.TempFile->discard())
+consumeError(std::move(E));
   continue;
+}
 
 // If '-working-directory' was passed, the output filename should be
 // relative to that.
 SmallString<128> NewOutFile(OF.Filename);
 FileMgr->FixupRelativePath(NewOutFile);
-std::error_code EC = llvm::sys::fs::rename(OF.TempFilename, NewOutFile);
-if (!EC)
+
+std::string ErrorMsg;
+llvm::Error E = OF.TempFile->keep(NewOutFile);
+if (!E)
   continue;
+
 getDiagnostics().Report(diag::err_unable_to_rename_temp)
-<< OF.TempFilename << OF.Filename << EC.message();
+<< OF.TempFile->TmpName << OF.Filename << toString(std::move(E));
 
-llvm::sys::fs::remove(OF.TempFilename);
+llvm::sys::fs::remove(OF.TempFile->TmpName);
   }
   OutputFiles.clear();
   if (DeleteBuiltModules) {
@@ -808,7 +817,7 @@
 }
   }
 
-  std::string TempFile;
+  Optional Temp;
   if (UseTemporary) {
 // Create a temporary file.
 // Insert - before the extension (if any), and because some tools
@@ -820,25 +829,36 @@
 TempPath += "-";
 TempPath += OutputExtension;
 TempPath += ".tmp";
-int fd;
-std::error_code EC = llvm::sys::fs::createUniqueFile(
-TempPath, fd, TempPath,
-Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);
-
-if (CreateMissingDirectories &&
-EC == llvm::errc::no_such_file_or_directory) {
-  StringRef Parent = llvm::sys::path::parent_path(OutputPath);
-  EC = llvm::sys::fs::create_directories(Parent);
-  if (!EC) {
-EC = llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath,
- Binary ? llvm::sys::fs::OF_None
-: llvm::sys::fs::OF_Text);
-  }
-}
+Expected ExpectedFile =
+llvm::sys::fs::TempFile::create(TempPath);
+
+llvm::Error E = handleErrors(
+ExpectedFile.takeError(), [&](const llvm::ECError ) -> llvm::Error {
+  std::error_code EC = E.convertToErrorCode();
+  if (CreateMissingDirectories &&
+  EC == llvm::errc::no_such_file_or_directory) {
+StringRef Parent = llvm::sys::path::parent_path(OutputPath);
+EC = llvm::sys::fs::create_directories(Parent);
+if (!EC) {
+  ExpectedFile = llvm::sys::fs::TempFile::create(TempPath);
+  if (!ExpectedFile)
+return llvm::errorCodeToError(
+

[PATCH] D102736: Fix tmp files being left on Windows builds.

[Amy Huang via Phabricator via cfe-commits]

akhuang marked 2 inline comments as done.
akhuang added a comment.

In D102736#2767432 , @aganea wrote:

> Do you think the existing crash tests can be modified to validate that .tmp 
> files are deleted indeed?

Looks like the existing crash tests use `#pragma clang __debug crash` but in 
the case where clang crashes it's actually fine. I guess the only cases where 
this patch actually applies is when the program is killed -- don't know if 
there's a good way to do that in the test suite?




Comment at: clang/lib/Frontend/CompilerInstance.cpp:733
+} else {
+  if (std::error_code EC =
+  llvm::sys::fs::rename(OF.TempFilename, NewOutFile))

aganea wrote:
> Here you can probably use `ECError` or `errorCodeToError()` with what is said 
> slightly above.
I think this path goes away if we always use TempFile



Comment at: clang/lib/Frontend/CompilerInstance.cpp:840
 int fd;
-std::error_code EC = llvm::sys::fs::createUniqueFile(
-TempPath, fd, TempPath,
-Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);
+#if _WIN32
+// On Windows, use llvm::sys::fs::TempFile to write the output file so

aganea wrote:
> Usually it is better to avoid platform-specific logic. Could we use 
> `TempFile` all the time on all platforms?
Hm, probably, that would make things simpler. 



Comment at: llvm/lib/Support/Path.cpp:1253
   }
   sys::DontRemoveFileOnSignal(TmpName);
 #endif

amccarth wrote:
> Possibly stupid idea:  What if RemoveFileOnSignal and DontRemoveFileOnSignal 
> did the Win32-specific delete disposition flag twiddling?  With that 
> encapsulated, and assuming we can still explicitly delete a Windows file 
> that's already marked for deletion, it seems like all of the special handling 
> here could go away.
> 
> Then again, the ...OnSignal things are in sys rather than sys::fs, so maybe 
> it's not such a good idea.
> 
> I'm not trying to make more work for Amy.  The immediate goal probably isn't 
> well served by this level of overthinking.  But it would be nice to see all 
> this get simpler in the long term.
@rnk suggested that too - I think it would definitely make things nicer, but 
yeah, the fact that it's in sys and not sys::fs is inconvenient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102736

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Joachim Meyer via Phabricator via cfe-commits]

fodinabor added a comment.

Sadly don't have stdibc++11 available locally as well.. so can't verify, but 
looks good to me except for the push and pop macros.




Comment at: clang/lib/Headers/cuda_wrappers/complex:77
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assert")
+#if _GLIBCXX_RELEASE == 11 && __GLIBCXX__ == 20210427

Not sure I understand what the push of `__failed_assert` is for..? can't find 
any reference to `__failed_assert` anywhere... did you mean to write 
`__failed_assertion`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added reviewers: jlebar, jwakely.
Herald added subscribers: bixia, yaxunl.
tra requested review of this revision.
Herald added a project: clang.

libstdc++ 11.1.0 redeclares __failed_assertion multiple times and that results 
in the
function declared with conflicting set of attributes when we include 
with __host__ __device__ attributes force-applied to all functions.

In order to work around the issue, we rename __failed_assertion within the
region with forced attributes.

See https://bugs.llvm.org/show_bug.cgi?id=50383 for the details.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102936

Files:
  clang/lib/Headers/cuda_wrappers/complex


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assert")
+#if _GLIBCXX_RELEASE == 11 && __GLIBCXX__ == 20210427
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assert")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assert")
+#if _GLIBCXX_RELEASE == 11 && __GLIBCXX__ == 20210427
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assert")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D102696#2774169 , @martong wrote:

> In D102696#2773340 , @vsavchenko 
> wrote:
>
>> In D102696#2773304 , @martong 
>> wrote:
>>
 As for the solver, it is something that tormented me for a long time. Is 
 there a way to avoid a full-blown brute force check of all existing 
 constraints and get some knowledge about symbolic expressions by 
 constraints these symbolic expressions are actually part of (right now we 
 can reason about expressions if we know some information about its own 
 parts aka operands)?
>>>
>>> Well, it is a hard question.
>>> I've been thinking about building a "parent" map for the sub-expressions, 
>>> like we do in the AST (see clang::ParentMap). We could use this parent map 
>>> to inject new constraints during the "constant folding" mechanism.
>>> So, let's say we have `$x + $y = 0` and then when we process `$y = 0` then 
>>> we'd add a new constraint: `$x = 0`. We could add this new constraint by 
>>> knowing that we have to visit `$x + $y` because `$y` is connected to that 
>>> in the parent map.
>>> What do you think, could it work?
>>
>> Yes, this was exactly my line of thinking.  Instead of trying to use hard 
>> logic every time we are asked a question, we can try to simplify existing 
>> constraints based on the new bit of information.
>> The main problem with this is the tree nature of symbolic expressions.  If 
>> we have something trivial like `$x + $y` - sure.  But something with lot 
>> more nested subexpressions can get tricky really fast.  That can be solved 
>> if we have a canonical form for the trees (this will resolve the problem 
>> with `$x + $y` versus `$y + $x` as well).  Right now, we have bits of this 
>> all around our codebase, but those are more like workarounds as opposed to 
>> one general approach.
>
> OK, I have started  working on a constraint manager based prototype 
> implementation with a parent map and a hook in `setConstraint`. The test file 
> in this patch (even the commutativity test) passes with that, so, it looks 
> promising. I hope I can share that next week. Also, I'd like do some 
> performance measurements as well, both on this patch and for the new solver 
> based one.

Parent map is more or less trivial for situations like `$x + $y`, where we 
simply relate `$x` and `$y`. But what should we do for a more general case when 
`$x` and `$y` are expressions themselves. One simple example here is `$a +$b 
+$c` as originally constrained and `$a + $c` that got constrained later on.  
Should we include `$a`, `$b`, $c`, `$a + $b`, `$a + $c`, and `$b + $c` to the 
parent map to make it work?

And again even with parent map, we still need canonicalization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102696

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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:16389
+  for (const auto  : Styles) {
+verifyFormat("struct test demo[] = {\n"
+ "{56, 23,\"hello\" },\n"

may be worth testing a case with comments at the end of each line

```
verifyFormat("struct test demo[3] = {\n"
 "{56, 23,\"hello\" }, // first line\n"
 "{-1, 93463, \"world\" }, /* second line */\n"
 "{7,  5, \"!!\"} // at the end\n"
 "};\n",
 Style);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

This is a valiant effort, and I definitely don't want to discourage..especially 
as you shown a good understand of clang-format. Its been a long asked for 
feature.. but we should really ensure this covers the bulk of the complaints if 
possible..

For me I use this.. a search for `clang-format off` in github projects

https://github.com/search?q=clang-format+off

  Status AbsGrad(const AttrSlice& attrs, FunctionDef* g) {
// clang-format off
return GradForUnaryCwise(g, {
{{"sign"}, "Sign", {"x"}, {}, {"dy"}},
{{"dx"}, "Mul", {"dy", "sign"}},
});
// clang-format on
  }

https://github.com/lulzsec2012/tensorflow/blob/d68d869e397515655e9f41570f4db463df770563/tensorflow/core/ops/math_grad.cc

it would be my ideal if we could irradiate the need for these, as they heavily 
dominate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[Fred Grim via Phabricator via cfe-commits]

feg208 added a comment.

In D101868#2774152 , @MyDeveloperDay 
wrote:

> This looks like a good start..

Thanks. I am reworking it so I handle line breaking in a sane fashion and 
dropping the LineFormatter override since that really can't handle reformatting 
without reimplementing most of it

> All your tests are 3x3 have you considered mixing it up a bit.  i.e. 2x3, 
> what is the impact on 1x5 and 5x1 ?

I am also adding a bunch more tests. I'll roll this in.

> Also how about nested structs, I'm interested to see what happens
>
>   {56, 23, { "ABC", 35 }}
>   {57, 24, { "DEF", 36 }}

Will do. I added the braced initializer int constructor test to exercise the 
same code path. But it can't hurt to get this in there


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I think It would be also beneficial to document the semantics. Whenever we say 
`$x + $y` it's not entirely clear whether we talk about the addition operation 
in a mathematical sense or we follow C/C++ language semantics. Right now it's 
mostly mixed within the analyzer. It would be really nice to see for example 
how and when wrapping is considered.
What if the types of the operands don't match in bitwidth or signess. This is 
sort of the reason why `ArrayBoundV2` has false positives in some scenarios.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102696

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

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic added a comment.

Thanks for reviewing @bruno doesn't look like the failure is related to my 
change:
https://buildkite.com/llvm-project/premerge-checks/builds/39905#5f70c261-ae54-451b-b771-7012bcee7387
"No space left on device"

Unless I am looking at the wrong thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102782

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

[PATCH] D102863: [analyzer] RetainCountChecker: Disable tracking for references to OSMetaClass.

[Georgeta Igna via Phabricator via cfe-commits]

georgi_igna updated this revision to Diff 347070.

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

https://reviews.llvm.org/D102863

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/os_object_base.h
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -721,6 +721,16 @@
obj->release();
  }
  
++void test_osmetaclass_release(){
++  const char *name = "no_name";
++  const OSMetaClass *meta = OSMetaClass::copyMetaClassWithName(name);
++  if (!meta) {
++return;
++  } else {
++meta->releaseMetaClass();
++  }
++}
++
  class SampleClass {
  public:
OSObjectPtr field;
Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -68,6 +68,8 @@
  struct OSMetaClass : public OSMetaClassBase {
virtual OSObject * alloc() const;
static OSObject * allocClassWithName(const char * name);
++  static const OSMetaClass * copyMetaClassWithName(const char * name);
++  void releaseMetaClass() const;
virtual ~OSMetaClass(){}
  };
  
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -146,8 +146,15 @@
return !(match(SubclassM, *D, D->getASTContext()).empty());
  }
  
++static bool isExactClass(const Decl *D, StringRef ClassName) {
++  using namespace ast_matchers;
++  DeclarationMatcher sameClassM =
++  cxxRecordDecl(hasName(std::string(ClassName)));
++  return !(match(sameClassM, *D, D->getASTContext()).empty());
++}
++
  static bool isOSObjectSubclass(const Decl *D) {
--  return D && isSubclass(D, "OSMetaClassBase");
++  return D && isSubclass(D, "OSMetaClassBase") && !isExactClass(D, 
"OSMetaClass");
  }
  
  static bool isOSObjectDynamicCast(StringRef S) {


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -721,6 +721,16 @@
obj->release();
  }
  
++void test_osmetaclass_release(){
++  const char *name = "no_name";
++  const OSMetaClass *meta = OSMetaClass::copyMetaClassWithName(name);
++  if (!meta) {
++return;
++  } else {
++meta->releaseMetaClass();
++  }
++}
++
  class SampleClass {
  public:
OSObjectPtr field;
Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -68,6 +68,8 @@
  struct OSMetaClass : public OSMetaClassBase {
virtual OSObject * alloc() const;
static OSObject * allocClassWithName(const char * name);
++  static const OSMetaClass * copyMetaClassWithName(const char * name);
++  void releaseMetaClass() const;
virtual ~OSMetaClass(){}
  };
  
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -146,8 +146,15 @@
return !(match(SubclassM, *D, D->getASTContext()).empty());
  }
  
++static bool isExactClass(const Decl *D, StringRef ClassName) {
++  using namespace ast_matchers;
++  DeclarationMatcher sameClassM =
++  cxxRecordDecl(hasName(std::string(ClassName)));
++  return !(match(sameClassM, *D, D->getASTContext()).empty());
++}
++
  static bool isOSObjectSubclass(const Decl *D) {
--  return D && isSubclass(D, "OSMetaClassBase");
++  return D && isSubclass(D, "OSMetaClassBase") && !isExactClass(D, "OSMetaClass");
  }
  
  static bool isOSObjectDynamicCast(StringRef S) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D101793#2772717 , @yaxunl wrote:

> In D101793#2772461 , @weiwang wrote:
>
>> Thanks for the approval!
>>
>> Just want to understand the list of "decls to check for deferred 
>> diagnostics" better, where are these decls coming from? And why do they need 
>> to be checked for warnings? I see decls from libc are in the list, but I 
>> have no idea why are they selected.
>
> For offloading languages e.g. OpenMP/CUDA/HIP, there are apparent errors in 
> functions shared between host and device. However, unless these functions are 
> sure to be emitted on device or host, these errors should not be emitted. 
> These errors are so called deferred error messages. The function decls which 
> need to be checked are recorded. After AST is finalized, the AST of these 
> functions are iterated. If a function is found sure to be emitted, the 
> deferred error message in it are emitted.

Thanks! So the `DeclsToCheckForDeferredDiags` contains the candidate decls to 
be checked. The decls are selected because they would generate diags in the 
context of offloading languages, but whether or not an diag will be emitted is 
deferred till traversal of the AST is performed. I still don't quite understand 
why would libc functions be in the candidate list? They look simple enough (and 
I think they've been there forever).  For example `__uint16_identity` 
(https://code.woboq.org/userspace/glibc/bits/uintn-identity.h.html#32),

  static inline __uint16_t __uint16_identity(__uint16_t __x) {
  return __x;
  }

I don't see how this function would generate diag either on host or device.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 347069.
ldionne marked an inline comment as done.
ldionne added a comment.
Herald added a project: clang.

Rebase onto main and apply Aaron's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91630

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/SemaCXX/cxx11-attributes-on-using-declaration.cpp

Index: clang/test/SemaCXX/cxx11-attributes-on-using-declaration.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx11-attributes-on-using-declaration.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -pedantic -triple x86_64-apple-macos11 -std=c++20 -fsyntax-only -verify %s
+
+static_assert(__has_extension(cxx_attributes_on_using_declarations), "");
+
+namespace NS { typedef int x; }
+
+[[clang::annotate("foo")]] using NS::x; // expected-warning{{ISO C++ does not allow an attribute list to appear here}}
+
+
+[[deprecated]] using NS::x; // expected-warning {{'deprecated' currently has no effect on using-declarations}} expected-warning{{ISO C++ does not allow}}
+using NS::x [[deprecated]]; // expected-warning {{'deprecated' currently has no effect on using-declarations}} expected-warning{{ISO C++ does not allow}}
+using NS::x __attribute__((deprecated)); // expected-warning {{'deprecated' currently has no effect on using-declarations}}
+using NS::x __attribute__((availability(macos,introduced=1))); // expected-warning {{'availability' currently has no effect on using-declarations}}
+
+[[clang::availability(macos,introduced=1)]] using NS::x; // expected-warning {{'availability' currently has no effect on using-declarations}} expected-warning{{ISO C++ does not allow}}
+
+// expected-warning@+1 3 {{ISO C++ does not allow an attribute list to appear here}}
+[[clang::annotate("A")]] using NS::x [[clang::annotate("Y")]], NS::x [[clang::annotate("Z")]];
+
+template 
+struct S : T {
+  [[deprecated]] using typename T::x; // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on using-declarations}}
+  [[deprecated]] using T::y; // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on using-declarations}}
+
+  using typename T::z [[deprecated]]; // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on using-declarations}}
+  using T::a [[deprecated]]; // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on using-declarations}}
+};
+
+struct Base {};
+
+template 
+struct DepBase1 : B {
+  using B::B [[]];
+
+};
+template 
+struct DepBase2 : B {
+  using B::B __attribute__(());
+};
+
+DepBase1 db1;
+DepBase2 db2;
Index: clang/test/Parser/cxx0x-attributes.cpp
===
--- clang/test/Parser/cxx0x-attributes.cpp
+++ clang/test/Parser/cxx0x-attributes.cpp
@@ -131,12 +131,12 @@
 [[]] static_assert(true, ""); //expected-error {{an attribute list cannot appear here}}
 [[]] asm(""); // expected-error {{an attribute list cannot appear here}}
 
-[[]] using ns::i; // expected-error {{an attribute list cannot appear here}}
+[[]] using ns::i;
 [[unknown]] using namespace ns; // expected-warning {{unknown attribute 'unknown' ignored}}
 [[noreturn]] using namespace ns; // expected-error {{'noreturn' attribute only applies to functions}}
 namespace [[]] ns2 {} // expected-warning {{attributes on a namespace declaration are a C++17 extension}}
 
-using [[]] alignas(4) [[]] ns::i; // expected-error {{an attribute list cannot appear here}}
+using [[]] alignas(4) [[]] ns::i; // expected-error {{an attribute list cannot appear here}} expected-error {{'alignas' attribute only applies to variables, data members and tag types}} expected-warning {{ISO C++}}
 using [[]] alignas(4) [[]] foobar = int; // expected-error {{an attribute list cannot appear here}} expected-error {{'alignas' attribute only applies to}}
 
 void bad_attributes_in_do_while() {
@@ -157,7 +157,16 @@
 [[]] using T = int; // expected-error {{an attribute list cannot appear here}}
 using T [[]] = int; // ok
 template using U [[]] = T;
-using ns::i [[]]; // expected-error {{an attribute list cannot appear here}}
+using ns::i [[]];
+using ns::i [[]], ns::i [[]]; // expected-warning {{use of multiple declarators in a single using declaration is a C++17 extension}}
+struct using_in_struct_base {
+  typedef int i, j, k, l;
+};
+struct using_in_struct : using_in_struct_base {
+  

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D102742#2773954 , @tejohnson wrote:

> In D102742#2767569 , 
> @nickdesaulniers wrote:
>
>> Obviously needs work/cleanup, changes to x86, and tests, but posting for 
>> early feedback about module level attributes vs function level attributes, 
>> or possibly something else.  I tested this quickly with thin LTO of the 
>> Linux kernel and it worked.
>
> I haven't looked through in too much detail, but I see you have the module 
> flag type set as Warning on conflict. I guess the answer to module level vs 
> function level depends on whether it is valid to link together files compiled 
> with different values of these flags and if so what the expected behavior 
> should be. How does this work on non-LTO? If it works just fine, i.e. the 
> functions from the modules with one value are compiled ok with that value and 
> linked with functions compiled effectively with another value, then that 
> would point to a function attribute so you can mimic the mix-and-match 
> behavior. If it is unexpected, then perhaps better to keep as a module flag 
> but Error? With Warning, the value from the first module being linked is used 
> - would that be surprising?

Good point; it won't work as intended on mismatch. Let me upgrade that to Error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Nick Desaulniers via Phabricator via cfe-commits]

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

- upgrade module merge strategy to Error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector-guard.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
  llvm/test/CodeGen/X86/stack-protector-3.ll

Index: llvm/test/CodeGen/X86/stack-protector-3.ll
===
--- llvm/test/CodeGen/X86/stack-protector-3.ll
+++ llvm/test/CodeGen/X86/stack-protector-3.ll
@@ -1,10 +1,18 @@
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=tls -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard=global -o - < %s | FileCheck --check-prefix=CHECK-GLOBAL %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=fs -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=gs -o - < %s | FileCheck --check-prefix=CHECK-GS %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=20 -o - < %s | FileCheck --check-prefix=CHECK-OFFSET %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=-20 -o - < %s | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: cat %t/main.ll %t/c.ll > %t/c2.ll
+; RUN: cat %t/main.ll %t/d.ll > %t/d2.ll
+; RUN: cat %t/main.ll %t/e.ll > %t/e2.ll
+; RUN: cat %t/main.ll %t/f.ll > %t/f2.ll
+; RUN: cat %t/main.ll %t/g.ll > %t/g2.ll
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/a2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/b2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/c2.ll | FileCheck --check-prefix=CHECK-GLOBAL %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/d2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/e2.ll | FileCheck --check-prefix=CHECK-GS %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/f2.ll | FileCheck --check-prefix=CHECK-OFFSET %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/g2.ll | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
 
 ; CHECK-TLS-FS-40:   movq%fs:40, %rax
 ; CHECK-TLS-FS-40:   movq%fs:40, %rax
@@ -47,6 +55,8 @@
 ; CHECK-GLOBAL-NEXT:  .cfi_def_cfa_offset 32
 ; CHECK-GLOBAL-NEXT:  callq   __stack_chk_fail
 
+;--- main.ll
+
 ; ModuleID = 't.c'
 @.str = private unnamed_addr constant [14 x i8] c"stackoverflow\00", align 1
 @a = dso_local local_unnamed_addr global i8* null, align 8
@@ -75,3 +85,23 @@
 attributes #0 = { nounwind sspreq uwtable writeonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #1 = { argmemonly nounwind willreturn }
 attributes #2 = { nounwind }
+
+;--- a.ll
+;--- b.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+;--- c.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard", !"global"}
+;--- d.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-reg", !"fs"}
+;--- e.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-reg", !"gs"}
+;--- f.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-offset", i32 20}
+;--- g.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-offset", i32 -20}
Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -1,46 +1,39 @@
-; RUN: llc %s --stack-protector-guard=sysreg \
-; RUN:   --stack-protector-guard-reg=sp_el0 \
-; RUN:   --stack-protector-guard-offset=0 -verify-machineinstrs -o - | \
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll 

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D102696#2773340 , @vsavchenko 
wrote:

> In D102696#2773304 , @martong wrote:
>
>>> As for the solver, it is something that tormented me for a long time. Is 
>>> there a way to avoid a full-blown brute force check of all existing 
>>> constraints and get some knowledge about symbolic expressions by 
>>> constraints these symbolic expressions are actually part of (right now we 
>>> can reason about expressions if we know some information about its own 
>>> parts aka operands)?
>>
>> Well, it is a hard question.
>> I've been thinking about building a "parent" map for the sub-expressions, 
>> like we do in the AST (see clang::ParentMap). We could use this parent map 
>> to inject new constraints during the "constant folding" mechanism.
>> So, let's say we have `$x + $y = 0` and then when we process `$y = 0` then 
>> we'd add a new constraint: `$x = 0`. We could add this new constraint by 
>> knowing that we have to visit `$x + $y` because `$y` is connected to that in 
>> the parent map.
>> What do you think, could it work?
>
> Yes, this was exactly my line of thinking.  Instead of trying to use hard 
> logic every time we are asked a question, we can try to simplify existing 
> constraints based on the new bit of information.
> The main problem with this is the tree nature of symbolic expressions.  If we 
> have something trivial like `$x + $y` - sure.  But something with lot more 
> nested subexpressions can get tricky really fast.  That can be solved if we 
> have a canonical form for the trees (this will resolve the problem with `$x + 
> $y` versus `$y + $x` as well).  Right now, we have bits of this all around 
> our codebase, but those are more like workarounds as opposed to one general 
> approach.

OK, I have started  working on a constraint manager based prototype 
implementation with a parent map and a hook in `setConstraint`. The test file 
in this patch (even the commutativity test) passes with that, so, it looks 
promising. I hope I can share that next week. Also, I'd like do some 
performance measurements as well, both on this patch and for the new solver 
based one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102696

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

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

This looks like a good start..

All your tests are 3x3 have you considered mixing it up a bit.  i.e. 2x3, what 
is the impact on 1x5 and 5x1 ?

Also how about nested structs, I'm interested to see what happens

  {56, 23, { "ABC", 35 }}
  {57, 24, { "DEF", 36 }}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D102736: Fix tmp files being left on Windows builds.

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

Cool!  This is getting much simpler.  My remaining comments are mostly musings 
and you can tell me to buzz off.

But I'd like to see @aganea's questions addressed.  It does seem redundant to 
have to keep the file name when we already have an object that represents the 
file.




Comment at: llvm/lib/Support/Path.cpp:1237
   RenameEC = copy_file(TmpName, Name);
   setDeleteDisposition(H, true);
 }

I'm curious if this path has ever been exercised.

I see that rename_handle is implemented with MoveFileExW on Windows.  
MoveFileExW docs say it will fail if you're trying to move a _directory_ to 
another device, but that it can do copy+delete to move a _file_ to another 
device.  (That might require adding MOVEFILE_COPY_ALLOWED to the flags passed 
in the MoveFileExW in lib\Support\Windows\Path.inc near line 485.)

Anyway, it just seems like we're re-implementing functionality the OS calls can 
already do for us.

https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexw



Comment at: llvm/lib/Support/Path.cpp:1243
   if (RenameEC)
 setDeleteDisposition(H, true);
 #else

If I understand correctly, we're using the delete disposition flag to get the 
OS to clean up the file in case we crash.  But does that prevent us from just 
deleting it outright once we know it's safe to do so?

In other words if we could just delete the file here, then the divergence 
between Win32 and others could possibly be reduced.



Comment at: llvm/lib/Support/Path.cpp:1253
   }
   sys::DontRemoveFileOnSignal(TmpName);
 #endif

Possibly stupid idea:  What if RemoveFileOnSignal and DontRemoveFileOnSignal 
did the Win32-specific delete disposition flag twiddling?  With that 
encapsulated, and assuming we can still explicitly delete a Windows file that's 
already marked for deletion, it seems like all of the special handling here 
could go away.

Then again, the ...OnSignal things are in sys rather than sys::fs, so maybe 
it's not such a good idea.

I'm not trying to make more work for Amy.  The immediate goal probably isn't 
well served by this level of overthinking.  But it would be nice to see all 
this get simpler in the long term.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102736

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

[PATCH] D102494: [Clang, Driver] Default to Darwin_libsystem_m veclib on iOS based targets.

[Florian Hahn via Phabricator via cfe-commits]

fhahn planned changes to this revision.
fhahn added a comment.

We are still evaluating whether it is safe to use this as default. I'll mark it 
as 'changes planned' for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102494

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

[PATCH] D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr

[Balázs Benics via Phabricator via cfe-commits]

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

In D102835#2773818 , 
@tomasz-kaminski-sonarsource wrote:

> I do not have commit rights. If there are no additional changes needed, would 
> it be possible to you to commit the changes on my behalf?

I wanted to wait for a couple of days to see if anyone else has something to 
say.
I'm gonna commit your change on your behalf on Monday if it's good for you.


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

https://reviews.llvm.org/D102835

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

Please have a look at the other 2 patches which are now a dependency for this. 
Once you're happy with all 3 I'll push them all together.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102772

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

[Marco Elver via Phabricator via cfe-commits]

melver added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:757-760
+bool HaveSanitizeCoverage =
+CGM.getCodeGenOpts().SanitizeCoverageType ||
+CGM.getCodeGenOpts().SanitizeCoverageIndirectCalls ||
+CGM.getCodeGenOpts().SanitizeCoverageTraceCmp;

vitalybuka wrote:
> consider adding method into CodeGenOptions and replace here and and the rest 
> ideally in a separate patch
> 
https://reviews.llvm.org/D102927



Comment at: clang/test/CodeGen/sanitize-coverage.c:23-87
+static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) 
{
+  if (n)
+x[n] = 42;
+}
+// CHECK-LABEL: define dso_local void @test_always_inline(
+void test_always_inline(int n) {
+  // CHECK-DAG: call void @__sanitizer_cov_trace_pc

melver wrote:
> vitalybuka wrote:
> > you are adding tests for unrelated code.
> > Could you please land is as a separate NFC patch first?
> Which bits in particular? Just the 
> 
> 
> ```
> static inline __attribute__((__always_inline__)) void always_inlined_fn(int 
> n) {
>   if (n)
> x[n] = 42;
> }
> // CHECK-LABEL: define dso_local void @test_always_inline(
> void test_always_inline(int n) {
>   // CHECK-DAG: call void @__sanitizer_cov_trace_pc
>   // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
>   always_inlined_fn(n);
> }
> ```
> 
> part?
https://reviews.llvm.org/D102929



Comment at: llvm/lib/AsmParser/LLLexer.cpp:674
   KEYWORD(nounwind);
+  KEYWORD(no_sanitize_coverage);
   KEYWORD(null_pointer_is_valid);

vitalybuka wrote:
> looking at the rest, seems "nosanitize_coverage" follows pattern better :)
I wasn't sure myself, it's a bit inconsistent with "no_sanitize", but I've 
changed it. ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102772

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 347062.
melver marked 3 inline comments as done.
melver added a comment.

- Refactor test and SanitizeCoverage* checking.
- s/no_sanitize_coverage/nosanitize_coverage/

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102772

Files:
  clang/docs/SanitizerCoverage.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/sanitize-coverage.c
  llvm/bindings/go/llvm/ir_test.go
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Bitcode/compatibility.ll
  llvm/utils/emacs/llvm-mode.el
  llvm/utils/llvm.grm
  llvm/utils/vim/syntax/llvm.vim
  llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Index: llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
===
--- llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
+++ llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
@@ -237,6 +237,7 @@
 \\bnosync\\b|\
 \\bnoundef\\b|\
 \\bnounwind\\b|\
+\\bnosanitize_coverage\\b|\
 \\bnull_pointer_is_valid\\b|\
 \\boptforfuzzing\\b|\
 \\boptnone\\b|\
Index: llvm/utils/vim/syntax/llvm.vim
===
--- llvm/utils/vim/syntax/llvm.vim
+++ llvm/utils/vim/syntax/llvm.vim
@@ -138,6 +138,7 @@
   \ nosync
   \ noundef
   \ nounwind
+  \ nosanitize_coverage
   \ null_pointer_is_valid
   \ optforfuzzing
   \ optnone
Index: llvm/utils/llvm.grm
===
--- llvm/utils/llvm.grm
+++ llvm/utils/llvm.grm
@@ -176,6 +176,7 @@
  | sanitize_thread
  | sanitize_memory
  | mustprogress
+ | nosanitize_coverage
  ;
 
 OptFuncAttrs  ::= + _ | OptFuncAttrs FuncAttr ;
Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -25,7 +25,7 @@
'("alwaysinline" "argmemonly" "allocsize" "builtin" "cold" "convergent" "dereferenceable" "dereferenceable_or_null" "hot" "inaccessiblememonly"
  "inaccessiblemem_or_argmemonly" "inalloca" "inlinehint" "jumptable" "minsize" "mustprogress" "naked" "nobuiltin" "nonnull"
  "nocallback" "nocf_check" "noduplicate" "nofree" "noimplicitfloat" "noinline" "nomerge" "nonlazybind" "noprofile" "noredzone" "noreturn"
- "norecurse" "nosync" "noundef" "nounwind" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
+ "norecurse" "nosync" "noundef" "nounwind" "nosanitize_coverage" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
  "shadowcallstack" "speculatable" "speculative_load_hardening" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "swifterror" "uwtable" "vscale_range" "willreturn" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
Index: llvm/test/Bitcode/compatibility.ll
===
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/Bitcode/compatibility.ll
@@ -1510,7 +1510,7 @@
   ; CHECK: select <2 x i1> , <2 x i8> , <2 x i8> 
 
   call void @f.nobuiltin() builtin
-  ; CHECK: call void @f.nobuiltin() #44
+  ; CHECK: call void @f.nobuiltin() #45
 
   call fastcc noalias i32* @f.noalias() noinline
   ; CHECK: call fastcc noalias i32* @f.noalias() #12
@@ -1904,6 +1904,9 @@
   ret void
 }
 
+declare void @f.nosanitize_coverage() nosanitize_coverage
+; CHECK: declare void @f.nosanitize_coverage() #44
+
 ; immarg attribute
 declare void @llvm.test.immarg.intrinsic(i32 immarg)
 ; CHECK: declare void @llvm.test.immarg.intrinsic(i32 immarg)
@@ -1961,7 +1964,8 @@
 ; CHECK: attributes #41 = { writeonly }
 ; CHECK: attributes #42 = { speculatable }
 ; CHECK: attributes #43 = { strictfp }
-; CHECK: attributes #44 = { builtin }
+; CHECK: attributes #44 = { nosanitize_coverage }
+; CHECK: attributes #45 = { builtin }
 
 ;; Metadata
 
Index: llvm/test/Bitcode/attributes.ll

[PATCH] D102929: [NFC][SanitizeCoverage] Test always_inline functions work

[Marco Elver via Phabricator via cfe-commits]

melver created this revision.
melver added a reviewer: vitalybuka.
melver requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Test that always_inline functions are instrumented as expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102929

Files:
  clang/test/CodeGen/sanitize-coverage.c


Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -19,4 +19,16 @@
   if (n)
 x[n] = 42;
 }
+
+static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) 
{
+  if (n)
+x[n] = 42;
+}
+// CHECK-LABEL: define dso_local void @test_always_inline(
+void test_always_inline(int n) {
+  // CHECK-DAG: call void @__sanitizer_cov_trace_pc
+  // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
+  always_inlined_fn(n);
+}
+
 // CHECK-LABEL: declare void


Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -19,4 +19,16 @@
   if (n)
 x[n] = 42;
 }
+
+static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) {
+  if (n)
+x[n] = 42;
+}
+// CHECK-LABEL: define dso_local void @test_always_inline(
+void test_always_inline(int n) {
+  // CHECK-DAG: call void @__sanitizer_cov_trace_pc
+  // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
+  always_inlined_fn(n);
+}
+
 // CHECK-LABEL: declare void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102927: [NFC][CodeGenOptions] Refactor checking SanitizeCoverage options

[Marco Elver via Phabricator via cfe-commits]

melver created this revision.
melver added a reviewer: vitalybuka.
Herald added a subscriber: dexonsmith.
melver requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Refactor checking SanitizeCoverage options into
CodeGenOptions::hasSanitizeCoverage().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102927

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -736,9 +736,7 @@
addBoundsCheckingPass);
   }
 
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
+  if (CodeGenOpts.hasSanitizeCoverage()) {
 PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
addSanitizerCoveragePass);
 PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
@@ -1108,9 +1106,7 @@
   const LangOptions , PassBuilder ) {
   PB.registerOptimizerLastEPCallback([&](ModulePassManager ,
  PassBuilder::OptimizationLevel Level) 
{
-if (CodeGenOpts.SanitizeCoverageType ||
-CodeGenOpts.SanitizeCoverageIndirectCalls ||
-CodeGenOpts.SanitizeCoverageTraceCmp) {
+if (CodeGenOpts.hasSanitizeCoverage()) {
   auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
   MPM.addPass(ModuleSanitizerCoveragePass(
   SancovOpts, CodeGenOpts.SanitizeCoverageAllowlistFiles,
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -452,6 +452,12 @@
   bool hasMaybeUnusedDebugInfo() const {
 return getDebugInfo() >= codegenoptions::UnusedTypeInfo;
   }
+
+  // Check if any one of SanitizeCoverage* is enabled.
+  bool hasSanitizeCoverage() const {
+return SanitizeCoverageType || SanitizeCoverageIndirectCalls ||
+   SanitizeCoverageTraceCmp;
+  }
 };
 
 }  // end namespace clang


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -736,9 +736,7 @@
addBoundsCheckingPass);
   }
 
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
+  if (CodeGenOpts.hasSanitizeCoverage()) {
 PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
addSanitizerCoveragePass);
 PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
@@ -1108,9 +1106,7 @@
   const LangOptions , PassBuilder ) {
   PB.registerOptimizerLastEPCallback([&](ModulePassManager ,
  PassBuilder::OptimizationLevel Level) {
-if (CodeGenOpts.SanitizeCoverageType ||
-CodeGenOpts.SanitizeCoverageIndirectCalls ||
-CodeGenOpts.SanitizeCoverageTraceCmp) {
+if (CodeGenOpts.hasSanitizeCoverage()) {
   auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
   MPM.addPass(ModuleSanitizerCoveragePass(
   SancovOpts, CodeGenOpts.SanitizeCoverageAllowlistFiles,
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -452,6 +452,12 @@
   bool hasMaybeUnusedDebugInfo() const {
 return getDebugInfo() >= codegenoptions::UnusedTypeInfo;
   }
+
+  // Check if any one of SanitizeCoverage* is enabled.
+  bool hasSanitizeCoverage() const {
+return SanitizeCoverageType || SanitizeCoverageIndirectCalls ||
+   SanitizeCoverageTraceCmp;
+  }
 };
 
 }  // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you clang-format the tests please so I can more easily read them.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2790
 
+**LambdaBodyIndentation** (``LambdaBodyIndentationKind``)
+  What is the lambda body indented relative to (default is ``Signature``).

Did you generate this by hand or using the clang/doc/tools/dump_style.py?


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

https://reviews.llvm.org/D102706

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

[PATCH] D100794: [HIP] Support overloaded math functions for hipRTC

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 added inline comments.



Comment at: clang/lib/Headers/__clang_hip_cmath.h:20
 #include 
 #include 
 #include 

we may not need to `#include` limits and/or type_traits if we replaced them 
with `__hip::is_integral` and `__hip::is_arithmetic`.



Comment at: clang/lib/Headers/__clang_hip_cmath.h:587
 #endif
-#endif
+#endif // !defined(__HIPCC_RTC__)
 

I don't think this `#endif` is for `!defined(__HIPCC_RTC__)`. It should be for 
`_LIBCPP_BEGIN_NAMESPACE_STD`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100794

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

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Is ```Signature``` effectively as is?


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

https://reviews.llvm.org/D102706

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

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/pre-ra-sched.c:1-2
+// RUN: %clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: %clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+

TaoPan wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > The test as was committed is bogus. It checks the output binary(!) stream 
> > > for an error message when stderr was not even redirected. Please fix.
> > Fix committed: 
> > https://github.com/llvm/llvm-project/commit/603818b97c795114f66a6fc13e8a5f0e54b49a13
> Thanks for your review comment and fixing!
> I tried to check stderr as you guided and meet empty stdin problem, thanks 
> for your quick fixing and I got FileCheck option --allow-empty can resolve 
> this kind of problem. 
No problem. Thanks for looking into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

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

[PATCH] D102730: [clang-format] Support custom If macros

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:252
+- Option ``IfMacros`` has been added. This lets you define macros that get
+  formatted like conditionals much like ``ForEachMacros`` get stiled like
+  foreach loops.

stiled? did you mean styled?



Comment at: clang/lib/Format/FormatTokenLexer.cpp:1019
   FormatTok->setType(it->second);
+  if (it->second == TT_IfMacro) {
+FormatTok->Tok.setKind(tok::kw_if);

Is there any chance you could leave a comment here as to why this is needed, I 
can't work it out?



Comment at: clang/lib/Format/TokenAnnotator.cpp:3025
   return false;
+if (Left.is(TT_IfMacro)) {
+  return false;

I'll let you decide if you think we need another SBPO_XXX style?



Comment at: clang/unittests/Format/FormatTest.cpp:19699
   verifyFormat("if constexpr ( a )\n  return;", Spaces);
+  verifyFormat("MYIF( a )\n  return;", Spaces);
+  verifyFormat("MYIF( a )\n  return;\nelse MYIF( b )\n  return;", Spaces);

this should support ```MYIF(```  by processing 
SpacesInConditionalStatement  as we do with FOREACH


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

https://reviews.llvm.org/D102730

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

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D102742#2767569 , @nickdesaulniers 
wrote:

> Obviously needs work/cleanup, changes to x86, and tests, but posting for 
> early feedback about module level attributes vs function level attributes, or 
> possibly something else.  I tested this quickly with thin LTO of the Linux 
> kernel and it worked.

I haven't looked through in too much detail, but I see you have the module flag 
type set as Warning on conflict. I guess the answer to module level vs function 
level depends on whether it is valid to link together files compiled with 
different values of these flags and if so what the expected behavior should be. 
How does this work on non-LTO? If it works just fine, i.e. the functions from 
the modules with one value are compiled ok with that value and linked with 
functions compiled effectively with another value, then that would point to a 
function attribute so you can mimic the mix-and-match behavior. If it is 
unexpected, then perhaps better to keep as a module flag but Error? With 
Warning, the value from the first module being linked is used - would that be 
surprising?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102742

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

[PATCH] D100733: [clang] NFC: change uses of `Expr->getValueKind` into `is?Value`

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In D100733#2760890 , @mizvekov wrote:

> - I still think a new value category is simpler than the two phase overload 
> resolution it replaces :D

Not sure if it's simpler, but it'll produce less confusing results.

> - This new value category would (so far as this is only an experiment I am 
> running), only apply to standards older than c++23, where from then on it's 
> just the current categories and the rules as in P2266 
> .
> - Even in the old standards it replaces, it does not introduce any drastic 
> changes. This new category is the same as XValues as far as overload 
> resolution is concerned (with the small addition that it binds less 
> preferentially to non-const l-value references), and as far as type deduction 
> goes, it behaves exactly as LValues.

A new value category feels like a global change for a local problem. We can 
explain the behavior that we want without introducing a new value category: 
either as a “combined” overload resolution for xvalue and lvalue, where all 
candidates for the xvalue are strictly better than any candidate for the 
lvalue, or as a two-phase resolution that falls back only if there are no 
candidates. These explanations are equivalent.

In D100733#2761031 , @rsmith wrote:

> What do we think about renaming `isRValue()` to `isPRValue()` and renaming 
> `VK_RValue` to `VK_PRValue`, adding a "real" `isRValue()`, and then 
> performing this cleanup?

One problem might be that C and C++ have different terminology, right? I like 
the idea of going with prvalue instead of rvalue, then it's the right thing for 
C++ and if you're coming from the C world it's at least not confusing. But 
perhaps I would not introduce for now an `isRValue` function. (Although C 
doesn't have xvalues, so maybe it doesn't matter?)

In D100733#2761052 , @mizvekov wrote:

> Is there any special reason we ended up in this state of affairs with the 
> swapped out names?

My impression is that some people didn't like the changes that came with C++11 
and that it made the terminology of C and C++ somewhat inconsistent. (Though I 
think you can work with the C++ terminology in C, there are just no xvalues, 
but you can't work the other way around.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100733

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

[PATCH] D102924: [clang] NFC: Replace std::pair by a struct in InitHeaderSearch

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch replaces a `std::pair` by a proper struct in `InitHeaderSearch`. 
This will be useful in a follow-up: D102923 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102924

Files:
  clang/lib/Frontend/InitHeaderSearch.cpp


Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -32,14 +32,20 @@
 using namespace clang::frontend;
 
 namespace {
+/// Holds information about a single include path.
+struct IncludePathInfo {
+  IncludeDirGroup Group;
+  DirectoryLookup Path;
+
+  IncludePathInfo(IncludeDirGroup Group, DirectoryLookup Path)
+  : Group(Group), Path(Path) {}
+};
 
 /// InitHeaderSearch - This class makes it easier to set the search paths of
 ///  a HeaderSearch object. InitHeaderSearch stores several search path lists
 ///  internally, which can be sent to a HeaderSearch object in one swoop.
 class InitHeaderSearch {
-  std::vector > IncludePath;
-  typedef std::vector >::const_iterator path_iterator;
+  std::vector IncludePath;
   std::vector > SystemHeaderPrefixes;
   HeaderSearch 
   bool Verbose;
@@ -154,8 +160,7 @@
 
   // If the directory exists, add it.
   if (auto DE = FM.getOptionalDirectoryRef(MappedPathStr)) {
-IncludePath.push_back(
-  std::make_pair(Group, DirectoryLookup(*DE, Type, isFramework)));
+IncludePath.emplace_back(Group, DirectoryLookup(*DE, Type, isFramework));
 return true;
   }
 
@@ -165,9 +170,8 @@
 if (auto FE = FM.getFile(MappedPathStr)) {
   if (const HeaderMap *HM = Headers.CreateHeaderMap(*FE)) {
 // It is a headermap, add it to the search path.
-IncludePath.push_back(
-  std::make_pair(Group,
- DirectoryLookup(HM, Type, Group == IndexHeaderMap)));
+IncludePath.emplace_back(
+Group, DirectoryLookup(HM, Type, Group == IndexHeaderMap));
 return true;
   }
 }
@@ -558,32 +562,32 @@
 
   // Quoted arguments go first.
   for (auto  : IncludePath)
-if (Include.first == Quoted)
-  SearchList.push_back(Include.second);
+if (Include.Group == Quoted)
+  SearchList.push_back(Include.Path);
 
   // Deduplicate and remember index.
   RemoveDuplicates(SearchList, 0, Verbose);
   unsigned NumQuoted = SearchList.size();
 
   for (auto  : IncludePath)
-if (Include.first == Angled || Include.first == IndexHeaderMap)
-  SearchList.push_back(Include.second);
+if (Include.Group == Angled || Include.Group == IndexHeaderMap)
+  SearchList.push_back(Include.Path);
 
   RemoveDuplicates(SearchList, NumQuoted, Verbose);
   unsigned NumAngled = SearchList.size();
 
   for (auto  : IncludePath)
-if (Include.first == System || Include.first == ExternCSystem ||
-(!Lang.ObjC && !Lang.CPlusPlus && Include.first == CSystem) ||
+if (Include.Group == System || Include.Group == ExternCSystem ||
+(!Lang.ObjC && !Lang.CPlusPlus && Include.Group == CSystem) ||
 (/*FIXME !Lang.ObjC && */ Lang.CPlusPlus &&
- Include.first == CXXSystem) ||
-(Lang.ObjC && !Lang.CPlusPlus && Include.first == ObjCSystem) ||
-(Lang.ObjC && Lang.CPlusPlus && Include.first == ObjCXXSystem))
-  SearchList.push_back(Include.second);
+ Include.Group == CXXSystem) ||
+(Lang.ObjC && !Lang.CPlusPlus && Include.Group == ObjCSystem) ||
+(Lang.ObjC && Lang.CPlusPlus && Include.Group == ObjCXXSystem))
+  SearchList.push_back(Include.Path);
 
   for (auto  : IncludePath)
-if (Include.first == After)
-  SearchList.push_back(Include.second);
+if (Include.Group == After)
+  SearchList.push_back(Include.Path);
 
   // Remove duplicates across both the Angled and System directories.  GCC does
   // this and failing to remove duplicates across these two groups breaks


Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -32,14 +32,20 @@
 using namespace clang::frontend;
 
 namespace {
+/// Holds information about a single include path.
+struct IncludePathInfo {
+  IncludeDirGroup Group;
+  DirectoryLookup Path;
+
+  IncludePathInfo(IncludeDirGroup Group, DirectoryLookup Path)
+  : Group(Group), Path(Path) {}
+};
 
 /// InitHeaderSearch - This class makes it easier to set the search paths of
 ///  a HeaderSearch object. InitHeaderSearch stores several search path lists
 ///  internally, which can be sent to a HeaderSearch object in one swoop.
 class InitHeaderSearch {
-  std::vector > IncludePath;
-  typedef std::vector >::const_iterator 

[PATCH] D102923: [clang][lex] Remark for used header search paths

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For dependency scanning, it would be useful to collect header search paths 
(provided on command-line via `-I` and friends) that were actually used during 
preprocessing. This patch adds that feature to `HeaderSearch` along with a new 
remark that reports such paths as they get used.

Previous version of this patch tried to use the existing `LookupFileCache` to 
report used paths via `HitIdx`. That doesn't work for `ComputeUserEntryUsage` 
(which is intended to be called *after* preprocessing), because it indexes used 
search paths by the file name. This means the values get overwritten when the 
code contains `#include_next`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102923

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Preprocessor/Inputs/header-search-user-entries/a/a.h
  clang/test/Preprocessor/Inputs/header-search-user-entries/a_next/a.h
  clang/test/Preprocessor/Inputs/header-search-user-entries/b/b.h
  clang/test/Preprocessor/Inputs/header-search-user-entries/d/d.h
  clang/test/Preprocessor/header-search-user-entries.c

Index: clang/test/Preprocessor/header-search-user-entries.c
===
--- /dev/null
+++ clang/test/Preprocessor/header-search-user-entries.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only %s -Rinclude-header-search-usage \
+// RUN:   -I%S/Inputs/header-search-user-entries/a -I%S/Inputs/header-search-user-entries/a_next \
+// RUN:   -I%S/Inputs/header-search-user-entries/b -I%S/Inputs/header-search-user-entries/c \
+// RUN:   -I%S/Inputs/header-search-user-entries/d 2>&1 | FileCheck %s
+
+#include "a.h"
+#include "d.h"
+
+// CHECK: remark: user-provided search path used '{{.*}}/header-search-user-entries/a'
+// CHECK: remark: user-provided search path used '{{.*}}/header-search-user-entries/a_next'
+// CHECK: remark: user-provided search path used '{{.*}}/header-search-user-entries/d'
Index: clang/test/Preprocessor/Inputs/header-search-user-entries/a/a.h
===
--- /dev/null
+++ clang/test/Preprocessor/Inputs/header-search-user-entries/a/a.h
@@ -0,0 +1 @@
+#include_next "a.h"
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -108,6 +108,18 @@
<< NumSubFrameworkLookups << " subframework lookups.\n";
 }
 
+std::vector HeaderSearch::ComputeUserEntryUsage() const {
+  std::vector UserEntryUsage(HSOpts->UserEntries.size());
+  for (unsigned i = 0, e = SearchDirsUsage.size(); i < e; ++i) {
+if (SearchDirsUsage[i]) {
+  auto UserEntryIdxIt = SearchDirToHSEntry.find(i);
+  if (UserEntryIdxIt != SearchDirToHSEntry.end())
+UserEntryUsage[UserEntryIdxIt->second] = true;
+}
+  }
+  return UserEntryUsage;
+}
+
 /// CreateHeaderMap - This method returns a HeaderMap for the specified
 /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
 const HeaderMap *HeaderSearch::CreateHeaderMap(const FileEntry *FE) {
@@ -649,6 +661,17 @@
   return None;
 }
 
+void HeaderSearch::CacheLookupSuccess(LookupFileCacheInfo ,
+  unsigned HitIdx) {
+  CacheLookup.HitIdx = HitIdx;
+  SearchDirsUsage[HitIdx] = true;
+
+  auto UserEntryIdxIt = SearchDirToHSEntry.find(HitIdx);
+  if (UserEntryIdxIt != SearchDirToHSEntry.end())
+Diags.Report(diag::remark_pp_include_header_search_usage)
+<< HSOpts->UserEntries[UserEntryIdxIt->second].Path;
+}
+
 void HeaderSearch::setTarget(const TargetInfo ) {
   ModMap.setTarget(Target);
 }
@@ -987,7 +1010,7 @@
   >getFileEntry(), isAngled, FoundByHeaderMap);
 
 // Remember this location for the next lookup we do.
-CacheLookup.HitIdx = i;
+CacheLookupSuccess(CacheLookup, i);
 return File;
   }
 
@@ -1017,8 +1040,8 @@
 return MSFE;
   }
 
-  LookupFileCacheInfo  = LookupFileCache[Filename];
-  CacheLookup.HitIdx = LookupFileCache[ScratchFilename].HitIdx;
+  CacheLookupSuccess(LookupFileCache[Filename],
+ LookupFileCache[ScratchFilename].HitIdx);
   // FIXME: SuggestedModule.
   return File;
 }
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -36,9 +36,11 @@
 struct IncludePathInfo {
   IncludeDirGroup Group;
   DirectoryLookup Path;
+  Optional UserEntryIdx;
 
-  IncludePathInfo(IncludeDirGroup Group, DirectoryLookup Path)
-  : 

[PATCH] D102488: [clang][deps] Prune unused header search paths

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 marked an inline comment as done.
jansvoboda11 added a comment.

This is now ready for review.




Comment at: clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json:4
+"directory": "DIR",
+"command": "clang -E DIR/header-search-pruning.cpp -Ibegin -I1 -Ia -I3 -I4 
-I5 -I6 -Ib -I8 -Iend DEFINES -fmodules -fcxx-modules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules 
-fmodule-map-file=DIR/module.modulemap",
+"file": "DIR/header-search-pruning.cpp"

The extra numerical include directories here force `AST{Writer,Reader}` to work 
with multi-byte blob, providing extra test coverage.



Comment at: clang/test/ClangScanDeps/header-search-pruning.cpp:27
+// CHECK_A:"-I",
+// CHECK_A-NEXT:   "begin",
+// CHECK_A-NEXT:   "-I",

The `being` and `end` include directories here are sentinels that make it 
easier to make positive/negative occurrence checks with `-NEXT`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102488

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

[PATCH] D102853: [OpenCL] Align definition of __IMAGE_SUPPORT__ feature macro with OpenCL version and __opencl_c_images feature macro definition

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Although I think it would be better if we set `__IMAGE_SUPPORT__` 
conditionally also for earlier OpenCL versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102853

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

[PATCH] D102488: [clang][deps] Prune unused header search paths

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 347042.
jansvoboda11 added a comment.

Split out extra patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102488

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/header-search-pruning/a/a.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/b/b.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/begin/begin.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json
  clang/test/ClangScanDeps/Inputs/header-search-pruning/end/end.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/mod.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/module.modulemap
  clang/test/ClangScanDeps/header-search-pruning.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -163,6 +163,11 @@
 "'-fmodule-file=', '-o', '-fmodule-map-file='."),
 llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt OptimizeArgs(
+"optimize-args",
+llvm::cl::desc("Whether to optimize command-line arguments of modules."),
+llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -357,7 +362,26 @@
 
 private:
   StringRef lookupPCMPath(ModuleID MID) {
-return Modules[IndexedModuleID{MID, 0}].ImplicitModulePCMPath;
+auto PCMPath = PCMPaths.insert({IndexedModuleID{MID, 0}, ""});
+if (PCMPath.second)
+  PCMPath.first->second = constructPCMPath(lookupModuleDeps(MID));
+return PCMPath.first->second;
+  }
+
+  /// Construct a path where to put the explicitly built PCM - essentially the
+  /// path to implicitly built PCM with the context hash replaced by the final
+  /// (potentially modified) context hash.
+  std::string constructPCMPath(const ModuleDeps ) const {
+const std::string  = MD.ImplicitModulePCMPath;
+StringRef Filename = llvm::sys::path::filename(ImplicitPCMPath);
+StringRef ImplicitContextHashPath =
+llvm::sys::path::parent_path(ImplicitPCMPath);
+StringRef ModuleCachePath =
+llvm::sys::path::parent_path(ImplicitContextHashPath);
+
+SmallString<64> ExplicitPCMPath = ModuleCachePath;
+llvm::sys::path::append(ExplicitPCMPath, MD.ID.ContextHash, Filename);
+return std::string(ExplicitPCMPath);
   }
 
   const ModuleDeps (ModuleID MID) {
@@ -395,6 +419,8 @@
   std::mutex Lock;
   std::unordered_map
   Modules;
+  std::unordered_map
+  PCMPaths;
   std::vector Inputs;
 };
 
@@ -554,7 +580,7 @@
   SharedStream DependencyOS(llvm::outs());
 
   DependencyScanningService Service(ScanMode, Format, ReuseFileManager,
-SkipExcludedPPRanges);
+SkipExcludedPPRanges, OptimizeArgs);
   llvm::ThreadPool Pool(llvm::hardware_concurrency(NumThreads));
   std::vector> WorkerTools;
   for (unsigned I = 0; I < Pool.getThreadCount(); ++I)
Index: clang/test/ClangScanDeps/header-search-pruning.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/header-search-pruning.cpp
@@ -0,0 +1,85 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -r %S/Inputs/header-search-pruning/* %t
+// RUN: cp %S/header-search-pruning.cpp %t/header-search-pruning.cpp
+// RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_A|g" %S/Inputs/header-search-pruning/cdb.json > %t/cdb_a.json
+// RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_B|g" %S/Inputs/header-search-pruning/cdb.json > %t/cdb_b.json
+// RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_A -DINCLUDE_B|g" %S/Inputs/header-search-pruning/cdb.json > %t/cdb_ab.json
+//
+// RUN: clang-scan-deps -compilation-database %t/cdb_a.json -format experimental-full -optimize-args >> %t/result_a.json
+// RUN: cat %t/result_a.json | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK_A %s
+//
+// RUN: clang-scan-deps -compilation-database %t/cdb_b.json -format experimental-full -optimize-args >> %t/result_b.json

[PATCH] D99683: [HIP] Support ThinLTO

[Teresa Johnson via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D99683

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

[PATCH] D102850: [C++4OpenCL] Fix overloading resolution of addrspace constructors

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:9870
 
+  if (S.getLangOpts().OpenCL) {
+if (const auto *CD1 = 
dyn_cast_or_null(Cand1.Function)) {

I think we should remove the OpenCL check since it is not OpenCL specific rule 
and you are using common helpers indeed!

I also wonder if this should be applied to all member functions not only ctors 
since `isBetterOverloadCandidate` should be used for everything?

However it seems that other members are already handled somehow 
https://godbolt.org/z/MrWKPKed7. Do you know where this handling comes from?



Comment at: clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp:14
   // Local variables are handled in local_addrspace_init.clcpp
-  // FIXME: __private and __generic constructors clash for __private variable
-  // X() /*__generic*/ = default;
+  X() /*__generic*/ = default;
   X() __private : x(0) {}

Let's add a `CHECK` to make sure that the overload with the specific address 
space is used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102850

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

[PATCH] D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

I do not have commit rights. If there are no additional changes needed, would 
it be possible to you to commit the changes on my behalf?


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

https://reviews.llvm.org/D102835

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

[PATCH] D93565: scan-view: Remove Reporter.py and associated AppleScript files

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D93565#2773761 , @puremourning 
wrote:

> Erm, doh. I thought I was on main, but I was using clang 12.0.0 (tag 
> llvmorg-12.0.0 from llvm-project). We tend to only use release tagged 
> versions.

OK, I've cherry-picked this fix to the release/12.x branch, so it should be 
working in the 12.0.1 release, and in 12.0.1-rc1 when it comes out if you want 
to verify the fix there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93565

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

[PATCH] D102443: [PowerPC] Added multiple PowerPC builtins

[Quinn Pham via Phabricator via cfe-commits]

quinnp updated this revision to Diff 347029.
quinnp added a comment.

Adding a comment in BuiltinsPPC.def for motivation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102443

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/test/CodeGen/builtins-ppc-xlcompat-sync.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-msync.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync.ll
  llvm/test/CodeGen/PowerPC/eieio.ll

Index: llvm/test/CodeGen/PowerPC/eieio.ll
===
--- llvm/test/CodeGen/PowerPC/eieio.ll
+++ llvm/test/CodeGen/PowerPC/eieio.ll
@@ -4,7 +4,9 @@
 
 define void @eieio_test() {
 ; CHECK-LABEL: @eieio_test
-; CHECK: eieio
+; CHECK: ori r2, r2, 0
+; CHECK-NEXT: ori r2, r2, 0
+; CHECK-NEXT: eieio
 ; CHECK-NEXT: blr
 
 entry:
Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-sync.ll
@@ -0,0 +1,74 @@
+; RUN: llc -verify-machineinstrs -mtriple=powerpcle-unknown-linux-gnu \
+; RUN: -mcpu=pwr8 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-linux-gnu \
+; RUN: -mcpu=pwr8 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -mcpu=pwr8 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -mcpu=pwr8 < %s | FileCheck %s
+
+define dso_local void @test_builtin_ppc_eieio() #0 {
+; CHECK-LABEL: test_builtin_ppc_eieio
+
+entry:
+  call void @llvm.ppc.eieio()
+; CHECK: ori 2, 2, 0
+; CHECK-NEXT: ori 2, 2, 0
+; CHECK-NEXT: eieio
+ 
+  ret void
+}
+
+declare void @llvm.ppc.eieio() #2
+
+define dso_local void @test_builtin_ppc_iospace_eieio() #0 {
+; CHECK-LABEL: test_builtin_ppc_iospace_eieio
+
+entry:
+  call void @llvm.ppc.iospace.eieio()
+; CHECK: ori 2, 2, 0
+; CHECK-NEXT: ori 2, 2, 0
+; CHECK-NEXT: eieio
+ 
+  ret void
+}
+
+declare void @llvm.ppc.iospace.eieio() #2
+
+define dso_local void @test_builtin_ppc_iospace_lwsync() #0 {
+; CHECK-LABEL: test_builtin_ppc_iospace_lwsync
+
+entry:
+  call void @llvm.ppc.iospace.lwsync()
+; CHECK: lwsync
+
+  ret void
+}
+
+declare void @llvm.ppc.iospace.lwsync() #2
+
+define dso_local void @test_builtin_ppc_iospace_sync() #0 {
+; CHECK-LABEL: test_builtin_ppc_iospace_sync
+
+entry:
+  call void @llvm.ppc.iospace.sync()
+; CHECK: sync
+
+  ret void
+}
+
+declare void @llvm.ppc.iospace.sync() #2
+
+define dso_local void @test_builtin_ppc_icbt() #0 {
+; CHECK-LABEL: test_builtin_ppc_icbt
+
+entry:
+  %a = alloca i8*, align 8
+  %0 = load i8*, i8** %a, align 8
+  call void @llvm.ppc.icbt(i8* %0)
+; CHECK: icbt 0, 0, 3
+
+  ret void
+}
+
+declare void @llvm.ppc.icbt(i8*) #2
Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-msync.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-msync.ll
@@ -0,0 +1,33 @@
+; RUN: llc -verify-machineinstrs -mtriple=powerpcle-unknown-linux-gnu \
+; RUN:-mattr=+msync -mcpu=pwr8 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-linux-gnu \
+; RUN:-mattr=+msync -mcpu=pwr8 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:-mattr=+msync -mcpu=pwr8 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:-mattr=+msync -mcpu=pwr8 < %s | FileCheck %s
+
+define dso_local void @test_builtin_ppc_iospace_lwsync() #0 {
+; CHECK-LABEL: test_builtin_ppc_iospace_lwsync
+
+entry:
+  call void @llvm.ppc.iospace.lwsync()
+; CHECK: msync
+
+  ret void
+}
+
+declare void @llvm.ppc.iospace.lwsync() #2
+
+define dso_local void @test_builtin_ppc_iospace_sync() #0 {
+; CHECK-LABEL: test_builtin_ppc_iospace_sync
+
+entry:
+  call void @llvm.ppc.iospace.sync()
+; CHECK: msync
+
+  ret void
+}
+
+declare void @llvm.ppc.iospace.sync() #2
+
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.td
===
--- llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -2044,6 +2044,8 @@
   (DCBTST 0, xoaddr:$dst)>;
 def : Pat<(int_ppc_dcbf xoaddr:$dst),
   (DCBF 0, xoaddr:$dst)>;
+def : Pat<(int_ppc_icbt xoaddr:$dst),
+  (ICBT 0, xoaddr:$dst)>;
 
 def : Pat<(prefetch xoaddr:$dst, (i32 0), imm, (i32 1)),
   (DCBT 0, xoaddr:$dst)>;   // data prefetch for loads
@@ -2565,11 +2567,21 @@
 def EnforceIEIO : XForm_24_eieio<31, 854, (outs), (ins),
  "eieio", IIC_LdStLoad, []>;
 
+def PseudoEIEIO : PPCEmitTimePseudo<(outs), (ins), "#PPCEIEIO",
+  

[PATCH] D93565: scan-view: Remove Reporter.py and associated AppleScript files

[Ben Jackson via Phabricator via cfe-commits]

puremourning added a comment.

Erm, doh. I thought I was on main, but I was using clang 12.0.0 (tag 
llvmorg-10.0.0 from llvm-project). We tend to only use release tagged versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93565

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

[PATCH] D93565: scan-view: Remove Reporter.py and associated AppleScript files

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

Are you still seeing failures in the main branch?  I thought I had fixed this 
with Revision: https://reviews.llvm.org/D96367.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93565

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

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope marked 3 inline comments as done.
bjope added inline comments.



Comment at: llvm/test/Transforms/InstCombine/pow_fp_int16.ll:3
 
-; PR42190
+; Test case was copied from pow_fp_int.ll but adjusted for 16-bit int.
+; Assuming that we can't generate test checks for the same reason (PR42740).

xbolva00 wrote:
> Precommit? And we dont need full copy of existings tests - 2-3 tests for 
> 16bit int are anough.
I've pre-commited the tests now. Remove some of the tests (mainly the ones 
related to i64. But I think most of the others are relevant as regression test 
to see that we get what is expected for the different scenarios also with 
16-bit int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99439

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

[Marco Elver via Phabricator via cfe-commits]

melver added inline comments.



Comment at: clang/test/CodeGen/sanitize-coverage.c:23-87
+static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) 
{
+  if (n)
+x[n] = 42;
+}
+// CHECK-LABEL: define dso_local void @test_always_inline(
+void test_always_inline(int n) {
+  // CHECK-DAG: call void @__sanitizer_cov_trace_pc

vitalybuka wrote:
> you are adding tests for unrelated code.
> Could you please land is as a separate NFC patch first?
Which bits in particular? Just the 


```
static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) {
  if (n)
x[n] = 42;
}
// CHECK-LABEL: define dso_local void @test_always_inline(
void test_always_inline(int n) {
  // CHECK-DAG: call void @__sanitizer_cov_trace_pc
  // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
  always_inlined_fn(n);
}
```

part?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102772

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

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

> Does MS use this in any library headers or such

I didn't check if they do. But if any codebase does use "abstract" and they are 
trying to use clang-cl they would face a parsing error.
My use case was running static analysis on code that uses this feature. Such an 
error had a great impact on the quality of the analysis.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3314
   if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
-VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
-assert((Specifier == VirtSpecifiers::VS_Final ||
-Specifier == VirtSpecifiers::VS_GNU_Final ||
-Specifier == VirtSpecifiers::VS_Sealed) &&
+for (VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
+ Specifier != VirtSpecifiers::VS_None;

hans wrote:
> This 'for' construct is hard to follow. I think it might be easier to follow 
> as a while loop, maybe
> 
> ```
> while ((Specifier = isCXX11VirtSpecifier(Tok)) != VirtSpecifiers::VS_None) {
> ```
> 
> Also the "is" prefix in isCXX11VirtSpecifier is confusing given that it 
> doesn't return a bool.
For "for" vs "while" if you use while you will have to declare the variable 
before the loop giving it the wrong scope which makes it less readable in my 
opinion. 
For  !!isCXX11VirtSpecifier!! I agree but I wouldn't separate the rename from 
this patch(I'm not sure what is the guidelines here)




Comment at: clang/lib/Sema/DeclSpec.cpp:1479
+  case VS_Abstract:
+VS_abstractLoc = Loc;
+break;

hans wrote:
> nit: the other cases just use one line
clang-format doesn't like it. Should I ignore it?



Comment at: clang/lib/Sema/DeclSpec.cpp:1493
   case VS_Sealed: return "sealed";
+  case VS_Abstract:
+return "abstract";

hans wrote:
> nit: skip the newline for consistency here too
same clang-format splits the line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102517

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