[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

In D125340#3506210 , @balazske wrote:

> I found one other place in **LibASTImporter.rst** where `ImportError` is 
> used. LLDB should be checked too.

Yeah , I update the file **LibASTImporter.rst**, I'm not sure if there is need 
to change the ImportError that are used in lldb ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you so much for your review!

My public email address is: `07softy_br...@icloud.com`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428840.
ken-matsui added a comment.

Updated the code as reviewed, added a release note, and merged 
`ext-cpp2b-pp-directive.cpp` & `ext-c2x-pp-directive.c` into 
`ext-pp-directive.c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/Inputs/unsafe-macro-2.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Preprocessor/elifdef.c
  clang/test/Preprocessor/ext-pp-directive.c
  clang/test/Preprocessor/if_warning.c
  clang/test/Preprocessor/ifdef-recover.c
  clang/test/Preprocessor/macro_misc.c
  clang/test/Preprocessor/macro_vaopt_check.cpp

Index: clang/test/Preprocessor/macro_vaopt_check.cpp
===
--- clang/test/Preprocessor/macro_vaopt_check.cpp
+++ clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
-// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -Wno-c2x-extensions -pedantic -std=c99
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
Index: clang/test/Preprocessor/macro_misc.c
===
--- clang/test/Preprocessor/macro_misc.c
+++ clang/test/Preprocessor/macro_misc.c
@@ -4,6 +4,7 @@
 #ifdef defined
 #elifdef defined
 #endif
+// expected-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
 
 
 
Index: clang/test/Preprocessor/ifdef-recover.c
===
--- clang/test/Preprocessor/ifdef-recover.c
+++ clang/test/Preprocessor/ifdef-recover.c
@@ -19,12 +19,14 @@
 #if f(2
 #endif
 
-/* expected-error@+2 {{macro name missing}} */
+/* expected-error@+3 {{macro name missing}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef !
 #endif
Index: clang/test/Preprocessor/if_warning.c
===
--- clang/test/Preprocessor/if_warning.c
+++ clang/test/Preprocessor/if_warning.c
@@ -5,6 +5,7 @@
 #if foo   // expected-error {{'foo' is not defined, evaluates to 0}}
 #endif
 
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef foo
 #elifdef foo
 #endif
@@ -14,6 +15,7 @@
 
 
 // PR3938
+// expected-warning@+3 {{use of a '#elifdef' directive is a C2x extension}}
 #if 0
 #ifdef D
 #elifdef D
Index: clang/test/Preprocessor/ext-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-pp-directive.c
@@ -0,0 +1,59 @@
+// For C
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=pre-c2x-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+// For C++
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// For C
+// pre-c2x-pedantic-warning@#1 {{use of a '#elifdef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// For C
+// pre-c2x-pedantic-warning@#2 {{use of a '#elifndef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#2 {{use of a '#elifndef' directive is 

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips via Phabricator via cfe-commits]

hctim added a comment.

In D123534#3504789 , @rnk wrote:

> This seems reasonable, but I worry about the consequences of creating lots of 
> unnamed global variables. What will gdb do with so many unnamed globals? What 
> will the PDB linker do with all these unnamed globals? I can't answer these 
> questions, and you're welcome to try and find out, but I predict there will 
> be problems, so just be aware of that possibility.

Not sure about PDB. I did run a quick test with `gdb`, and very 
unscientifically, didn't notice any difference in usability or errors between 
pre- and post-this patch on a `clang` invocation under gdb.

> Looking at the S_GDATA32 record format, there is no place for the file & line 
> info:
> https://github.com/microsoft/microsoft-pdb/blob/master/include/cvinfo.h#L3629
> It's just type info & name, really, so there's not much point in creating 
> these when emitting codeview. It's probably best to filter these unnamed 
> globals out in AsmPrinter/CodeViewDebug.cpp rather than changing the IR clang 
> generates.

Sorry, I know pretty nothing about PDB. Are you thinking that these 
DIGlobalVariables should be filtered out by this patch when outputting PDB, or 
a subsequent patch? I don't even have a Windows machine, so I wouldn't be at 
all confident in writing such a patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:36
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifndef'?}}

Here, `#elfindef` is suggested to `#elifdef`, not `#elifndef`, but I believe it 
will help many users to realize that they have typo'd `#elifndef`, or maybe 
they might have meant actually `#elifdef`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428827.
ken-matsui added a comment.

Remove unused includes in `StringRef.h`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c

Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=not-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector ) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string found.
+SimilarStr = {C, CurDist};
+  } else if (CurDist < SimilarStr->second) {
+// More similar string found.
+SimilarStr = {C, CurDist};
+  }
+}
+  }
+
+  if (SimilarStr.hasValue()) {
+return SimilarStr->first;
+  } else {
+return None;
+  }
+}
+
 bool Preprocessor::CheckMacroName(Token , MacroUse isDefineUndef,
   bool *ShadowFlag) {
   // Missing macro name?
@@ -433,6 +478,27 @@
   return BytesToSkip - LengthDiff;
 }
 
+/// SuggestTypoedDirective - Provide a suggestion for a typoed directive. If
+/// there is no typo, 

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your support!

I updated the code, so could you please review this patch again?




Comment at: llvm/lib/Support/StringRef.cpp:102
+// Find a similar string in `Candidates`.
+Optional StringRef::find_similar_str(const std::vector 
, size_t Dist) const {
+  // We need to check if `rng` has the exact case-insensitive string because 
the Levenshtein distance match does not

aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > I am less convinced that we want this as a general interface on 
> > > `StringRef`. I think callers need to decide whether something is similar 
> > > enough or not. For example, case sensitivity may not matter for this case 
> > > but it might matter to another case. Others may wish to limit the max 
> > > edit_distance for each of the candidates for performance reasons, others 
> > > may not. We already saw there were questions about whether to allow 
> > > replacements or not. That sort of thing.
> > > 
> > > I think this functionality should be moved to PPDirectives.cpp as a 
> > > static helper function that's specific to this use. Also, because this 
> > > returns one of the candidates passed in, and those are a `StringRef`, I 
> > > think this function should return a `StringRef` -- this removes 
> > > allocation costs. I'd also drop the `Dist` parameter as the function is 
> > > no longer intended to be a general purpose one.
> > I am also in favor of it, but where should I put tests for this 
> > functionality?
> > Is it possible to write unit tests for static functions implemented in 
> > `.cpp` files?
> This is something we wouldn't usually add tests for directly but instead test 
> the behavior of through lit -- the tests you already have exercise this code 
> path and would show if there's a situation where we expect a viable candidate 
> and don't find any. So I'd not worry about test coverage for it in this case.
I see. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428826.
ken-matsui added a comment.

Update the code as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h

Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=not-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  not-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector ) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string 

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:421
 
 install(
   FILES ${ppc_wrapper_files}

There appear to be two installs of ppc_wrapper_files with different components. 
Is that intentional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[clang] 9519dac - Revert "[NFC][tests][AIX] XFAIL test for lack of visibility support"

[David Tenty via cfe-commits]

Author: David Tenty
Date: 2022-05-11T20:47:48-04:00
New Revision: 9519dacab7b8afd537811fc2abaceb4d14f4e16a

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

LOG: Revert "[NFC][tests][AIX] XFAIL test for lack of visibility support"

This reverts commit f5a9b5cc12658f4d6caa3e0cfc3e771698fb3798 since
https://reviews.llvm.org/D125141 has resolved the test issue.

Added: 


Modified: 
clang/test/OpenMP/target_update_messages.cpp

Removed: 




diff  --git a/clang/test/OpenMP/target_update_messages.cpp 
b/clang/test/OpenMP/target_update_messages.cpp
index d79b57943065..f936a075e1b4 100644
--- a/clang/test/OpenMP/target_update_messages.cpp
+++ b/clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,3 @@
-// Visibility hidden is not currently implemented on AIX.
-// XFAIL: aix
-
 // RUN: %clang_cc1 -verify=expected,lt50,lt51 -fopenmp -fopenmp-version=45 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51 -fopenmp -fopenmp-version=50 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51 -fopenmp -fopenmp-version=51 
-ferror-limit 100 -o - -std=c++11 %s -Wuninitialized



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

[PATCH] D122519: [NFC][tests][AIX] XFAIL test for lack of visibility support

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

Resolved by https://reviews.llvm.org/D125141


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122519

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

[PATCH] D125141: [clang][AIX] Don't ignore XCOFF visibility by default

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

Resolved by https://reviews.llvm.org/D125141


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125141

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/include/clang/AST/Comment.h:303
-
-Argument(SourceRange Range, StringRef Text) : Range(Range), Text(Text) { }
   };

Removing that allows me to build an array without initializing all members 
right away. Alternative would be to add a default constructor here.



Comment at: clang/include/clang/Basic/DiagnosticCommentKinds.td:159
+def warn_doc_inline_command_not_enough_arguments : Warning<
+  "'%select{\\|@}0%1' command has %plural{0:no|:%2}2 word argument%s2, 
expected %3">,
   InGroup, DefaultIgnore;

If you find that `%plural` too fancy, a plain `%2` should also do.



Comment at: clang/lib/AST/CommentParser.cpp:295-307
   typedef BlockCommandComment::Argument Argument;
   Argument *Args =
   new (Allocator.Allocate(NumArgs)) Argument[NumArgs];
   unsigned ParsedArgs = 0;
   Token Arg;
   while (ParsedArgs < NumArgs && Retokenizer.lexWord(Arg)) {
 Args[ParsedArgs] = Argument(SourceRange(Arg.getLocation(),

This is basically what I'm duplicating. As it happens, the two `Argument` 
structures are structurally the same, so we could unify them and factor out 
this code. I'd probably do that in a separate change though (prior to this or 
as follow-up).



Comment at: clang/test/Headers/x86-intrinsics-headers-clean.cpp:4
 // RUN: %clang_cc1 -ffreestanding -triple i386-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual 
-Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual 
-Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers 
-flax-vector-conversions=none -x c++ -verify %s

@RKSimon, according to https://github.com/llvm/llvm-project/issues/35297 you 
mostly wanted -pedantic, but I took the liberty of enabling both. They're 
currently coupled (similar to 
https://github.com/llvm/llvm-project/issues/19442).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
Herald added a project: All.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

That's required to support `\n`, but can also used for other commands.
The argument parsing is basically duplicated from block commands, but we
can't easily factor that out as they have a different argument data
type.

This should fix #55319.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125429

Files:
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/include/clang/AST/CommentSema.h
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/test/Headers/x86-intrinsics-headers-clean.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1116,49 +1116,49 @@
 void test_attach38::test_attach39(int, B);
 
 // The inline comments expect a string after the command.
-// expected-warning@+1 {{'\a' command does not have a valid word argument}}
+// expected-warning@+1 {{'\a' command has no word arguments, expected 1}}
 /// \a
 int test_inline_no_argument_a_bad(int);
 
 /// \a A
 int test_inline_no_argument_a_good(int);
 
-// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+// expected-warning@+1 {{'\anchor' command has no word arguments, expected 1}}
 /// \anchor
 int test_inline_no_argument_anchor_bad(int);
 
 /// \anchor A
 int test_inline_no_argument_anchor_good(int);
 
-// expected-warning@+1 {{'@b' command does not have a valid word argument}}
+// expected-warning@+1 {{'@b' command has no word arguments, expected 1}}
 /// @b
 int test_inline_no_argument_b_bad(int);
 
 /// @b A
 int test_inline_no_argument_b_good(int);
 
-// expected-warning@+1 {{'\c' command does not have a valid word argument}}
+// expected-warning@+1 {{'\c' command has no word arguments, expected 1}}
 /// \c
 int test_inline_no_argument_c_bad(int);
 
 /// \c A
 int test_inline_no_argument_c_good(int);
 
-// expected-warning@+1 {{'\e' command does not have a valid word argument}}
+// expected-warning@+1 {{'\e' command has no word arguments, expected 1}}
 /// \e
 int test_inline_no_argument_e_bad(int);
 
 /// \e A
 int test_inline_no_argument_e_good(int);
 
-// expected-warning@+1 {{'\em' command does not have a valid word argument}}
+// expected-warning@+1 {{'\em' command has no word arguments, expected 1}}
 /// \em
 int test_inline_no_argument_em_bad(int);
 
 /// \em A
 int test_inline_no_argument_em_good(int);
 
-// expected-warning@+1 {{'\p' command does not have a valid word argument}}
+// expected-warning@+1 {{'\p' command has no word arguments, expected 1}}
 /// \p
 int test_inline_no_argument_p_bad(int);
 
Index: clang/test/Headers/x86-intrinsics-headers-clean.cpp
===
--- clang/test/Headers/x86-intrinsics-headers-clean.cpp
+++ clang/test/Headers/x86-intrinsics-headers-clean.cpp
@@ -1,11 +1,11 @@
 // Make sure the intrinsic headers compile cleanly with no warnings or errors.
 
 // RUN: %clang_cc1 -ffreestanding -triple i386-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // expected-no-diagnostics
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -361,37 +361,15 @@
   checkBlockCommandEmptyParagraph(Command);
 }
 
-InlineCommandComment *Sema::actOnInlineCommand(SourceLocation CommandLocBegin,
-   SourceLocation CommandLocEnd,
-   unsigned CommandID) {
-  ArrayRef Args;
-  StringRef CommandName = Traits.getCommandInfo(CommandID)->Name;
-  return new (Allocator) InlineCommandComment(
-  CommandLocBegin,
-  CommandLocEnd,
-  CommandID,
- 

[PATCH] D125422: Comment parsing: Specify argument numbers for some block commands

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 428800.
aaronpuchert added a comment.

Add an AST test to check that we parse the arguments correctly. (We don't use 
them for diagnostics currently.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125422

Files:
  clang/include/clang/AST/CommentCommands.td
  clang/test/AST/ast-dump-comment.cpp
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -189,6 +189,14 @@
 int test_multiple_returns4(int);
 
 
+/// expected-warning@+1 {{empty paragraph passed to '\retval' command}}
+/// \retval 0
+int test_retval_no_paragraph();
+
+/// \retval 0 Everything is fine.
+int test_retval_fine();
+
+
 // expected-warning@+1 {{'\param' command used in a comment that is not 
attached to a function declaration}}
 /// \param a Blah blah.
 int test_param1_backslash;
Index: clang/test/AST/ast-dump-comment.cpp
===
--- clang/test/AST/ast-dump-comment.cpp
+++ clang/test/AST/ast-dump-comment.cpp
@@ -32,6 +32,13 @@
 // CHECK-NEXT: ParagraphComment
 // CHECK-NEXT:   TextComment{{.*}} Text=" Aaa"
 
+/// \retval 42 Aaa
+int Test_BlockCommandComment_WithArgs();
+// CHECK:  FunctionDecl{{.*}}Test_BlockCommandComment_WithArgs
+// CHECK:BlockCommandComment{{.*}} Name="retval" Arg[0]="42"
+// CHECK-NEXT: ParagraphComment
+// CHECK-NEXT:   TextComment{{.*}} Text=" Aaa"
+
 /// \param Aaa xxx
 /// \param [in,out] Bbb yyy
 void Test_ParamCommandComment(int Aaa, int Bbb);
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -154,7 +154,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
-def Retval : BlockCommand<"retval">;
+def Retval : BlockCommand<"retval"> { let NumArgs = 1; }
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
@@ -162,7 +162,7 @@
 def Todo   : BlockCommand<"todo">;
 def Version: BlockCommand<"version">;
 def Warning: BlockCommand<"warning">;
-def XRefItem   : BlockCommand<"xrefitem">;
+def XRefItem   : BlockCommand<"xrefitem"> { let NumArgs = 3; }
 // HeaderDoc commands
 def Abstract  : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
 def ClassDesign   : RecordLikeDetailCommand<"classdesign">;


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -189,6 +189,14 @@
 int test_multiple_returns4(int);
 
 
+/// expected-warning@+1 {{empty paragraph passed to '\retval' command}}
+/// \retval 0
+int test_retval_no_paragraph();
+
+/// \retval 0 Everything is fine.
+int test_retval_fine();
+
+
 // expected-warning@+1 {{'\param' command used in a comment that is not attached to a function declaration}}
 /// \param a Blah blah.
 int test_param1_backslash;
Index: clang/test/AST/ast-dump-comment.cpp
===
--- clang/test/AST/ast-dump-comment.cpp
+++ clang/test/AST/ast-dump-comment.cpp
@@ -32,6 +32,13 @@
 // CHECK-NEXT: ParagraphComment
 // CHECK-NEXT:   TextComment{{.*}} Text=" Aaa"
 
+/// \retval 42 Aaa
+int Test_BlockCommandComment_WithArgs();
+// CHECK:  FunctionDecl{{.*}}Test_BlockCommandComment_WithArgs
+// CHECK:BlockCommandComment{{.*}} Name="retval" Arg[0]="42"
+// CHECK-NEXT: ParagraphComment
+// CHECK-NEXT:   TextComment{{.*}} Text=" Aaa"
+
 /// \param Aaa xxx
 /// \param [in,out] Bbb yyy
 void Test_ParamCommandComment(int Aaa, int Bbb);
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -154,7 +154,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
-def Retval : BlockCommand<"retval">;
+def Retval : BlockCommand<"retval"> { let NumArgs = 1; }
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
@@ -162,7 +162,7 @@
 def Todo   : BlockCommand<"todo">;
 def Version: BlockCommand<"version">;
 def Warning: BlockCommand<"warning">;
-def XRefItem   : BlockCommand<"xrefitem">;
+def XRefItem   : BlockCommand<"xrefitem"> { let NumArgs = 3; }
 // HeaderDoc commands
 def Abstract  : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
 def 

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

[Julian Lettner via Phabricator via cfe-commits]

yln added inline comments.



Comment at: compiler-rt/lib/asan/asan_poisoning.cpp:263
+#if SANITIZER_ARM64
+  // The ARM64 cookie has a second "size_t" entry so poison it as well
+  *(reinterpret_cast(s)-1) = kAsanArrayCookieMagic;





Comment at: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp:19
 int main(int argc, char **argv) {
-  C *buffer = new C[argc];
-  buffer[-2].x = 10;
+  C *buffer = new C[1];
+  reinterpret_cast(buffer)[-1] = 42;

`C *buffer = new C[argc];`

Although this looks weird, I think this is to prevent the compiler from 
reasoning about the array size.
I know that we had tests in the past that were "silently passing", because the 
compiler optimized away parts of the test.

I am not sure if it applies here, but let's keep it just in case ...



Comment at: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp:20
+  C *buffer = new C[1];
+  reinterpret_cast(buffer)[-1] = 42;
 // CHECK: AddressSanitizer: heap-buffer-overflow





Comment at: 
compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp:4-15
+// Here we test that:
+//   1) w/o ASan, the array cookie location IS writable
+//   2) w/ASan, the array cookie location IS writeable by default
+//   3) w/ASan, the flag "-fno-sanitize-address-poison-custom-array-cookie"
+//  is a NOP (because this is the default) and finally,
+//   4) w/ASan AND "-fsanitize-address-poison-custom-array-cookie", the
+//  array cookie location is NOT writable (ASAN successfully stopped it)

Thanks for improving the test like this!  This is now better than "just a 
test", it's essentially "executable documentation"! :)



Comment at: 
compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp:36-38
-  assert(reinterpret_cast(foo) ==
- reinterpret_cast(Foo::allocated) + sizeof(void*));
-  *reinterpret_cast(Foo::allocated) = 42;

Is the reason that we had to remove this assert (and change how we overwrite 
the cookie) that on arm the cookie is 2 words?

Can we do the following?
```
int cooke_words =  : 2 : 1;
assert(reinterpret_cast(foo) ==
 reinterpret_cast(Foo::allocated) + cookie_words * 
sizeof(void*));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125195

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

[PATCH] D121733: Clean pathnames in FileManager.

[Paul Pluzhnikov via Phabricator via cfe-commits]

ppluzhnikov added inline comments.



Comment at: clang/lib/Basic/FileManager.cpp:218
+  llvm::sys::path::remove_dots(CleanFilename, /*remove_dot_dot=*/false);
+  Filename = CleanFilename;
+

kadircet wrote:
> this is actually breaking the [contract of 
> FileEntryRef](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/FileEntry.h#L58-L59),
>  is this the point?
> 
> ```
> /// A reference to a \c FileEntry that includes the name of the file as it was
> /// accessed by the FileManager's client.
> ```
> 
> I don't see mention of this contract being changed explicitly anywhere, if so 
> can you mention it in the commit message and also update the documentation? 
> (the same applies to DirectoryEntryRef changes as well)
> 
> I am not able to take a detailed look at this at the moment, but this doesn't 
> feel like a safe change for all clients. Because people might be relying on 
> this contract without explicitly testing the behaviour for "./" in the 
> filenames. So tests passing (especially when you're just updating them to not 
> have `./`) might not be implying it's safe.
I chased this comment down to commit 4dc5573acc0d2e7c59d8bac2543eb25cb4b32984.

The commit message says:

   This commit introduces a parallel API to FileManager's getFile: 
getFileEntryRef, which returns
a reference to the FileEntry, and the name that was used to access the 
file. In the case of
a VFS with 'use-external-names', the FileEntyRef contains the external name 
of the file,
not the filename that was used to access it.

So it appears that the comment itself is not quite correct.

It's also a bit ambiguous (I think) -- "name used to access the file"
could be interpreted as the name which clang itself used to access
the file, and not necessarily the name client used.

> people might be relying on this contract without explicitly testing the 
> behaviour for "./" in the filenames.

That's a possibility.

I am not sure how to evaluate the benefit of this patch (avoiding noise 
everywhere) vs. possibly breaking clients.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

[PATCH] D125425: Explicitly add -target for Windows builds in file_test_windows.c

[Arthur Eubanks 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 rG8016a0e5a56b: Explicitly add -target for Windows builds in 
file_test_windows.c (authored by ayzhao, committed by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125425

Files:
  clang/test/Preprocessor/file_test_windows.c


Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,15 +1,15 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -fno-file-reproducible 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | 
FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
 
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | 
FileCheck %s --check-prefix CHECK-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | 
FileCheck %s --check-prefix CHECK-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | 
FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s 
--check-prefix CHECK-REMOVE-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s 
--check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s 
--check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 -fmacro-prefix-map=%p\= -c -o - %s | 
FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
 
 // RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - 
%s | FileCheck %s --check-prefix CHECK-REMOVE
 // RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - 
%s | FileCheck %s --check-prefix CHECK-REMOVE


Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,15 +1,15 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -fno-file-reproducible -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s 

[clang] 8016a0e - Explicitly add -target for Windows builds in file_test_windows.c

[Arthur Eubanks via cfe-commits]

Author: Alan Zhao
Date: 2022-05-11T15:05:55-07:00
New Revision: 8016a0e5a56b8afc0f328412adae97369c71af78

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

LOG: Explicitly add -target for Windows builds in file_test_windows.c

It turns out that the llvm buildbots run the test with
-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4, which would cause this
test to fail as the test assumed that the default target is Windows. To
fix this, we explicitly set -target for the Windows testcases.

Reviewed By: aeubanks

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

Added: 


Modified: 
clang/test/Preprocessor/file_test_windows.c

Removed: 




diff  --git a/clang/test/Preprocessor/file_test_windows.c 
b/clang/test/Preprocessor/file_test_windows.c
index 4d284b746717..7cc8a8d9e4a2 100644
--- a/clang/test/Preprocessor/file_test_windows.c
+++ b/clang/test/Preprocessor/file_test_windows.c
@@ -1,15 +1,15 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -fno-file-reproducible 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | 
FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
 
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | 
FileCheck %s --check-prefix CHECK-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | 
FileCheck %s --check-prefix CHECK-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | 
FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s 
--check-prefix CHECK-REMOVE-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s 
--check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s 
--check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 -fmacro-prefix-map=%p\= -c -o - %s | 
FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
 
 // RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - 
%s | FileCheck %s --check-prefix CHECK-REMOVE
 // RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - 
%s | FileCheck %s --check-prefix CHECK-REMOVE



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

[PATCH] D125425: Explicitly add -target for Windows builds in file_test_windows.c

[Alan Zhao via Phabricator via cfe-commits]

ayzhao created this revision.
Herald added a subscriber: pengfei.
Herald added a project: All.
ayzhao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It turns out that the llvm buildbots run the test with
-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4, which would cause this
test to fail as the test assumed that the default target is Windows. To
fix this, we explicitly set -target for the Windows testcases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125425

Files:
  clang/test/Preprocessor/file_test_windows.c


Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,15 +1,15 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -fno-file-reproducible 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL
-// RUN: %clang -E -fno-file-reproducible 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | 
FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-win32 
-fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
 
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | 
FileCheck %s --check-prefix CHECK-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | 
FileCheck %s --check-prefix CHECK-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | 
FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE-REPRODUCIBLE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s 
--check-prefix CHECK-REMOVE-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s 
--check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s 
--check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s 
-check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 
-fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ 
-fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s 
-check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -target x86_64-pc-win32 -fmacro-prefix-map=%p\= -c -o - %s | 
FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
 
 // RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - 
%s | FileCheck %s --check-prefix CHECK-REMOVE
 // RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - 
%s | FileCheck %s --check-prefix CHECK-REMOVE


Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,15 +1,15 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -fno-file-reproducible -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
-// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix 

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:5219
+// function.
+else if (!CGF.CurFn->getAttributes().hasFnAttr(AttributeNoBuiltin))
   return CGCallee::forBuiltin(builtinID, FD);

steplong wrote:
> hans wrote:
> > What if CurFn has the "wildcard" no-builtins attribute?
> Hmmm, good question. Currently, "*" generates a warning. I was able to get 
> "*" to add the attribute no-builtins to the function, but if I try making it 
> affect the calls in the function, I see a compiler crash when compiling 
> compiler-rt. It looks like some builtins must be generated even if 
> -fno-builtins is called such as __sync_lock_test_and_set_1. I'm not sure how 
> to check for that.
I think you're looking for isPredefinedLibFunction()


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

https://reviews.llvm.org/D124701

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

[clang] 52af5df - [Driver][test] run one test in darwin-dsymutil.c for Darwin only

[Yuanfang Chen via cfe-commits]

Author: Yuanfang Chen
Date: 2022-05-11T14:45:00-07:00
New Revision: 52af5df8aef737b5b609912ea28f79fcb3d9ba22

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

LOG: [Driver][test] run one test in darwin-dsymutil.c for Darwin only

Added: 


Modified: 
clang/test/Driver/darwin-dsymutil.c

Removed: 




diff  --git a/clang/test/Driver/darwin-dsymutil.c 
b/clang/test/Driver/darwin-dsymutil.c
index 9f77e7237d023..7a6e4940afeac 100644
--- a/clang/test/Driver/darwin-dsymutil.c
+++ b/clang/test/Driver/darwin-dsymutil.c
@@ -68,4 +68,4 @@
 // RUN: not grep "Dsymutil" %t
 
 // Check that we don't crash when translating arguments for dsymutil.
-// RUN: %clang -m32 -arch x86_64 -g %s -###
+// RUN: %clang -m32 -target x86_64-apple-darwin10 -arch x86_64 -g %s -###



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

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D124836#3507354 , @nickdesaulniers 
wrote:

> In D124836#3507268 , @void wrote:
>
>> I'll split off the TableGen changes into a separate patch. It will supersede 
>> those changes here, so it shouldn't delay other reviews here. :-)
>
> I'm referring to the changes to llvm/utils/TableGen/CallingConvEmitter.cpp 
> and llvm/utils/TableGen/RegisterInfoEmitter.cpp. I would like to see those as 
> a parent patch.  I'm not sure what you're referring to, and it sounds like a 
> child patch, not a parent patch.

That's what I meant. And I need those TableGen changes for this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

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

[PATCH] D125422: Comment parsing: Specify argument numbers for some block commands

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
Herald added a project: All.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The command traits have a member NumArgs for which all the parsing
infrastructure is in place, but no command was setting it to a value
other than 0. By doing so we get warnings when passing an empty
paragraph to \retval (the first argument is the return value, then comes
the description). We also take \xrefitem along for the ride, although as
the documentation states it's unlikely to be used directly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125422

Files:
  clang/include/clang/AST/CommentCommands.td
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -189,6 +189,14 @@
 int test_multiple_returns4(int);
 
 
+/// expected-warning@+1 {{empty paragraph passed to '\retval' command}}
+/// \retval 0
+int test_retval_no_paragraph();
+
+/// \retval 0 Everything is fine.
+int test_retval_fine();
+
+
 // expected-warning@+1 {{'\param' command used in a comment that is not 
attached to a function declaration}}
 /// \param a Blah blah.
 int test_param1_backslash;
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -154,7 +154,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
-def Retval : BlockCommand<"retval">;
+def Retval : BlockCommand<"retval"> { let NumArgs = 1; }
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
@@ -162,7 +162,7 @@
 def Todo   : BlockCommand<"todo">;
 def Version: BlockCommand<"version">;
 def Warning: BlockCommand<"warning">;
-def XRefItem   : BlockCommand<"xrefitem">;
+def XRefItem   : BlockCommand<"xrefitem"> { let NumArgs = 3; }
 // HeaderDoc commands
 def Abstract  : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
 def ClassDesign   : RecordLikeDetailCommand<"classdesign">;


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -189,6 +189,14 @@
 int test_multiple_returns4(int);
 
 
+/// expected-warning@+1 {{empty paragraph passed to '\retval' command}}
+/// \retval 0
+int test_retval_no_paragraph();
+
+/// \retval 0 Everything is fine.
+int test_retval_fine();
+
+
 // expected-warning@+1 {{'\param' command used in a comment that is not attached to a function declaration}}
 /// \param a Blah blah.
 int test_param1_backslash;
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -154,7 +154,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
-def Retval : BlockCommand<"retval">;
+def Retval : BlockCommand<"retval"> { let NumArgs = 1; }
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
@@ -162,7 +162,7 @@
 def Todo   : BlockCommand<"todo">;
 def Version: BlockCommand<"version">;
 def Warning: BlockCommand<"warning">;
-def XRefItem   : BlockCommand<"xrefitem">;
+def XRefItem   : BlockCommand<"xrefitem"> { let NumArgs = 3; }
 // HeaderDoc commands
 def Abstract  : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
 def ClassDesign   : RecordLikeDetailCommand<"classdesign">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125420: [AArch64] Add support for -fzero-call-used-regs

[Bill Wendling via Phabricator via cfe-commits]

void abandoned this revision.
void added a comment.

Ugh!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125420

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

[PATCH] D125420: [AArch64] Add support for -fzero-call-used-regs

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added reviewers: nickdesaulniers, nhaehnle, thakis, jankratochvil.
Herald added subscribers: pengfei, hiraditya, kristof.beyls.
Herald added a project: All.
void requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

Support the "-fzero-call-used-regs" option on AArch64. This involves much less
specialized code than the X86 version. Most of the checks can be done with
TableGen.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125420

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
  llvm/lib/Target/AArch64/AArch64FrameLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.td
  llvm/test/CodeGen/AArch64/zero-call-used-regs.ll

Index: llvm/test/CodeGen/AArch64/zero-call-used-regs.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/zero-call-used-regs.ll
@@ -0,0 +1,666 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown | FileCheck %s --check-prefixes=CHECK,DEFAULT
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown -mattr=+sve | FileCheck %s --check-prefixes=CHECK,SVE
+
+@result = dso_local global i32 0, align 4
+
+define dso_local i32 @skip(i32 noundef %a, i32 noundef %b, i32 noundef %c) local_unnamed_addr #0 "zero-call-used-regs"="skip" {
+; CHECK-LABEL: skip:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mul w8, w1, w0
+; CHECK-NEXT:orr w0, w8, w2
+; CHECK-NEXT:ret
+
+entry:
+  %mul = mul nsw i32 %b, %a
+  %or = or i32 %mul, %c
+  ret i32 %or
+}
+
+define dso_local i32 @used_gpr_arg(i32 noundef %a, i32 noundef %b, i32 noundef %c) local_unnamed_addr #0 noinline optnone "zero-call-used-regs"="used-gpr-arg" {
+; CHECK-LABEL: used_gpr_arg:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mul w8, w1, w0
+; CHECK-NEXT:orr w0, w8, w2
+; CHECK-NEXT:mov x1, #0
+; CHECK-NEXT:mov x2, #0
+; CHECK-NEXT:ret
+
+entry:
+  %mul = mul nsw i32 %b, %a
+  %or = or i32 %mul, %c
+  ret i32 %or
+}
+
+define dso_local i32 @used_gpr(i32 noundef %a, i32 noundef %b, i32 noundef %c) local_unnamed_addr #0 noinline optnone "zero-call-used-regs"="used-gpr" {
+; CHECK-LABEL: used_gpr:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mul w8, w1, w0
+; CHECK-NEXT:orr w0, w8, w2
+; CHECK-NEXT:mov x1, #0
+; CHECK-NEXT:mov x2, #0
+; CHECK-NEXT:mov x8, #0
+; CHECK-NEXT:ret
+
+entry:
+  %mul = mul nsw i32 %b, %a
+  %or = or i32 %mul, %c
+  ret i32 %or
+}
+
+define dso_local i32 @used_arg(i32 noundef %a, i32 noundef %b, i32 noundef %c) local_unnamed_addr #0 noinline optnone "zero-call-used-regs"="used-arg" {
+; CHECK-LABEL: used_arg:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mul w8, w1, w0
+; CHECK-NEXT:orr w0, w8, w2
+; CHECK-NEXT:mov x1, #0
+; CHECK-NEXT:mov x2, #0
+; CHECK-NEXT:ret
+
+entry:
+  %mul = mul nsw i32 %b, %a
+  %or = or i32 %mul, %c
+  ret i32 %or
+}
+
+define dso_local i32 @used(i32 noundef %a, i32 noundef %b, i32 noundef %c) local_unnamed_addr #0 noinline optnone "zero-call-used-regs"="used" {
+; CHECK-LABEL: used:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mul w8, w1, w0
+; CHECK-NEXT:orr w0, w8, w2
+; CHECK-NEXT:mov x1, #0
+; CHECK-NEXT:mov x2, #0
+; CHECK-NEXT:mov x8, #0
+; CHECK-NEXT:ret
+
+entry:
+  %mul = mul nsw i32 %b, %a
+  %or = or i32 %mul, %c
+  ret i32 %or
+}
+
+define dso_local i32 @all_gpr_arg(i32 noundef %a, i32 noundef %b, i32 noundef %c) local_unnamed_addr #0 "zero-call-used-regs"="all-gpr-arg" {
+; CHECK-LABEL: all_gpr_arg:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mul w8, w1, w0
+; CHECK-NEXT:mov x1, #0
+; CHECK-NEXT:mov x3, #0
+; CHECK-NEXT:mov x4, #0
+; CHECK-NEXT:orr w0, w8, w2
+; CHECK-NEXT:mov x2, #0
+; CHECK-NEXT:mov x5, #0
+; CHECK-NEXT:mov x6, #0
+; CHECK-NEXT:mov x7, #0
+; CHECK-NEXT:mov x8, #0
+; CHECK-NEXT:mov x18, #0
+; CHECK-NEXT:ret
+
+entry:
+  %mul = mul nsw i32 %b, %a
+  %or = or i32 %mul, %c
+  ret i32 %or
+}
+
+define dso_local i32 @all_gpr(i32 noundef %a, i32 noundef %b, i32 noundef %c) local_unnamed_addr #0 "zero-call-used-regs"="all-gpr" {
+; CHECK-LABEL: all_gpr:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mul w8, w1, w0
+; CHECK-NEXT:mov x1, #0
+; CHECK-NEXT:mov x3, #0
+; CHECK-NEXT:mov x4, #0
+; CHECK-NEXT:orr w0, w8, w2
+; CHECK-NEXT:mov x2, #0
+; CHECK-NEXT:mov x5, #0
+; CHECK-NEXT:mov x6, #0
+; CHECK-NEXT:mov x7, #0
+; CHECK-NEXT:mov x8, #0
+; CHECK-NEXT:mov x9, #0
+; CHECK-NEXT:mov x10, #0
+; CHECK-NEXT:mov x11, #0
+; CHECK-NEXT:mov x12, #0
+; CHECK-NEXT:mov x13, #0
+; CHECK-NEXT:mov 

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

[Vang Thao via Phabricator via cfe-commits]

vangthao added a comment.

I am not sure if allowing clang to accept newlines is a good idea. It seems 
like clang wants to know what type of message is being outputted. For example 
whether this is a remark, warning, etc. but allowing for a diagnostic to output 
their own newline makes it ambiguous where exactly that output is coming from.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

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

[PATCH] D122766: [clang] Add the flag -ffile-reproducible

[Hans Wennborg 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 rG6398f3f2e904: [clang] Add the flag -ffile-reproducible 
(authored by ayzhao, committed by hans).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test_windows.c

Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,9 +1,25 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
+
+// RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// Clang defaults to forward slashes for the non-prefix portion of the path even if the build environment is Windows.
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-win32 -c -o - %s | FileCheck %s --check-prefix CHECK-WINDOWS-FULL
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-linux-gnu -c -o - %s | FileCheck %s --check-prefix CHECK-LINUX-FULL
 
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
@@ -27,3 +43,34 @@
 // CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
 // CHECK-REMOVE: basefile: "file_test_windows.c"
 // CHECK-REMOVE-NOT: filename:
+
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE-NOT: filename:
+
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-EVIL-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE-NOT: filename:
+
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_INC\\include-file-test\\file_test.h"
+// CHECK-CASE-REPRODUCIBLE: 

[clang] 6398f3f - [clang] Add the flag -ffile-reproducible

[Hans Wennborg via cfe-commits]

Author: Alan Zhao
Date: 2022-05-11T23:04:36+02:00
New Revision: 6398f3f2e9045cb38c73425fcc4dddbfb8933a57

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

LOG: [clang] Add the flag -ffile-reproducible

When Clang generates the path prefix (i.e. the path of the directory
where the file is) when generating FILE, __builtin_FILE(), and
std::source_location, Clang uses the platform-specific path separator
character of the build environment where Clang _itself_ is built. This
leads to inconsistencies in Chrome builds where Clang running on
non-Windows environments uses the forward slash (/) path separator
while Clang running on Windows builds uses the backslash (\) path
separator. To fix this, we add a flag -ffile-reproducible (and its
inverse, -fno-file-reproducible) to have Clang use the target's
platform-specific file separator character.

Additionally, the existing flags -fmacro-prefix-map and
-ffile-prefix-map now both imply -ffile-reproducible. This can be
overriden by setting -fno-file-reproducible.

[0]: https://crbug.com/1310767

Differential revision: https://reviews.llvm.org/D122766

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
clang/include/clang/Lex/Preprocessor.h
clang/lib/AST/Expr.cpp
clang/lib/Basic/LangOptions.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Lex/PPMacroExpansion.cpp
clang/test/CodeGenCXX/builtin-source-location.cpp
clang/test/Preprocessor/file_test.c
clang/test/Preprocessor/file_test_windows.c

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index caea03a9968e8..d442b4d96e765 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -452,6 +452,14 @@ class LangOptions : public LangOptionsBase {
   /// The seed used by the randomize structure layout feature.
   std::string RandstructSeed;
 
+  /// Indicates whether the __FILE__ macro should use the target's
+  /// platform-specific file separator or whether it should use the build
+  /// environment's platform-specific file separator.
+  ///
+  /// The plaform-specific path separator is the backslash(\) for Windows and
+  /// forward slash (/) elsewhere.
+  bool UseTargetPathSeparator = false;
+
   LangOptions();
 
   /// Set language defaults for the given input language and
@@ -577,7 +585,7 @@ class LangOptions : public LangOptionsBase {
   bool isSYCL() const { return SYCLIsDevice || SYCLIsHost; }
 
   /// Remap path prefix according to -fmacro-prefix-path option.
-  void remapPathPrefix(SmallString<256> ) const;
+  void remapPathPrefix(SmallVectorImpl ) const;
 };
 
 /// Floating point control options

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 05988cbbf8d7e..8e840ede926a9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1507,6 +1507,14 @@ def : Flag<["-"], "fextended-identifiers">, 
Group;
 def : Flag<["-"], "fno-extended-identifiers">, Group, 
Flags<[Unsupported]>;
 def fhosted : Flag<["-"], "fhosted">, Group;
 def fdenormal_fp_math_EQ : Joined<["-"], "fdenormal-fp-math=">, 
Group, Flags<[CC1Option]>;
+def ffile_reproducible : Flag<["-"], "ffile-reproducible">, Group,
+  Flags<[CoreOption, CC1Option]>,
+  HelpText<"Use the target's platform-specific path separator character when "
+   "expanding the __FILE__ macro">;
+def fno_file_reproducible : Flag<["-"], "fno-file-reproducible">,
+  Group, Flags<[CoreOption, CC1Option]>,
+  HelpText<"Use the host's platform-specific path separator character when "
+   "expanding the __FILE__ macro">;
 def ffp_eval_method_EQ : Joined<["-"], "ffp-eval-method=">, Group, 
Flags<[CC1Option]>,
   HelpText<"Specifies the evaluation method to use for floating-point 
arithmetic.">,
   Values<"source,double,extended">, NormalizedValuesScope<"LangOptions">,
@@ -3009,10 +3017,12 @@ def fcoverage_prefix_map_EQ
 HelpText<"remap file source paths in coverage mapping">;
 def ffile_prefix_map_EQ
   : Joined<["-"], "ffile-prefix-map=">, Group,
-HelpText<"remap file source paths in debug info, predefined preprocessor 
macros and __builtin_FILE()">;
+HelpText<"remap file source paths in debug info, predefined preprocessor "
+ "macros and __builtin_FILE(). Implies -ffile-reproducible.">;
 def fmacro_prefix_map_EQ
   : Joined<["-"], "fmacro-prefix-map=">, Group, Flags<[CC1Option]>,
-HelpText<"remap file source paths in predefined preprocessor macros and 
__builtin_FILE()">;
+HelpText<"remap file source paths in predefined preprocessor macros and "
+ "__builtin_FILE(). Implies 

[PATCH] D125406: [OpenMP] Fix mangling for linear parameters with negative stride

[Mike Rice 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 rG772b0c44a429: [OpenMP] Fix mangling for linear parameters 
with negative stride (authored by mikerice).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125406

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_simd_codegen.cpp


Index: clang/test/OpenMP/declare_simd_codegen.cpp
===
--- clang/test/OpenMP/declare_simd_codegen.cpp
+++ clang/test/OpenMP/declare_simd_codegen.cpp
@@ -155,6 +155,14 @@
   return a + int(b);
 }
 
+// Test negative strides
+#pragma omp declare simd simdlen(4) linear(a:-2) linear(b:-8) \
+linear(uval(c):-4) linear(ref(d):-16) \
+linear(e:-1) linear(f:-1) linear(g:0)
+double Six(int a, float *b, int , int *, char e, char *f, short g) {
+ return a + int(*b) + c + *d + e + *f + g;
+}
+
 // CHECK-DAG: define {{.+}}@_Z5add_1Pf(
 // CHECK-DAG: define {{.+}}@_Z1hIiEvPT_S1_S1_S1_(
 // CHECK-DAG: define {{.+}}@_Z1hIfEvPT_S1_S1_S1_(
@@ -178,6 +186,7 @@
 // CHECK-DAG: define {{.+}}@_Z5ThreeRiS_
 // CHECK-DAG: define {{.+}}@_Z4FourRiS_
 // CHECK-DAG: define {{.+}}@_Z4FiveiRsS_S_S_S_S_S_S_
+// CHECK-DAG: define {{.+}}@_Z3SixiPfRiRPicPcs
 
 // CHECK-DAG: "_ZGVbM4l32__Z5add_1Pf"
 // CHECK-DAG: "_ZGVbN4l32__Z5add_1Pf"
@@ -399,6 +408,8 @@
 // CHECK-DAG: "_ZGVbN4R8R4__Z4FourRiS_"
 // CHECK-DAG: "_ZGVbM4uL2Ls0L4Ls0U8Us0R32Rs0__Z4FiveiRsS_S_S_S_S_S_S_"
 // CHECK-DAG: "_ZGVbN4uL2Ls0L4Ls0U8Us0R32Rs0__Z4FiveiRsS_S_S_S_S_S_S_"
+// CHECK-DAG: "_ZGVbM4ln2ln32Un4Rn128ln1ln1l0__Z3SixiPfRiRPicPcs"
+// CHECK-DAG: "_ZGVbN4ln2ln32Un4Rn128ln1ln1l0__Z3SixiPfRiRPicPcs"
 
 // CHECK-NOT: "_ZGV{{.+}}__Z1fRA_i
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -11506,7 +11506,9 @@
  ParamAttr.Kind == LinearUVal || ParamAttr.Kind == LinearVal) {
   // Don't print the step value if it is not present or if it is
   // equal to 1.
-  if (ParamAttr.StrideOrArg != 1)
+  if (ParamAttr.StrideOrArg < 0)
+Out << 'n' << -ParamAttr.StrideOrArg;
+  else if (ParamAttr.StrideOrArg != 1)
 Out << ParamAttr.StrideOrArg;
 }
 


Index: clang/test/OpenMP/declare_simd_codegen.cpp
===
--- clang/test/OpenMP/declare_simd_codegen.cpp
+++ clang/test/OpenMP/declare_simd_codegen.cpp
@@ -155,6 +155,14 @@
   return a + int(b);
 }
 
+// Test negative strides
+#pragma omp declare simd simdlen(4) linear(a:-2) linear(b:-8) \
+linear(uval(c):-4) linear(ref(d):-16) \
+linear(e:-1) linear(f:-1) linear(g:0)
+double Six(int a, float *b, int , int *, char e, char *f, short g) {
+ return a + int(*b) + c + *d + e + *f + g;
+}
+
 // CHECK-DAG: define {{.+}}@_Z5add_1Pf(
 // CHECK-DAG: define {{.+}}@_Z1hIiEvPT_S1_S1_S1_(
 // CHECK-DAG: define {{.+}}@_Z1hIfEvPT_S1_S1_S1_(
@@ -178,6 +186,7 @@
 // CHECK-DAG: define {{.+}}@_Z5ThreeRiS_
 // CHECK-DAG: define {{.+}}@_Z4FourRiS_
 // CHECK-DAG: define {{.+}}@_Z4FiveiRsS_S_S_S_S_S_S_
+// CHECK-DAG: define {{.+}}@_Z3SixiPfRiRPicPcs
 
 // CHECK-DAG: "_ZGVbM4l32__Z5add_1Pf"
 // CHECK-DAG: "_ZGVbN4l32__Z5add_1Pf"
@@ -399,6 +408,8 @@
 // CHECK-DAG: "_ZGVbN4R8R4__Z4FourRiS_"
 // CHECK-DAG: "_ZGVbM4uL2Ls0L4Ls0U8Us0R32Rs0__Z4FiveiRsS_S_S_S_S_S_S_"
 // CHECK-DAG: "_ZGVbN4uL2Ls0L4Ls0U8Us0R32Rs0__Z4FiveiRsS_S_S_S_S_S_S_"
+// CHECK-DAG: "_ZGVbM4ln2ln32Un4Rn128ln1ln1l0__Z3SixiPfRiRPicPcs"
+// CHECK-DAG: "_ZGVbN4ln2ln32Un4Rn128ln1ln1l0__Z3SixiPfRiRPicPcs"
 
 // CHECK-NOT: "_ZGV{{.+}}__Z1fRA_i
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -11506,7 +11506,9 @@
  ParamAttr.Kind == LinearUVal || ParamAttr.Kind == LinearVal) {
   // Don't print the step value if it is not present or if it is
   // equal to 1.
-  if (ParamAttr.StrideOrArg != 1)
+  if (ParamAttr.StrideOrArg < 0)
+Out << 'n' << -ParamAttr.StrideOrArg;
+  else if (ParamAttr.StrideOrArg != 1)
 Out << ParamAttr.StrideOrArg;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 772b0c4 - [OpenMP] Fix mangling for linear parameters with negative stride

[Mike Rice via cfe-commits]

Author: Mike Rice
Date: 2022-05-11T14:02:09-07:00
New Revision: 772b0c44a4296a34cbc072c2a7cf294410d07a1a

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

LOG: [OpenMP] Fix mangling for linear parameters with negative stride

The 'n' character is used in place of '-' in the mangled name.

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/declare_simd_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 52f6ca4cfb3d0..f197c331e6cfc 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -11506,7 +11506,9 @@ static std::string 
mangleVectorParameters(ArrayRef ParamAttrs) {
  ParamAttr.Kind == LinearUVal || ParamAttr.Kind == LinearVal) {
   // Don't print the step value if it is not present or if it is
   // equal to 1.
-  if (ParamAttr.StrideOrArg != 1)
+  if (ParamAttr.StrideOrArg < 0)
+Out << 'n' << -ParamAttr.StrideOrArg;
+  else if (ParamAttr.StrideOrArg != 1)
 Out << ParamAttr.StrideOrArg;
 }
 

diff  --git a/clang/test/OpenMP/declare_simd_codegen.cpp 
b/clang/test/OpenMP/declare_simd_codegen.cpp
index fa0be2acc192f..573dbdbfd69e9 100644
--- a/clang/test/OpenMP/declare_simd_codegen.cpp
+++ b/clang/test/OpenMP/declare_simd_codegen.cpp
@@ -155,6 +155,14 @@ double Five(int a, short , short , short , short , 
short , short ,
   return a + int(b);
 }
 
+// Test negative strides
+#pragma omp declare simd simdlen(4) linear(a:-2) linear(b:-8) \
+linear(uval(c):-4) linear(ref(d):-16) \
+linear(e:-1) linear(f:-1) linear(g:0)
+double Six(int a, float *b, int , int *, char e, char *f, short g) {
+ return a + int(*b) + c + *d + e + *f + g;
+}
+
 // CHECK-DAG: define {{.+}}@_Z5add_1Pf(
 // CHECK-DAG: define {{.+}}@_Z1hIiEvPT_S1_S1_S1_(
 // CHECK-DAG: define {{.+}}@_Z1hIfEvPT_S1_S1_S1_(
@@ -178,6 +186,7 @@ double Five(int a, short , short , short , short , 
short , short ,
 // CHECK-DAG: define {{.+}}@_Z5ThreeRiS_
 // CHECK-DAG: define {{.+}}@_Z4FourRiS_
 // CHECK-DAG: define {{.+}}@_Z4FiveiRsS_S_S_S_S_S_S_
+// CHECK-DAG: define {{.+}}@_Z3SixiPfRiRPicPcs
 
 // CHECK-DAG: "_ZGVbM4l32__Z5add_1Pf"
 // CHECK-DAG: "_ZGVbN4l32__Z5add_1Pf"
@@ -399,6 +408,8 @@ double Five(int a, short , short , short , short , 
short , short ,
 // CHECK-DAG: "_ZGVbN4R8R4__Z4FourRiS_"
 // CHECK-DAG: "_ZGVbM4uL2Ls0L4Ls0U8Us0R32Rs0__Z4FiveiRsS_S_S_S_S_S_S_"
 // CHECK-DAG: "_ZGVbN4uL2Ls0L4Ls0U8Us0R32Rs0__Z4FiveiRsS_S_S_S_S_S_S_"
+// CHECK-DAG: "_ZGVbM4ln2ln32Un4Rn128ln1ln1l0__Z3SixiPfRiRPicPcs"
+// CHECK-DAG: "_ZGVbN4ln2ln32Un4Rn128ln1ln1l0__Z3SixiPfRiRPicPcs"
 
 // CHECK-NOT: "_ZGV{{.+}}__Z1fRA_i
 



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

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D124836#3507268 , @void wrote:

> I'll split off the TableGen changes into a separate patch. It will supersede 
> those changes here, so it shouldn't delay other reviews here. :-)

I'm referring to the changes to llvm/utils/TableGen/CallingConvEmitter.cpp and 
llvm/utils/TableGen/RegisterInfoEmitter.cpp. I would like to see those as a 
parent patch.  I'm not sure what you're referring to, and it sounds like a 
child patch, not a parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

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

[PATCH] D125414: Revert "[HLSL] add -D option for dxc mode."

[Xiang Li via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6055837f6d29: Revert [HLSL] add -D option for dxc 
mode. (authored by python3kgae).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125414

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/dxc_D.hlsl


Index: clang/test/Driver/dxc_D.hlsl
===
--- clang/test/Driver/dxc_D.hlsl
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
-// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s 
--check-prefix=ERROR
-
-// Make sure -D send to cc1.
-// CHECK:"-D" "TEST=2"
-
-#ifndef TEST
-#error "TEST not defined"
-#elif TEST != 2
-#error "TEST defined to wrong value"
-#endif
-
-// ERROR-NOT: error:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {
-  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
-  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
+ options::OPT_S, 
options::OPT_emit_llvm,
+ options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6756,8 +6756,6 @@
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
-def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
-  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM 
passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;


Index: clang/test/Driver/dxc_D.hlsl
===
--- clang/test/Driver/dxc_D.hlsl
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
-// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s --check-prefix=ERROR
-
-// Make sure -D send to cc1.
-// CHECK:"-D" "TEST=2"
-
-#ifndef TEST
-#error "TEST not defined"
-#elif TEST != 2
-#error "TEST defined to wrong value"
-#endif
-
-// ERROR-NOT: error:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {
-  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
-  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
+ options::OPT_S, options::OPT_emit_llvm,
+ options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6756,8 +6756,6 @@
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
-def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
-  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6055837 - Revert "[HLSL] add -D option for dxc mode."

[Xiang Li via cfe-commits]

Author: Xiang Li
Date: 2022-05-11T13:57:47-07:00
New Revision: 6055837f6d2941bbc09e44dc88b6a39b934a7453

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

LOG: Revert "[HLSL] add -D option for dxc mode."

This reverts commit 4dae38ebfba0d8583e52c3ded8f62f5f9fa77fda.

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

Added: 


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

Removed: 
clang/test/Driver/dxc_D.hlsl



diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index e0eea7310fb2d..05988cbbf8d7e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6756,8 +6756,6 @@ def target_profile : DXCJoinedOrSeparate<"T">, 
MetaVarName<"">,
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
-def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
-  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM 
passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a9fbdd4272c42..a4777b55072ba 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@ static void RenderOpenCLOptions(const ArgList , 
ArgStringList ,
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {
-  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
-  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
+ options::OPT_S, 
options::OPT_emit_llvm,
+ options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))

diff  --git a/clang/test/Driver/dxc_D.hlsl b/clang/test/Driver/dxc_D.hlsl
deleted file mode 100644
index d32d885f11b00..0
--- a/clang/test/Driver/dxc_D.hlsl
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
-// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s 
--check-prefix=ERROR
-
-// Make sure -D send to cc1.
-// CHECK:"-D" "TEST=2"
-
-#ifndef TEST
-#error "TEST not defined"
-#elif TEST != 2
-#error "TEST defined to wrong value"
-#endif
-
-// ERROR-NOT: error:



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

[clang] 42a1fb5 - [LinkerWrapper][Fix} Fix bad alignment from extracted archive members

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-05-11T16:56:41-04:00
New Revision: 42a1fb5ca56c494e25419a97057a9526f3e8608d

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

LOG: [LinkerWrapper][Fix} Fix bad alignment from extracted archive members

Summary:
We use embedded binaries to extract offloading device code from the host
fatbinary. This uses a binary format whose necessary alignment is
eight bytes. The alignment is included within the ELF section type so
the data extracted from the ELF should always be aligned at that amount.
However, if this file was extraqcted from a static archive, it was being
sent as an offset in the archive file which did not have the same
alignment guaruntees as the ELF file. This was causing errors in the
UB-sanitizer build as it would occasionally try to access a misaligned
address. To fix this, I simply copy the memory directly to a new buffer
which is guarnteed to have worst-case alignment of 16 in the case that
it's not properly aligned.

Added: 


Modified: 
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Removed: 




diff  --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp 
b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 670e1af943046..ad9f6c9b55770 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -536,11 +536,18 @@ extractFromArchive(const Archive ,
   // offloading code removed.
   Error Err = Error::success();
   for (auto Child : Library.children(Err)) {
-auto ChildBufferRefOrErr = Child.getMemoryBufferRef();
-if (!ChildBufferRefOrErr)
-  return ChildBufferRefOrErr.takeError();
+auto ChildBufferOrErr = Child.getMemoryBufferRef();
+if (!ChildBufferOrErr)
+  return ChildBufferOrErr.takeError();
 std::unique_ptr ChildBuffer =
-MemoryBuffer::getMemBuffer(*ChildBufferRefOrErr, false);
+MemoryBuffer::getMemBuffer(*ChildBufferOrErr, false);
+
+// Check if the buffer has the required alignment.
+if (!isAddrAligned(Align(OffloadBinary::getAlignment()),
+   ChildBuffer->getBufferStart()))
+  ChildBuffer = MemoryBuffer::getMemBufferCopy(
+  ChildBufferOrErr->getBuffer(),
+  ChildBufferOrErr->getBufferIdentifier());
 
 auto FileOrErr = extractFromBuffer(std::move(ChildBuffer), DeviceFiles,
/*IsLibrary*/ true);



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

[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
Herald added subscribers: zzheng, kristof.beyls.
Herald added a project: All.
efriedma requested review of this revision.
Herald added a project: clang.

Part of initial Arm64EC patchset.

Don't try to duplicate the existing logic; instead, just call the actual code 
we use for native x64.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125419

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/arm64ec.c
  clang/test/CodeGenCXX/arm64ec.cpp


Index: clang/test/CodeGenCXX/arm64ec.cpp
===
--- clang/test/CodeGenCXX/arm64ec.cpp
+++ clang/test/CodeGenCXX/arm64ec.cpp
@@ -2,6 +2,8 @@
 
 // CHECK: @"?g@@YAXUA@@UB@@@Z" = alias void ([2 x float], [4 x float]), void 
([2 x float], [4 x float])* @"?g@@$$hYAXUA@@UB@@@Z"
 // CHECK: define dso_local void @"?g@@$$hYAXUA@@UB@@@Z"
+// CHECK: call void (i64, ...) @"?f@@YAXUA@@ZZ"(i64 %{{.*}}, %struct.B* 
noundef %{{.*}})
 typedef struct { float x[2]; } A;
 typedef struct { float x[4]; } B;
-void g(A a, B b) { }
+void f(A a, ...);
+void g(A a, B b) { f(a, b); }
Index: clang/test/CodeGen/arm64ec.c
===
--- clang/test/CodeGen/arm64ec.c
+++ clang/test/CodeGen/arm64ec.c
@@ -2,6 +2,8 @@
 
 // CHECK: @g = alias void ([2 x float], [4 x float]), void ([2 x float], [4 x 
float])* @"#g"
 // CHECK: define dso_local void @"#g"
+// CHECK: call void (i64, ...) @f(i64 %{{.*}}, %struct.B* noundef %{{.*}})
 typedef struct { float x[2]; } A;
 typedef struct { float x[4]; } B;
-void g(A a, B b) { }
+void f(A a, ...);
+void g(A a, B b) { f(a, b); }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -2458,6 +2458,12 @@
 return true;
   }
 
+  ABIArgInfo classifyArgForArm64ECVarArg(QualType Ty) {
+unsigned FreeSSERegs = 0;
+return classify(Ty, FreeSSERegs, /*IsReturnType=*/false,
+/*IsVectorCall=*/false, /*IsRegCall=*/false);
+  }
+
 private:
   ABIArgInfo classify(QualType Ty, unsigned , bool IsReturnType,
   bool IsVectorCall, bool IsRegCall) const;
@@ -5743,6 +5749,13 @@
  unsigned CallingConvention) const {
   Ty = useFirstFieldIfTransparentUnion(Ty);
 
+  if (IsVariadic && getTarget().getTriple().isWindowsArm64EC()) {
+// Arm64EC varargs functions use the x86_64 classification rules,
+// not the AArch64 ABI rules.
+WinX86_64ABIInfo Win64ABIInfo(CGT, X86AVXABILevel::None);
+return Win64ABIInfo.classifyArgForArm64ECVarArg(Ty);
+  }
+
   // Handle illegal vector types here.
   if (isIllegalVectorType(Ty))
 return coerceIllegalVector(Ty);


Index: clang/test/CodeGenCXX/arm64ec.cpp
===
--- clang/test/CodeGenCXX/arm64ec.cpp
+++ clang/test/CodeGenCXX/arm64ec.cpp
@@ -2,6 +2,8 @@
 
 // CHECK: @"?g@@YAXUA@@UB@@@Z" = alias void ([2 x float], [4 x float]), void ([2 x float], [4 x float])* @"?g@@$$hYAXUA@@UB@@@Z"
 // CHECK: define dso_local void @"?g@@$$hYAXUA@@UB@@@Z"
+// CHECK: call void (i64, ...) @"?f@@YAXUA@@ZZ"(i64 %{{.*}}, %struct.B* noundef %{{.*}})
 typedef struct { float x[2]; } A;
 typedef struct { float x[4]; } B;
-void g(A a, B b) { }
+void f(A a, ...);
+void g(A a, B b) { f(a, b); }
Index: clang/test/CodeGen/arm64ec.c
===
--- clang/test/CodeGen/arm64ec.c
+++ clang/test/CodeGen/arm64ec.c
@@ -2,6 +2,8 @@
 
 // CHECK: @g = alias void ([2 x float], [4 x float]), void ([2 x float], [4 x float])* @"#g"
 // CHECK: define dso_local void @"#g"
+// CHECK: call void (i64, ...) @f(i64 %{{.*}}, %struct.B* noundef %{{.*}})
 typedef struct { float x[2]; } A;
 typedef struct { float x[4]; } B;
-void g(A a, B b) { }
+void f(A a, ...);
+void g(A a, B b) { f(a, b); }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -2458,6 +2458,12 @@
 return true;
   }
 
+  ABIArgInfo classifyArgForArm64ECVarArg(QualType Ty) {
+unsigned FreeSSERegs = 0;
+return classify(Ty, FreeSSERegs, /*IsReturnType=*/false,
+/*IsVectorCall=*/false, /*IsRegCall=*/false);
+  }
+
 private:
   ABIArgInfo classify(QualType Ty, unsigned , bool IsReturnType,
   bool IsVectorCall, bool IsRegCall) const;
@@ -5743,6 +5749,13 @@
  unsigned CallingConvention) const {
   Ty = useFirstFieldIfTransparentUnion(Ty);
 
+  if (IsVariadic && getTarget().getTriple().isWindowsArm64EC()) {
+// Arm64EC varargs functions use the x86_64 classification rules,
+// not the AArch64 ABI rules.
+WinX86_64ABIInfo Win64ABIInfo(CGT, X86AVXABILevel::None);
+return 

[PATCH] D124699: [DeadArgElim] Set unused arguments for internal functions

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D124699#3507250 , @qcolombet wrote:

> Thanks @thakis !
>
> @fhahn are you okay with the clang tests update as well?

Yes looks good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124699

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
Herald added subscribers: zzheng, kristof.beyls.
Herald added a project: All.
efriedma requested review of this revision.
Herald added a project: clang.

Part of initial Arm64EC patchset.

For the Arm64EC ABI, ARM64 functions have an alternate name.  For C code, this 
name is just the original name prefixed with "#".  For C++ code, we stick a 
"$$h" modifier in the middle of the mangling.

For functions which are not hybrid_patchable, the normal name is then an alias 
for the alternate name.  (For functions that are patchable, we have to do 
something more complicated to tell the linker to generate a stub; I haven't 
tried to implement that yet.)

This doesn't emit quite the same symbols table as MSVC for simple cases: MSVC 
generates a IMAGE_WEAK_EXTERN_ANTI_DEPENDENCY alias, where this just makes 
another symbol pointing at the function definition. This probably matters for 
the hybmp$x table, but I don't have the complete documentation at the moment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125418

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/arm64ec.c
  clang/test/CodeGenCXX/arm64ec.cpp

Index: clang/test/CodeGenCXX/arm64ec.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/arm64ec.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -no-opaque-pointers -triple aarch64-windows-msvc_arm64ec -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @"?g@@YAXUA@@UB@@@Z" = alias void ([2 x float], [4 x float]), void ([2 x float], [4 x float])* @"?g@@$$hYAXUA@@UB@@@Z"
+// CHECK: define dso_local void @"?g@@$$hYAXUA@@UB@@@Z"
+typedef struct { float x[2]; } A;
+typedef struct { float x[4]; } B;
+void g(A a, B b) { }
Index: clang/test/CodeGen/arm64ec.c
===
--- /dev/null
+++ clang/test/CodeGen/arm64ec.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -no-opaque-pointers -triple aarch64-windows-msvc_arm64ec -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @g = alias void ([2 x float], [4 x float]), void ([2 x float], [4 x float])* @"#g"
+// CHECK: define dso_local void @"#g"
+typedef struct { float x[2]; } A;
+typedef struct { float x[4]; } B;
+void g(A a, B b) { }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5117,6 +5117,23 @@
   if (!GV->isDeclaration())
 return;
 
+  if (getTriple().isWindowsArm64EC()) {
+// For ARM64EC targets, a function definition's name is mangled differently
+// from the normal symbol.  We then emit an alias from the normal
+// symbol to the remangled definition.
+// FIXME: MSVC uses IMAGE_WEAK_EXTERN_ANTI_DEPENDENCY, we just emit
+// multiple definition symbols.  Why does this matter?
+// FIXME: For hybrid_patchable functions, the alias doesn't point
+// to the function itself; it points to a stub for the compiler.
+// FIXME: We also need to emit an entry thunk.
+SmallString<256> MangledName;
+llvm::raw_svector_ostream Out(MangledName);
+getCXXABI().getMangleContext().mangleArm64ECFnDef(GD, Out);
+auto *Alias = llvm::GlobalAlias::create("", GV);
+Alias->takeName(GV);
+GV->setName(MangledName);
+  }
+
   // We need to set linkage and visibility on the function before
   // generating code for it because various parts of IR generation
   // want to propagate this information down (e.g. to local static
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -298,6 +298,7 @@
 return AnonymousNamespaceHash;
   }
 
+  void mangleArm64ECFnDef(GlobalDecl GD, raw_ostream &) override;
 private:
   void mangleInitFiniStub(const VarDecl *D, char CharCode, raw_ostream );
 };
@@ -360,7 +361,7 @@
 
   void mangle(GlobalDecl GD, StringRef Prefix = "?");
   void mangleName(GlobalDecl GD);
-  void mangleFunctionEncoding(GlobalDecl GD, bool ShouldMangle);
+  void mangleFunctionEncoding(GlobalDecl GD, bool ShouldMangle, bool Arm64ECDef = false);
   void mangleVariableEncoding(const VarDecl *VD);
   void mangleMemberDataPointer(const CXXRecordDecl *RD, const ValueDecl *VD,
StringRef Prefix = "$");
@@ -384,6 +385,7 @@
   bool ForceThisQuals = false,
   bool MangleExceptionSpec = true);
   void mangleNestedName(GlobalDecl GD);
+  void mangleArm64ECFnDef(GlobalDecl GD);
 
 private:
   bool isStructorDecl(const NamedDecl *ND) const {
@@ -573,7 +575,8 @@
 }
 
 void MicrosoftCXXNameMangler::mangleFunctionEncoding(GlobalDecl GD,
- bool ShouldMangle) {
+ bool 

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

I'll split off the TableGen changes into a separate patch. It will supersede 
those changes here, so it shouldn't delay other reviews here. :-)




Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:798
+
+  if (STI.hasSVE()) {
+for (MCRegister PReg :

nickdesaulniers wrote:
> Reuse `HasSVE` from L771?
Doh! Changed.



Comment at: llvm/test/CodeGen/AArch64/zero-call-used-regs.ll:2-3
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown | FileCheck %s 
--check-prefix=DEFAULT
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown -mattr=+sve | FileCheck %s 
--check-prefix=SVE
+

nickdesaulniers wrote:
> If you use `--check-prefixes=CHECK,` (ie. 
> `--check-prefixes=CHECK,DEFAULT` and `--check-prefixes=CHECK,SVE`) then when 
> `DEFAULT` and `SVE` match, you can just use `CHECK`.
> 
> That should help reduce the number of checks in this test significantly. 
> Otherwise it's hard to tell what's different between the two cases, if 
> anything at all.
> 
> update_llc_test_checks should work with --check-prefixes IME.
What hath God wrought?!

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

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

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 428764.
void marked 4 inline comments as done.
void added a comment.

Fix think-o use of HasSVE. Use `--check-prefixes` in the testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
  llvm/lib/Target/AArch64/AArch64FrameLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.td
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/test/CodeGen/AArch64/zero-call-used-regs.ll
  llvm/utils/TableGen/CallingConvEmitter.cpp
  llvm/utils/TableGen/RegisterInfoEmitter.cpp

Index: llvm/utils/TableGen/RegisterInfoEmitter.cpp
===
--- llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -1188,6 +1188,8 @@
  << "MCRegister) const override;\n"
  << "  bool isFixedRegister(const MachineFunction &, "
  << "MCRegister) const override;\n"
+ << "  bool isArgumentRegister(const MachineFunction &, "
+ << "MCRegister) const override;\n"
  << "  /// Devirtualized TargetFrameLowering.\n"
  << "  static const " << TargetName << "FrameLowering *getFrameLowering(\n"
  << "  const MachineFunction );\n"
@@ -1662,6 +1664,20 @@
   OS << "  false;\n";
   OS << "}\n\n";
 
+  OS << "bool " << ClassName << "::\n"
+ << "isArgumentRegister(const MachineFunction , "
+ << "MCRegister PhysReg) const {\n"
+ << "  return\n";
+  for (const CodeGenRegisterCategory  : RegCategories)
+if (Category.getName() == "ArgumentRegisters") {
+  for (const CodeGenRegisterClass *RC : Category.getClasses())
+OS << "  " << RC->getQualifiedName()
+   << "RegClass.contains(PhysReg) ||\n";
+  break;
+}
+  OS << "  false;\n";
+  OS << "}\n\n";
+
   OS << "ArrayRef " << ClassName
  << "::getRegMaskNames() const {\n";
   if (!CSRSets.empty()) {
Index: llvm/utils/TableGen/CallingConvEmitter.cpp
===
--- llvm/utils/TableGen/CallingConvEmitter.cpp
+++ llvm/utils/TableGen/CallingConvEmitter.cpp
@@ -20,6 +20,14 @@
 namespace {
 class CallingConvEmitter {
   RecordKeeper 
+  unsigned Counter;
+  std::string CurrentAction;
+  bool SwiftAction;
+
+  std::map> AssignedRegsMap;
+  std::map> AssignedSwiftRegsMap;
+  std::map> DelegateToMap;
+
 public:
   explicit CallingConvEmitter(RecordKeeper ) : Records(R) {}
 
@@ -28,7 +36,7 @@
 private:
   void EmitCallingConv(Record *CC, raw_ostream );
   void EmitAction(Record *Action, unsigned Indent, raw_ostream );
-  unsigned Counter;
+  void EmitArgRegisterLists(raw_ostream );
 };
 } // End anonymous namespace
 
@@ -38,6 +46,7 @@
   // Emit prototypes for all of the non-custom CC's so that they can forward ref
   // each other.
   Records.startTimer("Emit prototypes");
+  O << "#ifndef GET_CC_REGISTER_LISTS\n\n";
   for (Record *CC : CCs) {
 if (!CC->getValueAsBit("Custom")) {
   unsigned Pad = CC->getName().size();
@@ -58,18 +67,25 @@
   // Emit each non-custom calling convention description in full.
   Records.startTimer("Emit full descriptions");
   for (Record *CC : CCs) {
-if (!CC->getValueAsBit("Custom"))
+if (!CC->getValueAsBit("Custom")) {
+  // Call upon the creation of a map entry from the void!
+  CurrentAction = CC->getName().str();
+  (void)AssignedRegsMap[CurrentAction];
   EmitCallingConv(CC, O);
+}
   }
-}
 
+  EmitArgRegisterLists(O);
+
+  O << "\n#endif // CC_REGISTER_LIST\n";
+}
 
 void CallingConvEmitter::EmitCallingConv(Record *CC, raw_ostream ) {
   ListInit *CCActions = CC->getValueAsListInit("Actions");
   Counter = 0;
 
   O << "\n\n";
-  unsigned Pad = CC->getName().size();
+  unsigned Pad = CurrentAction.size();
   if (CC->getValueAsBit("Entry")) {
 O << "bool llvm::";
 Pad += 12;
@@ -77,13 +93,21 @@
 O << "static bool ";
 Pad += 13;
   }
-  O << CC->getName() << "(unsigned ValNo, MVT ValVT,\n"
+  O << CurrentAction << "(unsigned ValNo, MVT ValVT,\n"
 << std::string(Pad, ' ') << "MVT LocVT, CCValAssign::LocInfo LocInfo,\n"
 << std::string(Pad, ' ') << "ISD::ArgFlagsTy ArgFlags, CCState ) {\n";
   // Emit all of the actions, in order.
   for (unsigned i = 0, e = CCActions->size(); i != e; ++i) {
+Record *Action = CCActions->getElementAsRecord(i);
+SwiftAction = llvm::any_of(Action->getSuperClasses(),
+   [](const std::pair ) {
+ std::string Name =
+ Class.first->getNameInitAsString();
+ return StringRef(Name).startswith("CCIfSwift");
+   });
+
 O << "\n";
-

[PATCH] D124699: [DeadArgElim] Set unused arguments for internal functions

[Quentin Colombet via Phabricator via cfe-commits]

qcolombet added a comment.

Thanks @thakis !

@fhahn are you okay with the clang tests update as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124699

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

[PATCH] D124700: [AMDGPU] Add llvm.amdgcn.sched.barrier intrinsic

[Austin Kerbow via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2db700215a2e: [AMDGPU] Add llvm.amdgcn.sched.barrier 
intrinsic (authored by kerbowa).
Herald added a subscriber: kosarev.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124700

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/test/CodeGenOpenCL/builtins-amdgcn.cl
  clang/test/SemaOpenCL/builtins-amdgcn-error.cl
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  llvm/lib/Target/AMDGPU/SIInstructions.td
  llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp
  llvm/test/CodeGen/AMDGPU/hazard-pseudo-machineinstrs.mir
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sched.barrier.ll
  llvm/test/CodeGen/AMDGPU/sched_barrier.mir

Index: llvm/test/CodeGen/AMDGPU/sched_barrier.mir
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/sched_barrier.mir
@@ -0,0 +1,99 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -mcpu=gfx908 -run-pass=machine-scheduler -verify-misched -o - %s | FileCheck %s
+
+--- |
+  define amdgpu_kernel void @no_sched_barrier(i32 addrspace(1)* noalias %out, i32 addrspace(1)* noalias %in) { ret void }
+  define amdgpu_kernel void @sched_barrier_0(i32 addrspace(1)* noalias %out, i32 addrspace(1)* noalias %in) { ret void }
+  define amdgpu_kernel void @sched_barrier_1(i32 addrspace(1)* noalias %out, i32 addrspace(1)* noalias %in) { ret void }
+
+  !0 = distinct !{!0}
+  !1 = !{!1, !0}
+...
+
+---
+name: no_sched_barrier
+tracksRegLiveness: true
+body: |
+  bb.0:
+; CHECK-LABEL: name: no_sched_barrier
+; CHECK: [[DEF:%[0-9]+]]:sreg_64 = IMPLICIT_DEF
+; CHECK-NEXT: [[DEF1:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 0, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR1:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 512, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR]], [[GLOBAL_LOAD_DWORD_SADDR]], implicit $exec
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_1:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR1]], [[GLOBAL_LOAD_DWORD_SADDR1]], implicit $exec
+; CHECK-NEXT: S_NOP 0
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_]], [[DEF]], 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_1]], [[DEF]], 512, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+; CHECK-NEXT: S_ENDPGM 0
+%0:sreg_64 = IMPLICIT_DEF
+%1:vgpr_32 = IMPLICIT_DEF
+%3:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR %0, %1, 0, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+%4:vgpr_32 = nsw V_MUL_LO_U32_e64 %3, %3, implicit $exec
+GLOBAL_STORE_DWORD_SADDR %1, %4, %0, 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+S_NOP 0
+%5:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR %0, %1, 512, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+%6:vgpr_32 = nsw V_MUL_LO_U32_e64 %5, %5, implicit $exec
+GLOBAL_STORE_DWORD_SADDR %1, %6, %0, 512, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+S_ENDPGM 0
+...
+
+---
+name: sched_barrier_0
+tracksRegLiveness: true
+body: |
+  bb.0:
+; CHECK-LABEL: name: sched_barrier_0
+; CHECK: [[DEF:%[0-9]+]]:sreg_64 = IMPLICIT_DEF
+; CHECK-NEXT: [[DEF1:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 0, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR]], [[GLOBAL_LOAD_DWORD_SADDR]], implicit $exec
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_]], [[DEF]], 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+; CHECK-NEXT: S_NOP 0
+; CHECK-NEXT: SCHED_BARRIER 0
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR1:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 512, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_1:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR1]], [[GLOBAL_LOAD_DWORD_SADDR1]], implicit $exec
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_1]], [[DEF]], 512, 0, implicit $exec :: (store (s32) into %ir.out, 

[clang] 2db7002 - [AMDGPU] Add llvm.amdgcn.sched.barrier intrinsic

[Austin Kerbow via cfe-commits]

Author: Austin Kerbow
Date: 2022-05-11T13:22:51-07:00
New Revision: 2db700215a2eebce7358c0a81a3d52d0a9d4a997

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

LOG: [AMDGPU] Add llvm.amdgcn.sched.barrier intrinsic

Adds an intrinsic/builtin that can be used to fine tune scheduler behavior. If
there is a need to have highly optimized codegen and kernel developers have
knowledge of inter-wave runtime behavior which is unknown to the compiler this
builtin can be used to tune scheduling.

This intrinsic creates a barrier between scheduling regions. The immediate
parameter is a mask to determine the types of instructions that should be
prevented from crossing the sched_barrier. In this initial patch, there are only
two variations. A mask of 0 means that no instructions may be scheduled across
the sched_barrier. A mask of 1 means that non-memory, non-side-effect inducing
instructions may cross the sched_barrier.

Note that this intrinsic is only meant to work with the scheduling passes. Any
other transformations that may move code will not be impacted in the ways
described above.

Reviewed By: rampitec

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

Added: 
llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sched.barrier.ll
llvm/test/CodeGen/AMDGPU/sched_barrier.mir

Modified: 
clang/include/clang/Basic/BuiltinsAMDGPU.def
clang/test/CodeGenOpenCL/builtins-amdgcn.cl
clang/test/SemaOpenCL/builtins-amdgcn-error.cl
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
llvm/lib/Target/AMDGPU/SIInstructions.td
llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp
llvm/test/CodeGen/AMDGPU/hazard-pseudo-machineinstrs.mir

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsAMDGPU.def 
b/clang/include/clang/Basic/BuiltinsAMDGPU.def
index afcfa07f6df13..19e4ea998aa47 100644
--- a/clang/include/clang/Basic/BuiltinsAMDGPU.def
+++ b/clang/include/clang/Basic/BuiltinsAMDGPU.def
@@ -62,6 +62,7 @@ BUILTIN(__builtin_amdgcn_s_sendmsg, "vIiUi", "n")
 BUILTIN(__builtin_amdgcn_s_sendmsghalt, "vIiUi", "n")
 BUILTIN(__builtin_amdgcn_s_barrier, "v", "n")
 BUILTIN(__builtin_amdgcn_wave_barrier, "v", "n")
+BUILTIN(__builtin_amdgcn_sched_barrier, "vIi", "n")
 BUILTIN(__builtin_amdgcn_s_dcache_inv, "v", "n")
 BUILTIN(__builtin_amdgcn_buffer_wbinvl1, "v", "n")
 BUILTIN(__builtin_amdgcn_ds_gws_init, "vUiUi", "n")

diff  --git a/clang/test/CodeGenOpenCL/builtins-amdgcn.cl 
b/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
index 7d1509e4c4cb0..9853045ea19f9 100644
--- a/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
+++ b/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -396,6 +396,19 @@ void test_wave_barrier()
   __builtin_amdgcn_wave_barrier();
 }
 
+// CHECK-LABEL: @test_sched_barrier
+// CHECK: call void @llvm.amdgcn.sched.barrier(i32 0)
+// CHECK: call void @llvm.amdgcn.sched.barrier(i32 1)
+// CHECK: call void @llvm.amdgcn.sched.barrier(i32 4)
+// CHECK: call void @llvm.amdgcn.sched.barrier(i32 15)
+void test_sched_barrier()
+{
+  __builtin_amdgcn_sched_barrier(0);
+  __builtin_amdgcn_sched_barrier(1);
+  __builtin_amdgcn_sched_barrier(4);
+  __builtin_amdgcn_sched_barrier(15);
+}
+
 // CHECK-LABEL: @test_s_sleep
 // CHECK: call void @llvm.amdgcn.s.sleep(i32 1)
 // CHECK: call void @llvm.amdgcn.s.sleep(i32 15)

diff  --git a/clang/test/SemaOpenCL/builtins-amdgcn-error.cl 
b/clang/test/SemaOpenCL/builtins-amdgcn-error.cl
index af4fdf32c272f..1351ab58c1f9a 100644
--- a/clang/test/SemaOpenCL/builtins-amdgcn-error.cl
+++ b/clang/test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -60,6 +60,11 @@ void test_s_setprio(int x)
   __builtin_amdgcn_s_setprio(65536); // expected-warning {{implicit conversion 
from 'int' to 'short' changes value from 65536 to 0}}
 }
 
+void test_sched_barrier(int x)
+{
+  __builtin_amdgcn_sched_barrier(x); // expected-error {{argument to 
'__builtin_amdgcn_sched_barrier' must be a constant integer}}
+}
+
 void test_sicmp_i32(global ulong* out, int a, int b, uint c)
 {
   *out = __builtin_amdgcn_sicmp(a, b, c); // expected-error {{argument to 
'__builtin_amdgcn_sicmp' must be a constant integer}}

diff  --git a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td 
b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
index f148fda5b37f5..df552982a4ee9 100644
--- a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
+++ b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
@@ -213,6 +213,15 @@ def int_amdgcn_s_barrier : 
GCCBuiltin<"__builtin_amdgcn_s_barrier">,
 def int_amdgcn_wave_barrier : GCCBuiltin<"__builtin_amdgcn_wave_barrier">,
   Intrinsic<[], [], [IntrNoMem, IntrHasSideEffects, IntrConvergent, 
IntrWillReturn]>;
 
+// The 1st parameter is a mask for the types of instructions that may be 
allowed
+// to cross the SCHED_BARRIER 

[PATCH] D125414: Revert "[HLSL] add -D option for dxc mode."

[Xiang Li via Phabricator via cfe-commits]

python3kgae created this revision.
Herald added a subscriber: Anastasia.
Herald added a project: All.
python3kgae requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This reverts commit 4dae38ebfba0d8583e52c3ded8f62f5f9fa77fda 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125414

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/dxc_D.hlsl


Index: clang/test/Driver/dxc_D.hlsl
===
--- clang/test/Driver/dxc_D.hlsl
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
-// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s 
--check-prefix=ERROR
-
-// Make sure -D send to cc1.
-// CHECK:"-D" "TEST=2"
-
-#ifndef TEST
-#error "TEST not defined"
-#elif TEST != 2
-#error "TEST defined to wrong value"
-#endif
-
-// ERROR-NOT: error:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {
-  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
-  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
+ options::OPT_S, 
options::OPT_emit_llvm,
+ options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6756,8 +6756,6 @@
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
-def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
-  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM 
passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;


Index: clang/test/Driver/dxc_D.hlsl
===
--- clang/test/Driver/dxc_D.hlsl
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
-// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s --check-prefix=ERROR
-
-// Make sure -D send to cc1.
-// CHECK:"-D" "TEST=2"
-
-#ifndef TEST
-#error "TEST not defined"
-#elif TEST != 2
-#error "TEST defined to wrong value"
-#endif
-
-// ERROR-NOT: error:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {
-  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
-  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
+ options::OPT_S, options::OPT_emit_llvm,
+ options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6756,8 +6756,6 @@
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
-def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
-  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125338: [HLSL] add -D option for dxc mode.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D125338#3507063 , @thakis wrote:

> Looks like this breaks tests on Mac: http://45.33.8.238/macm1/35103/step_7.txt
>
> Please take a look and revert for now if it takes a while to fix.

I'll revert it. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125338

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

[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1044
+
+  // Unfortunately, debug_aranges by default don't inclue global variables. If
+  // we failed to find the CU using aranges, try and search for variables as

Might be worth a more precise statement here - at least my understanding is 
that GCC never puts global variables in aranges and clang always does (but it's 
not possible to differentiate the situations, making aranges untrustworthy for 
this query - so the code here is correct if it's got to handle GCC, not 
suggesting any code change, but maybe more info in the comment to point to GCC 
for a concrete example of the issue)



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:769-783
+DataExtractor Data(Location.Expr, /*IsLittleEndian=*/true, 8);
+uint64_t DataOffset = 0;
+uint8_t Operation = Data.getU8();
+if (Operation == dwarf::DW_OP_addr) {
+  uint64_t Pointer = Data.getAddress();
+  VariableDieMap[Pointer] = Die;
+  return;

Are there any examples of global merge where this might not be correct?

Like if a global was described as "addrx, 5, add" (eg: 5 beyond this address) 
then only looking at the first operation might mis-conclude that the variable 
is at this address when it's at 5 bytes past this address - and some other 
variable is at this address)

LLVM has a "global merge" optimization that might cause something like this. 
Let's see if I can create an example.

Ah, yeah, here we go:
```
static int x;
static int y;
int *f(bool b) { return b ?  :  }
```
```
$ clang++-tot -O3 merge.cpp -c -g -target aarch64-linux-gnuabi -mllvm 
-global-merge-ignore-single-use && llvm-dwarfdump-tot merge.o | grep 
DW_AT_location
DW_AT_location  (DW_OP_addrx 0x0)
DW_AT_location  (DW_OP_addrx 0x0, DW_OP_plus_uconst 0x4)
```

(the `-global-merge-ignore-single-use` is to simplify the test case - without 
that it could still be tickled with a more complicated example - seems we only 
enable global merge on ARM 32 and 64 bit due to the higher register pressure 
there, by the sounds of it)

I'm guessing this patch would overwrite the VariableDieMap entry for the first 
global with the second one so queries for the first address would result in the 
second DIE and queries for the second address wouldn't give any result?

Hmm, also - how does this handle queries that aren't at exactly the starting 
address of the variable? Presumably the `DW_AT_type` of the 
`DW_TAG_global_variable` would have to be inspected to find the size of the 
variable starting at the address, and any query in that range should be 
successful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

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

[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.
Herald added a project: All.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113793

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

[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:787
+
+  const int Length = LastToken->TotalLength;
+

owenpan wrote:
> curdeius wrote:
> > Why not like this?
> > Why not like this?
> 
> See the assertion on line 781 above. We are computing the `TotalLength` of 
> `LastToken` via `Line`. Either way works, but I prefer the simpler 
> expression. I can change it though if you insist.
Yeah, what I meant is to just get rid of `LastToken` variable and write the 
suggested code.
But well, both ways work. No strong feelings.


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

https://reviews.llvm.org/D125137

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

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

In D123878#3506500 , @afanfa wrote:

> If possible, I would like to keep some kind of delimiter. I like the idea of 
> having it at the beginning and at the end of the section. The best option 
> would be to convince clang to print new lines.

It seems like the stripping of non-printable characters is intentional, but 
only for diagnostics that don't use the TableGen based diagnostic formatting 
scheme in clang. I'm not sure exactly why, but regardless there is precedent 
for newlines in other messages.

To get the newlines to hit the terminal for our case the following patch is 
enough:

  --- a/clang/lib/Basic/Diagnostic.cpp
  +++ b/clang/lib/Basic/Diagnostic.cpp
  @@ -812,7 +812,7 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
 getArgKind(0) == DiagnosticsEngine::ak_std_string) {   


   const std::string  = getArgStdStr(0);  


   for (char c : S) {
  -  if (llvm::sys::locale::isPrint(c) || c == '\t') {
  +  if (llvm::sys::locale::isPrint(c) || c == '\t' || c == '\n') {
   OutStr.push_back(c);
 }
   }

To get the right indentation inserted for the extra lines the `TextDiagnostic` 
consumer needs to also be updated, but that should be a small change. Only one 
test breaks, and it seems useful for it to pick up the new behavior anyway.

The only other potential conflict is that in `TextDiagnostic` specifically 
there is also the Clang option `-fmessage-length=N` which will forcibly wrap 
diagnostic messages on word-boundaries (although never breaking up a word 
across lines). It seems to only apply to text preceding the first newline in 
the message, so it is likely just a non-issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

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

[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:766
+  assert(!Tokens.empty());
+  const auto *LastToken = Tokens.back().Tok;
+  assert(LastToken);

curdeius wrote:
> It might be a matter of taste but adding this variable makes the code harder 
> to read to me.
The last token of `ParsedLine` is of interest here. We want the annotator to 
compute its `TotalLength` to determine whether the line might fit on a single 
line. I'm open to renaming the variable if you have a better suggestion.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:777
+SavedToken.Tok->copyFrom(*Token.Tok);
+SavedToken.Children = std::move(Token.Children);
+  }

curdeius wrote:
> So the token's children are modified (moved)? Is it done so that children be 
> not considered by the annotator?
Both the constructor and the destructor of `AnnotatedLine` clear the children 
of `UnwrappedLineNode`, so we save them beforehand and restore them afterward.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:787
+
+  const int Length = LastToken->TotalLength;
+

curdeius wrote:
> Why not like this?
> Why not like this?

See the assertion on line 781 above. We are computing the `TotalLength` of 
`LastToken` via `Line`. Either way works, but I prefer the simpler expression. 
I can change it though if you insist.


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

https://reviews.llvm.org/D125137

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

[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Chris Lattner via Phabricator via cfe-commits]

lattner edited reviewers, added: rsmith; removed: lattner.
lattner added a comment.

I'm not a competent reviewer for this, Richard can you recommend someone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125402

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

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added inline comments.



Comment at: llvm/test/CodeGen/AArch64/zero-call-used-regs.ll:259-262
+; SVE-NEXT:pfalse p0.b
+; SVE-NEXT:pfalse p1.b
+; SVE-NEXT:pfalse p2.b
+; SVE-NEXT:pfalse p3.b

nickdesaulniers wrote:
> N00b question about SVE: do we need `pfalse` for each of the numbered p 
> registers corresponding to the x registers we zeroed? i.e. here we have 
> pfalse for p0-3, yet we zero z0-7.
No, the set of p registers are independent of the z registers.

The calling convention states [0] that the predicate registers p0-p3 may be 
used for parameter passing (if you have an argument which belongs in a p 
register), so this looks reasonable.

[0] 
https://github.com/ARM-software/abi-aa/blob/8a7b266879c60ca1c76e94ebb279b2dac60ed6a5/aapcs64/aapcs64.rst#scalable-predicate-registers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

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

[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM with nits.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:766
+  assert(!Tokens.empty());
+  const auto *LastToken = Tokens.back().Tok;
+  assert(LastToken);

It might be a matter of taste but adding this variable makes the code harder to 
read to me.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:777
+SavedToken.Tok->copyFrom(*Token.Tok);
+SavedToken.Children = std::move(Token.Children);
+  }

So the token's children are modified (moved)? Is it done so that children be 
not considered by the annotator?



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:787
+
+  const int Length = LastToken->TotalLength;
+

Why not like this?


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

https://reviews.llvm.org/D125137

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

[PATCH] D125338: [HLSL] add -D option for dxc mode.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests on Mac: http://45.33.8.238/macm1/35103/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125338

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

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

I still think this would be easier to review if the `isArgumentRegister` 
tablegen changes were separated out into a distinct parent patch and then the 
existing x86 implementation updated to use, then this would rebased on top of 
as a child patch.




Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:798
+
+  if (STI.hasSVE()) {
+for (MCRegister PReg :

Reuse `HasSVE` from L771?



Comment at: llvm/test/CodeGen/AArch64/zero-call-used-regs.ll:2-3
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown | FileCheck %s 
--check-prefix=DEFAULT
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown -mattr=+sve | FileCheck %s 
--check-prefix=SVE
+

If you use `--check-prefixes=CHECK,` (ie. 
`--check-prefixes=CHECK,DEFAULT` and `--check-prefixes=CHECK,SVE`) then when 
`DEFAULT` and `SVE` match, you can just use `CHECK`.

That should help reduce the number of checks in this test significantly. 
Otherwise it's hard to tell what's different between the two cases, if anything 
at all.

update_llc_test_checks should work with --check-prefixes IME.



Comment at: llvm/test/CodeGen/AArch64/zero-call-used-regs.ll:259-262
+; SVE-NEXT:pfalse p0.b
+; SVE-NEXT:pfalse p1.b
+; SVE-NEXT:pfalse p2.b
+; SVE-NEXT:pfalse p3.b

N00b question about SVE: do we need `pfalse` for each of the numbered p 
registers corresponding to the x registers we zeroed? i.e. here we have pfalse 
for p0-3, yet we zero z0-7.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

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

[PATCH] D124702: [MSVC] Add support for pragma function

[Stephen Long via Phabricator via cfe-commits]

steplong added inline comments.



Comment at: clang/test/Preprocessor/pragma_microsoft.c:210
+#pragma function(main)   // expected-warning {{'main' is not a 
recognized builtin; consider including }}
+#pragma function(// expected-warning {{missing ')' 
after}}
+#pragma function(int)// expected-warning {{missing ')' 
after}}

Hmm does it make sense for this to be a warning and not an error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

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

[PATCH] D125386: [clang][ppc] Creating Seperate Install Target for PPC htm Headers

[Qiongsi Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ca6328637b3: [clang][ppc] Creating Seperate Install Target 
for PPC htm Headers (authored by qiongsiwu1).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125386

Files:
  clang/lib/Headers/CMakeLists.txt


Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -73,6 +73,9 @@
 
 set(ppc_files
   altivec.h
+  )
+
+set(ppc_htm_files
   htmintrin.h
   htmxlintrin.h
   )
@@ -212,6 +215,7 @@
   ${mips_msa_files}
   ${opencl_files}
   ${ppc_files}
+  ${ppc_htm_files}
   ${systemz_files}
   ${ve_files}
   ${x86_files}
@@ -370,6 +374,7 @@
  "hip-resource-headers"
  "mips-resource-headers"
  "ppc-resource-headers"
+ "ppc-htm-resource-headers"
  "riscv-resource-headers"
  "systemz-resource-headers"
  "ve-resource-headers"
@@ -392,6 +397,7 @@
 add_header_target("hip-resource-headers" "${hip_files}")
 add_header_target("mips-resource-headers" "${mips_msa_files}")
 add_header_target("ppc-resource-headers" "${ppc_files};${ppc_wrapper_files}")
+add_header_target("ppc-htm-resource-headers" "${ppc_htm_files}")
 add_header_target("riscv-resource-headers" "${riscv_generated_files}")
 add_header_target("systemz-resource-headers" "${systemz_files}")
 add_header_target("ve-resource-headers" "${ve_files}")
@@ -491,11 +497,17 @@
   COMPONENT ppc-resource-headers)
 
 install(
-  FILES ${ppc_files} ${utility_files}
+  FILES ${ppc_files}
   DESTINATION ${header_install_dir}
   EXCLUDE_FROM_ALL
   COMPONENT ppc-resource-headers)
 
+install(
+  FILES ${ppc_htm_files}
+  DESTINATION ${header_install_dir}
+  EXCLUDE_FROM_ALL
+  COMPONENT ppc-htm-resource-headers)
+
 install(
   FILES ${riscv_generated_files}
   DESTINATION ${header_install_dir}
@@ -583,6 +595,9 @@
   add_llvm_install_targets(install-ppc-resource-headers
DEPENDS ppc-resource-headers
COMPONENT ppc-resource-headers)
+  add_llvm_install_targets(install-ppc-htm-resource-headers
+   DEPENDS ppc-htm-resource-headers
+   COMPONENT ppc-htm-resource-headers)
   add_llvm_install_targets(install-riscv-resource-headers
DEPENDS riscv-resource-headers
COMPONENT riscv-resource-headers)


Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -73,6 +73,9 @@
 
 set(ppc_files
   altivec.h
+  )
+
+set(ppc_htm_files
   htmintrin.h
   htmxlintrin.h
   )
@@ -212,6 +215,7 @@
   ${mips_msa_files}
   ${opencl_files}
   ${ppc_files}
+  ${ppc_htm_files}
   ${systemz_files}
   ${ve_files}
   ${x86_files}
@@ -370,6 +374,7 @@
  "hip-resource-headers"
  "mips-resource-headers"
  "ppc-resource-headers"
+ "ppc-htm-resource-headers"
  "riscv-resource-headers"
  "systemz-resource-headers"
  "ve-resource-headers"
@@ -392,6 +397,7 @@
 add_header_target("hip-resource-headers" "${hip_files}")
 add_header_target("mips-resource-headers" "${mips_msa_files}")
 add_header_target("ppc-resource-headers" "${ppc_files};${ppc_wrapper_files}")
+add_header_target("ppc-htm-resource-headers" "${ppc_htm_files}")
 add_header_target("riscv-resource-headers" "${riscv_generated_files}")
 add_header_target("systemz-resource-headers" "${systemz_files}")
 add_header_target("ve-resource-headers" "${ve_files}")
@@ -491,11 +497,17 @@
   COMPONENT ppc-resource-headers)
 
 install(
-  FILES ${ppc_files} ${utility_files}
+  FILES ${ppc_files}
   DESTINATION ${header_install_dir}
   EXCLUDE_FROM_ALL
   COMPONENT ppc-resource-headers)
 
+install(
+  FILES ${ppc_htm_files}
+  DESTINATION ${header_install_dir}
+  EXCLUDE_FROM_ALL
+  COMPONENT ppc-htm-resource-headers)
+
 install(
   FILES ${riscv_generated_files}
   DESTINATION ${header_install_dir}
@@ -583,6 +595,9 @@
   add_llvm_install_targets(install-ppc-resource-headers
DEPENDS ppc-resource-headers
COMPONENT ppc-resource-headers)
+  add_llvm_install_targets(install-ppc-htm-resource-headers
+   DEPENDS ppc-htm-resource-headers
+   COMPONENT ppc-htm-resource-headers)
   add_llvm_install_targets(install-riscv-resource-headers
DEPENDS riscv-resource-headers
COMPONENT riscv-resource-headers)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang] 3ca6328 - [clang][ppc] Creating Seperate Install Target for PPC htm Headers

[Qiongsi Wu via cfe-commits]

Author: Qiongsi Wu
Date: 2022-05-11T14:48:40-04:00
New Revision: 3ca6328637b3f42096c652e4df53282649956bdb

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

LOG: [clang][ppc] Creating Seperate Install Target for PPC htm Headers

This patch splits out the htm intrinsic headers from the PPC headers list.

Reviewed By: jsji

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

Added: 


Modified: 
clang/lib/Headers/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Headers/CMakeLists.txt 
b/clang/lib/Headers/CMakeLists.txt
index bb24b4274380..08c5dccb3b77 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -73,6 +73,9 @@ set(opencl_files
 
 set(ppc_files
   altivec.h
+  )
+
+set(ppc_htm_files
   htmintrin.h
   htmxlintrin.h
   )
@@ -212,6 +215,7 @@ set(files
   ${mips_msa_files}
   ${opencl_files}
   ${ppc_files}
+  ${ppc_htm_files}
   ${systemz_files}
   ${ve_files}
   ${x86_files}
@@ -370,6 +374,7 @@ add_dependencies("clang-resource-headers"
  "hip-resource-headers"
  "mips-resource-headers"
  "ppc-resource-headers"
+ "ppc-htm-resource-headers"
  "riscv-resource-headers"
  "systemz-resource-headers"
  "ve-resource-headers"
@@ -392,6 +397,7 @@ add_header_target("hexagon-resource-headers" 
"${hexagon_files}")
 add_header_target("hip-resource-headers" "${hip_files}")
 add_header_target("mips-resource-headers" "${mips_msa_files}")
 add_header_target("ppc-resource-headers" "${ppc_files};${ppc_wrapper_files}")
+add_header_target("ppc-htm-resource-headers" "${ppc_htm_files}")
 add_header_target("riscv-resource-headers" "${riscv_generated_files}")
 add_header_target("systemz-resource-headers" "${systemz_files}")
 add_header_target("ve-resource-headers" "${ve_files}")
@@ -491,11 +497,17 @@ install(
   COMPONENT ppc-resource-headers)
 
 install(
-  FILES ${ppc_files} ${utility_files}
+  FILES ${ppc_files}
   DESTINATION ${header_install_dir}
   EXCLUDE_FROM_ALL
   COMPONENT ppc-resource-headers)
 
+install(
+  FILES ${ppc_htm_files}
+  DESTINATION ${header_install_dir}
+  EXCLUDE_FROM_ALL
+  COMPONENT ppc-htm-resource-headers)
+
 install(
   FILES ${riscv_generated_files}
   DESTINATION ${header_install_dir}
@@ -583,6 +595,9 @@ if (NOT LLVM_ENABLE_IDE)
   add_llvm_install_targets(install-ppc-resource-headers
DEPENDS ppc-resource-headers
COMPONENT ppc-resource-headers)
+  add_llvm_install_targets(install-ppc-htm-resource-headers
+   DEPENDS ppc-htm-resource-headers
+   COMPONENT ppc-htm-resource-headers)
   add_llvm_install_targets(install-riscv-resource-headers
DEPENDS riscv-resource-headers
COMPONENT riscv-resource-headers)



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

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:5037
   if (auto builtinID = FD->getBuiltinID()) {
+std::string AttributeNoBuiltin = "no-builtin-" + FD->getName().str();
 std::string FDInlineName = (FD->getName() + ".inline").str();

Might as well use a Twine since the next line uses one as well.



Comment at: clang/test/CodeGen/no-builtin-2.c:38
+}
+
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memset"{{.*}}}

We should have test coverage for the wildcard form of the attribute.


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

https://reviews.llvm.org/D124701

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

[PATCH] D125011: [MSVC] Add support for pragma alloc_text

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 428727.
steplong added a comment.

- Reject `#pragma alloc_text(a, a`
- On Line 1180, I didn't use `ExpectAndConsume()` there because I wanted to 
accept both identifiers and strings (i.e `pragma alloc_text(a, foo)` and 
`pragma alloc_text("a", foo)`
- It looks like we need `->getRedeclContext()`  to accept

  extern "C" {
  void foo();
  #pragma alloc_text(a, foo)
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125011

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/msvc_pragma_alloc_text.cpp
  clang/test/Sema/pragma-ms-alloc-text.cpp

Index: clang/test/Sema/pragma-ms-alloc-text.cpp
===
--- /dev/null
+++ clang/test/Sema/pragma-ms-alloc-text.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+
+#pragma alloc_text()// expected-warning {{expected a string literal for the section name in '#pragma alloc_text'}}
+#pragma alloc_text(a// expected-warning {{expected ',' in '#pragma alloc_text'}}
+#pragma alloc_text(a, a // expected-warning {{missing ')' after '#pragma alloc_text'}}
+#pragma alloc_text(a, a)// expected-error {{use of undeclared a}}
+#pragma alloc_text(L"a", a) // expected-warning {{expected a string literal for the section name}}
+
+void foo();
+#pragma alloc_text(a, foo) // expected-error {{'#pragma alloc_text' is applicable only to functions with C linkage}}
+
+extern "C" void foo1();
+#pragma alloc_text(a, foo1)  // no-warning
+#pragma alloc_text(a, foo1) asdf // expected-warning {{extra tokens at end of '#pragma alloc_text'}}
+#pragma alloc_text(a, foo1   // expected-warning {{missing ')' after '#pragma alloc_text'}}
+
+namespace N {
+#pragma alloc_text(b, foo1) // no-warning
+}
+
+extern "C" {
+void foo2();
+#pragma alloc_text(a, foo2) // no-warning
+}
+
+void foo3() {
+#pragma alloc_text(a, foo1) // expected-error {{'#pragma alloc_text' can only appear at file scope}}
+}
+
+extern "C" void foo4();
+#pragma alloc_text(c, foo4) // no-warning
+void foo4() {}
+
+void foo5();// expected-note {{previous declaration is here}}
+#pragma alloc_text(c, foo5) // expected-error {{'#pragma alloc_text' is applicable only to functions with C linkage}}
+extern "C" void foo5() {}   // expected-error {{declaration of 'foo5' has a different language linkage}}
Index: clang/test/CodeGen/msvc_pragma_alloc_text.cpp
===
--- /dev/null
+++ clang/test/CodeGen/msvc_pragma_alloc_text.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fms-extensions -S -emit-llvm -o - %s | FileCheck %s
+
+extern "C" {
+
+void foo();
+void foo1();
+void foo2();
+void foo3();
+
+#pragma alloc_text("abc", foo, foo1)
+#pragma alloc_text("def", foo2)
+#pragma alloc_text("def", foo3)
+
+// CHECK-LABEL: define{{.*}} void @foo() {{.*}} section "abc"
+void foo() {}
+
+// CHECK-LABEL: define{{.*}} void @foo1() {{.*}} section "abc"
+void foo1() {}
+
+// CHECK-LABEL: define{{.*}} void @foo2() {{.*}} section "def"
+void foo2() {}
+
+// CHECK-LABEL: define{{.*}} void @foo3() {{.*}} section "def"
+void foo3() {}
+
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10220,8 +10220,10 @@
 
   // If this is a function definition, check if we have to apply optnone due to
   // a pragma.
-  if(D.isFunctionDefinition())
+  if(D.isFunctionDefinition()) {
 AddRangeBasedOptnone(NewFD);
+AddSectionMSAllocText(NewFD);
+  }
 
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -741,6 +741,37 @@
   CurInitSegLoc = PragmaLocation;
 }
 
+void Sema::ActOnPragmaMSAllocText(
+SourceLocation PragmaLocation, StringRef Section,
+const SmallVector>
+) {
+  if (!CurContext->getRedeclContext()->isFileContext()) {
+Diag(PragmaLocation, diag::err_pragma_expected_file_scope) << "alloc_text";
+return;
+  }
+
+  for (auto  : Functions) {
+IdentifierInfo *II;
+SourceLocation Loc;
+std::tie(II, Loc) = Function;
+
+DeclarationName DN(II);
+NamedDecl *ND = LookupSingleName(TUScope, DN, Loc, LookupOrdinaryName);
+if (!ND) {
+  Diag(Loc, diag::err_undeclared_use) << II->getName();
+  return;
+}
+
+DeclContext *DC = ND->getDeclContext();
+if (!DC->isExternCContext()) {
+  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
+  return;
+}
+
+

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D125378#3506954 , @aaron.ballman 
wrote:

> In D125378#3506446 , @jdoerfert 
> wrote:
>
>> What I would suggest, if you want to use C/C++ attributes (which makes sense 
>> to me), is an attribute that avoids undef or introduces frozen:
>
> I would prefer to avoid this approach for the moment. Such an attribute is 
> highly focused on how the backend works and I'm not convinced that users in 
> general would know when or how to properly use it. I believe all of the 
> functions which need to be marked are known, and so we can keep an internal 
> list of "interesting" functions that need special attention during clang 
> codegen. If we later find that there's a wider need that cannot be handled 
> transparently like this, we can reevaluate what to expose to users via an 
> attribute at that point.

Works for me.


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

https://reviews.llvm.org/D125378

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

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl updated this revision to Diff 428731.
rsundahl marked 6 inline comments as done.
rsundahl added a comment.

Update diff back to dc7e02d4b4dc30d44ddebd832719a6e63396e718


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125195

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  compiler-rt/lib/asan/asan_poisoning.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp

Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
@@ -1,16 +1,22 @@
-// Test that we do not poison the array cookie if the operator new is defined
-// inside the class.
-// RUN: %clangxx_asan  %s -o %t && %run %t
+// For arrays allocated by classes that define their own custom "new", ASAN
+// should NOT poison the array cookie unless the compilation unit is compiled
+// with "-fsanitize-address-poison-custom-array-cookie".
+// Here we test that:
+//   1) w/o ASan, the array cookie location IS writable
+//   2) w/ASan, the array cookie location IS writeable by default
+//   3) w/ASan, the flag "-fno-sanitize-address-poison-custom-array-cookie"
+//  is a NOP (because this is the default) and finally,
+//   4) w/ASan AND "-fsanitize-address-poison-custom-array-cookie", the
+//  array cookie location is NOT writable (ASAN successfully stopped it)
 //
-// XFAIL: arm
+// RUN: %clangxx   %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan  %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -fno-sanitize-address-poison-custom-array-cookie %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -fsanitize-address-poison-custom-array-cookie%s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=COOKIE
 
-// UNSUPPORTED: ios
-
-#include 
-#include 
-#include 
-#include 
 #include 
+#include 
+
 struct Foo {
   void *operator new(size_t s) { return Allocate(s); }
   void *operator new[] (size_t s) { return Allocate(s); }
@@ -22,7 +28,6 @@
   }
 };
 
-Foo::~Foo() {}
 void *Foo::allocated;
 
 Foo *getFoo(size_t n) {
@@ -33,8 +38,12 @@
   Foo *foo = getFoo(10);
   fprintf(stderr, "foo  : %p\n", foo);
   fprintf(stderr, "alloc: %p\n", Foo::allocated);
-  assert(reinterpret_cast(foo) ==
- reinterpret_cast(Foo::allocated) + sizeof(void*));
-  *reinterpret_cast(Foo::allocated) = 42;
+  reinterpret_cast(foo)[-1] = 42;
+// COOKIE: AddressSanitizer: heap-buffer-overflow
+// COOKIE: in main {{.*}}new_array_cookie_with_new_from_class.cpp:[[@LINE-2]]
+// COOKIE: is located {{7 bytes inside of 18|15 bytes inside of 26}}-byte region
+  printf("Unsurprisingly, we were able to write to the array cookie\n");
+// CHECK: Unsurprisingly, we were able to write to the array cookie
+
   return 0;
 }
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
@@ -3,15 +3,10 @@
 // RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1  | FileCheck %s --check-prefix=COOKIE
 // RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1  | FileCheck %s --check-prefix=NO_COOKIE
 
-// UNSUPPORTED: ios
-
-// Added to allow enabling of tests but needs investigation (rdar://91448627)
-// XFAIL: (darwin && (arm64-target-arch || arm64e-target-arch))
-
-#include 
-#include 
 #include 
-int dtor_counter;
+#include 
+
+int dtor_counter=0;
 struct C {
   int x;
   ~C() {
@@ -22,12 +17,11 @@
 
 __attribute__((noinline)) void Delete(C *c) { delete[] c; }
 __attribute__((no_sanitize_address)) void Write42ToCookie(C *c) {
-  long *p = reinterpret_cast(c);
-  p[-1] = 42;
+  reinterpret_cast(c)[-1] = 42;
 }
 
 int main(int argc, char **argv) {
-  C *buffer = new C[argc];
+  C *buffer = new C[1];
   delete [] buffer;
   Write42ToCookie(buffer);
   delete [] buffer;
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
@@ -1,16 +1,12 @@
 // REQUIRES: asan-64-bits
 // RUN: %clangxx_asan -O3 %s -o %t
-// RUN:not %run %t 2>&1  | FileCheck %s
+// RUN:  not %run %t 2>&1  | FileCheck %s
 // RUN: 

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D125378#3506446 , @jdoerfert wrote:

> What I would suggest, if you want to use C/C++ attributes (which makes sense 
> to me), is an attribute that avoids undef or introduces frozen:

I would prefer to avoid this approach for the moment. Such an attribute is 
highly focused on how the backend works and I'm not convinced that users in 
general would know when or how to properly use it. I believe all of the 
functions which need to be marked are known, and so we can keep an internal 
list of "interesting" functions that need special attention during clang 
codegen. If we later find that there's a wider need that cannot be handled 
transparently like this, we can reevaluate what to expose to users via an 
attribute at that point.


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

https://reviews.llvm.org/D125378

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

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/docs/ClangOffloadPackager.rst:31-32
+uint32_t Flags;
+StringMap StringData;
+MemoryBufferRef Image;
+  };

yaxunl wrote:
> jhuber6 wrote:
> > yaxunl wrote:
> > > This makes the file format depend on LLVM version and potentially 
> > > standard C++ library version.
> > > 
> > > If it is consumed by the same version of LLVM, it may be fine.
> > > 
> > > However, if it is intended for a generic file format to be consumed by 
> > > generic offloading language runtimes, it is better to use C-like simple 
> > > data layout which does not depend on LLVM or standard C++ library.
> > That format just stores the data before it's serialized to a binary format. 
> > The binary format is basically just a few headers and a string table. See 
> > https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Object/OffloadBinary.h#L91
> >  for the real format. I didn't want to explain it all in detail here.
> If the file format is intended to be consumed by generic offloading language 
> runtimes or development tools, better to describe its layout like 
> https://clang.llvm.org/docs/ClangOffloadBundler.html
> 
> Since language runtimes or development tools may not use LLVM to load the 
> file. The documentation serves as a spec for this binary format.
> 
> Especially, it is not clear where to find the target triple and GPU arch for 
> each image in this documentation.
Sure, I'll add some more comprehensive documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125165

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

[PATCH] D125396: [clang] Fix KEYALL

[Yaxun Liu 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 rG84db35594953: [clang] Fix KEYALL (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125396

Files:
  clang/lib/Basic/IdentifierTable.cpp


Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -109,9 +109,10 @@
 KEYMSCOMPAT   = 0x80,
 KEYSYCL   = 0x100,
 KEYCUDA   = 0x200,
+KEYMAX= KEYCUDA, // The maximum key
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
-KEYALL = (0x1ff & ~KEYNOMS18 &
-  ~KEYNOOPENCL) // KEYNOMS18 and KEYNOOPENCL are used to exclude.
+KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYNOMS18 &
+ ~KEYNOOPENCL // KEYNOMS18 and KEYNOOPENCL are used to exclude.
   };
 
   /// How a keyword is treated in the selected standard.


Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -109,9 +109,10 @@
 KEYMSCOMPAT   = 0x80,
 KEYSYCL   = 0x100,
 KEYCUDA   = 0x200,
+KEYMAX= KEYCUDA, // The maximum key
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
-KEYALL = (0x1ff & ~KEYNOMS18 &
-  ~KEYNOOPENCL) // KEYNOMS18 and KEYNOOPENCL are used to exclude.
+KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYNOMS18 &
+ ~KEYNOOPENCL // KEYNOMS18 and KEYNOOPENCL are used to exclude.
   };
 
   /// How a keyword is treated in the selected standard.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 84db355 - [clang] Fix KEYALL

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2022-05-11T14:28:08-04:00
New Revision: 84db35594953a6f7aff7cbc007f1c5d2fd35b1a9

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

LOG: [clang] Fix KEYALL

Update KEYALL to cover KEYCUDA. Introduce KEYMAX and
a generic way to update KEYALL.

Reviewed by: Dan Liew

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

Added: 


Modified: 
clang/lib/Basic/IdentifierTable.cpp

Removed: 




diff  --git a/clang/lib/Basic/IdentifierTable.cpp 
b/clang/lib/Basic/IdentifierTable.cpp
index af19de44bed8..82cee4aa052d 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -109,9 +109,10 @@ namespace {
 KEYMSCOMPAT   = 0x80,
 KEYSYCL   = 0x100,
 KEYCUDA   = 0x200,
+KEYMAX= KEYCUDA, // The maximum key
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
-KEYALL = (0x1ff & ~KEYNOMS18 &
-  ~KEYNOOPENCL) // KEYNOMS18 and KEYNOOPENCL are used to exclude.
+KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYNOMS18 &
+ ~KEYNOOPENCL // KEYNOMS18 and KEYNOOPENCL are used to exclude.
   };
 
   /// How a keyword is treated in the selected standard.



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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

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

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



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:70-73
+auto ExternCBlockBegin =
+SM.getDecomposedExpansionLoc(LinkSpecDecl->getBeginLoc());
+auto ExternCBlockEnd =
+SM.getDecomposedExpansionLoc(LinkSpecDecl->getEndLoc());

whisperity wrote:
> This unconditionally assumes that an encountered LSD will be `extern "C"`. 
> However, `extern "C++"` is also possible (and supported by Clang). (And 
> things like `extern "Java"` are also possible according to the standard, 
> albeit not supported by Clang.)
O.O I did not know its a thing :D
It seems like clang only supports `C` or `C++`.



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:87-88
 const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
-PP->addPPCallbacks(
-::std::make_unique(*this, getLangOpts()));
+  PP->addPPCallbacks(::std::make_unique(
+  *this, getLangOpts(), PP->getSourceManager()));
+}

whisperity wrote:
> (: Same here. Is this state that might keep a dangling reference if multiple 
> TUs are consumed in sequence?)
How can I acquire the right `SourceManager` if not like this?
I haven't found any alternative.
What do you suggest?



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:135
 {"float.h", "cfloat"},
 {"limits.h", "climits"},
 {"locale.h", "clocale"},

whisperity wrote:
> POSIX defines a "global" include `limits.h` which gives you the definition of 
> macros like `PATH_MAX`. Will such code keep on working if the include is 
> turned into `climits`?
IDK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D125338: [HLSL] add -D option for dxc mode.

[Xiang Li 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 rG4dae38ebfba0: [HLSL] add -D option for dxc mode. (authored 
by python3kgae).

Changed prior to commit:
  https://reviews.llvm.org/D125338?vs=428698=428725#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125338

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/dxc_D.hlsl


Index: clang/test/Driver/dxc_D.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_D.hlsl
@@ -0,0 +1,13 @@
+// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s 
--check-prefix=ERROR
+
+// Make sure -D send to cc1.
+// CHECK:"-D" "TEST=2"
+
+#ifndef TEST
+#error "TEST not defined"
+#elif TEST != 2
+#error "TEST defined to wrong value"
+#endif
+
+// ERROR-NOT: error:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
- options::OPT_S, 
options::OPT_emit_llvm,
- options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {
+  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
+  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6756,6 +6756,8 @@
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
+def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
+  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM 
passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;


Index: clang/test/Driver/dxc_D.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_D.hlsl
@@ -0,0 +1,13 @@
+// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s --check-prefix=ERROR
+
+// Make sure -D send to cc1.
+// CHECK:"-D" "TEST=2"
+
+#ifndef TEST
+#error "TEST not defined"
+#elif TEST != 2
+#error "TEST defined to wrong value"
+#endif
+
+// ERROR-NOT: error:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
- options::OPT_S, options::OPT_emit_llvm,
- options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {
+  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
+  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6756,6 +6756,8 @@
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
+def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
+  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4dae38e - [HLSL] add -D option for dxc mode.

[Xiang Li via cfe-commits]

Author: Xiang Li
Date: 2022-05-11T11:26:31-07:00
New Revision: 4dae38ebfba0d8583e52c3ded8f62f5f9fa77fda

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

LOG: [HLSL] add -D option for dxc mode.

Create dxc_D as alias to option D which Define  to  (or 1 if 
 omitted).

Reviewed By: aaron.ballman

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

Added: 
clang/test/Driver/dxc_D.hlsl

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

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 05988cbbf8d7..e0eea7310fb2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6756,6 +6756,8 @@ def target_profile : DXCJoinedOrSeparate<"T">, 
MetaVarName<"">,
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
+def dxc_D : Option<["--", "/", "-"], "D", KIND_JOINED_OR_SEPARATE>,
+  Group, Flags<[DXCOption, NoXarchOption]>, Alias;
 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM 
passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a4777b55072b..a9fbdd4272c4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,9 +3471,9 @@ static void RenderOpenCLOptions(const ArgList , 
ArgStringList ,
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
- options::OPT_S, 
options::OPT_emit_llvm,
- options::OPT_disable_llvm_passes};
+  const unsigned ForwardedArguments[] = {
+  options::OPT_dxil_validator_version, options::OPT_D, options::OPT_S,
+  options::OPT_emit_llvm, options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))

diff  --git a/clang/test/Driver/dxc_D.hlsl b/clang/test/Driver/dxc_D.hlsl
new file mode 100644
index ..d32d885f11b0
--- /dev/null
+++ b/clang/test/Driver/dxc_D.hlsl
@@ -0,0 +1,13 @@
+// RUN: %clang_dxc -DTEST=2  -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -DTEST=2  -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s 
--check-prefix=ERROR
+
+// Make sure -D send to cc1.
+// CHECK:"-D" "TEST=2"
+
+#ifndef TEST
+#error "TEST not defined"
+#elif TEST != 2
+#error "TEST defined to wrong value"
+#endif
+
+// ERROR-NOT: error:



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

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/docs/ClangOffloadPackager.rst:31-32
+uint32_t Flags;
+StringMap StringData;
+MemoryBufferRef Image;
+  };

jhuber6 wrote:
> yaxunl wrote:
> > This makes the file format depend on LLVM version and potentially standard 
> > C++ library version.
> > 
> > If it is consumed by the same version of LLVM, it may be fine.
> > 
> > However, if it is intended for a generic file format to be consumed by 
> > generic offloading language runtimes, it is better to use C-like simple 
> > data layout which does not depend on LLVM or standard C++ library.
> That format just stores the data before it's serialized to a binary format. 
> The binary format is basically just a few headers and a string table. See 
> https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Object/OffloadBinary.h#L91
>  for the real format. I didn't want to explain it all in detail here.
If the file format is intended to be consumed by generic offloading language 
runtimes or development tools, better to describe its layout like 
https://clang.llvm.org/docs/ClangOffloadBundler.html

Since language runtimes or development tools may not use LLVM to load the file. 
The documentation serves as a spec for this binary format.

Especially, it is not clear where to find the target triple and GPU arch for 
each image in this documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125165

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

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

steakhal updated this revision to Diff 428724.
steakhal added a comment.

- Ignore `extern "C++"` blocks; also added a test for this.
- Also ignore `LinkageSpecDecl`s without braces.
- Clear the `IncludesToBeProcessed` list at the end of each translation unit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

Files:
  clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
  
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s modernize-deprecated-headers %t \
+// RUN:   -- -header-filter='.*' -system-headers \
+// RUN:  -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers
+
+#define EXTERN_C extern "C"
+
+extern "C++" {
+// We should still have the warnings here.
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
+}
+
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-FIXES: {{^}}#include {{$}}
+
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
+
+#include 
+// CHECK-MESSAGES: mylib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+
+namespace wrapping {
+extern "C" {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+}
+}
+
+extern "C" {
+namespace wrapped {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+}
+}
+
+namespace wrapping {
+extern "C" {
+namespace wrapped {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+}
+}
+}
+
+EXTERN_C {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
@@ -0,0 +1 @@
+#include "assert.h"
Index: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
+++ clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
@@ -15,6 +15,22 @@
 namespace tidy {
 namespace modernize {
 
+namespace detail {
+class IncludeModernizePPCallbacks;
+class ExternCRefutationVisitor;
+struct IncludeMarker {
+  std::string Replacement;
+  StringRef FileName;
+  SourceRange ReplacementRange;
+  std::pair DecomposedDiagLoc;
+};
+bool operator<(const IncludeMarker , const IncludeMarker );
+bool operator<(const IncludeMarker ,
+   const std::pair );
+bool operator<(const std::pair ,
+   const IncludeMarker );
+} // namespace detail
+
 /// This check replaces deprecated C library headers with their C++ STL
 /// alternatives.
 ///
@@ -41,6 +57,14 @@
   }
   void registerPPCallbacks(const SourceManager , Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void onEndOfTranslationUnit() override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+  friend class detail::IncludeModernizePPCallbacks;
+  friend class detail::ExternCRefutationVisitor;
+  std::vector IncludesToBeProcessed;
 };
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -7,23 +7,37 @@
 //===--===//
 
 #include "DeprecatedHeadersCheck.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 
+#include 
 #include 
 
 namespace clang {
 namespace tidy {
 namespace modernize {
+namespace detail {
+bool operator<(const 

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl updated this revision to Diff 428718.
rsundahl added a comment.

Revert ItaniumCXXABI.cpp for now (unintentional push)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125195

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp


Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2339,10 +2339,16 @@
  QualType ElementType) {
   assert(requiresArrayCookie(expr));
 
-  CharUnits SizeSize = CGF.getSizeSize();
-  CharUnits CookieSize = getArrayCookieSizeImpl(ElementType);
   unsigned AS = NewPtr.getAddressSpace();
 
+  ASTContext  = getContext();
+  CharUnits SizeSize = CGF.getSizeSize();
+
+  // The size of the cookie.
+  CharUnits CookieSize =
+  std::max(SizeSize, Ctx.getPreferredTypeAlignInChars(ElementType));
+  assert(CookieSize == getArrayCookieSizeImpl(ElementType));
+
   // Compute an offset to the cookie.
   Address CookiePtr = NewPtr;
   CharUnits CookieOffset = CookieSize - SizeSize;
@@ -2418,19 +2424,11 @@
  QualType elementType) {
   assert(requiresArrayCookie(expr));
 
-  CharUnits sizeSize = CGF.getSizeSize();
-  CharUnits cookieSize = getArrayCookieSizeImpl(elementType);
   unsigned AS = newPtr.getAddressSpace();
 
   // The cookie is always at the start of the buffer.
   Address cookie = newPtr;
 
-  // Compute an offset to the cookie.
-  CharUnits cookieOffset = cookieSize - sizeSize*2;
-  assert(cookieOffset.isZero());
-  if (!cookieOffset.isZero())
-cookie = CGF.Builder.CreateConstInBoundsByteGEP(cookie, cookieOffset);
-
   // The first element is the element size.
   cookie = CGF.Builder.CreateElementBitCast(cookie, CGF.SizeTy);
   llvm::Value *elementSize = llvm::ConstantInt::get(CGF.SizeTy,
@@ -2456,6 +2454,7 @@
 
   // Finally, compute a pointer to the actual data buffer by skipping
   // over the cookie completely.
+  CharUnits cookieSize = ARMCXXABI::getArrayCookieSizeImpl(elementType);
   return CGF.Builder.CreateConstInBoundsByteGEP(newPtr, cookieSize);
 }
 


Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2339,10 +2339,16 @@
  QualType ElementType) {
   assert(requiresArrayCookie(expr));
 
-  CharUnits SizeSize = CGF.getSizeSize();
-  CharUnits CookieSize = getArrayCookieSizeImpl(ElementType);
   unsigned AS = NewPtr.getAddressSpace();
 
+  ASTContext  = getContext();
+  CharUnits SizeSize = CGF.getSizeSize();
+
+  // The size of the cookie.
+  CharUnits CookieSize =
+  std::max(SizeSize, Ctx.getPreferredTypeAlignInChars(ElementType));
+  assert(CookieSize == getArrayCookieSizeImpl(ElementType));
+
   // Compute an offset to the cookie.
   Address CookiePtr = NewPtr;
   CharUnits CookieOffset = CookieSize - SizeSize;
@@ -2418,19 +2424,11 @@
  QualType elementType) {
   assert(requiresArrayCookie(expr));
 
-  CharUnits sizeSize = CGF.getSizeSize();
-  CharUnits cookieSize = getArrayCookieSizeImpl(elementType);
   unsigned AS = newPtr.getAddressSpace();
 
   // The cookie is always at the start of the buffer.
   Address cookie = newPtr;
 
-  // Compute an offset to the cookie.
-  CharUnits cookieOffset = cookieSize - sizeSize*2;
-  assert(cookieOffset.isZero());
-  if (!cookieOffset.isZero())
-cookie = CGF.Builder.CreateConstInBoundsByteGEP(cookie, cookieOffset);
-
   // The first element is the element size.
   cookie = CGF.Builder.CreateElementBitCast(cookie, CGF.SizeTy);
   llvm::Value *elementSize = llvm::ConstantInt::get(CGF.SizeTy,
@@ -2456,6 +2454,7 @@
 
   // Finally, compute a pointer to the actual data buffer by skipping
   // over the cookie completely.
+  CharUnits cookieSize = ARMCXXABI::getArrayCookieSizeImpl(elementType);
   return CGF.Builder.CreateConstInBoundsByteGEP(newPtr, cookieSize);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121733: Clean pathnames in FileManager.

[Paul Pluzhnikov via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:24
+  static auto *Mappings =
+  new std::array, 645>{{
+  {"algorithm", ""},

ilya-biryukov wrote:
> Don't specify sizes of arrays explicitly. This is error prone.
std::array requires size.

I could use std::vector instead, at the cost of an extra allocation.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:546
+  {"include/wordexp.h", ""},
+  {"include/x86intrin.h", ""},
+  {"include/xlocale.h", ""},

ilya-biryukov wrote:
> Why do we change the order of elements here?
> Please revert, this increases the diff and is not relevant to the actual 
> change.
Note that the elements are inserted into a map
(after commit b3a991df3cd6a; used to be a vector before).

Also note that there are duplicates, e.g.

{"bits/typesizes.h", ""},
{"bits/typesizes.h", ""},

which can't work as intended / is already broken.

Sorting helps to find these duplicates.




Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:670
+  }};
+  auto *HeaderMapping = new llvm::StringMap(Mappings->size());
+

ilya-biryukov wrote:
> This line introduces a memory leak.
> Notice how the previous version had a `static` variable.
No, it does not. This function is called only once to initialize a static 
variable: 

static const auto *SystemHeaderMap = GetHeaderMapping();
 



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:677
+  };
+  for (auto  : *Mappings) {
+Canonicalize(p.first);

ilya-biryukov wrote:
> This function is on a critical path. We don't want to pay for `Canonicalize` 
> on every call to it.
> Please create a static variable and initialize in a lambda if that's 
> absolutely necessary.
> ```
> static auto *Mapping = []{ StringMap *Mapping = new ...; /* init code*/ 
> return Mapping; }();
> ```
This function is only called once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

Re: [PATCH] D121733: Clean pathnames in FileManager.

[Paul Pluzhnikov via cfe-commits]

Please note that this patch still breaks Winx64 tests,
and that I marked it as "further changes required" / not ready for review
some time ago.

On Wed, May 11, 2022 at 10:37 AM Ilya Biryukov via Phabricator <
revi...@reviews.llvm.org> wrote:

> ilya-biryukov requested changes to this revision.
> ilya-biryukov added a comment.
> This revision now requires changes to proceed.
>
> Please allow some time for the clangd team to review this before landing.
> Changes to filenames used to cause unintended consequences for us before.
> We switched to using file UIDs since then, but we're still not sure it's
> not going to break us.
>
> Also see other comments, there are a few important issues.
>
>
>
> 
> Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:24
> +  static auto *Mappings =
> +  new std::array, 645>{{
> +  {"algorithm", ""},
> 
> Don't specify sizes of arrays explicitly. This is error prone.
>

std::array requires size.
I could use std::vector instead, at the cost of an extra memory allocation.


> Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:546
> +  {"include/wordexp.h", ""},
> +  {"include/x86intrin.h", ""},
> +  {"include/xlocale.h", ""},
> 
> Why do we change the order of elements here?
>

Note that the elements are inserted into a map
(after commit b3a991df3cd6a; used to be a vector before).

Also note that there are duplicates, e.g.

{"bits/typesizes.h", ""},
{"bits/typesizes.h", ""},

which can't work as intended / is already broken.

Sorting helps to find these duplicates.



> Please revert, this increases the diff and is not relevant to the actual
> change.
>
>
> 
> Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:670
> +  }};
> +  auto *HeaderMapping = new
> llvm::StringMap(Mappings->size());
> +
> 
> This line introduces a memory leak.
> Notice how the previous version had a `static` variable.
>

No, it does not. This function is called only once to initialize a static
variable:

static const auto *SystemHeaderMap = GetHeaderMapping();


>
>
> 
> Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:677
> +  };
> +  for (auto  : *Mappings) {
> +Canonicalize(p.first);
> 
> This function is on a critical path.


This function is only called once.



> We don't want to pay for `Canonicalize` on every call to it.
> Please create a static variable and initialize in a lambda if that's
> absolutely necessary.
> ```
> static auto *Mapping = []{ StringMap *Mapping = new ...; /* init code*/
> return Mapping; }();
> ```
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D121733/new/
>
> https://reviews.llvm.org/D121733
>
>

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

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl updated this revision to Diff 428716.
rsundahl added a comment.

Implement suggestions from reviews. (Incremental update.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125195

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  compiler-rt/lib/asan/asan_poisoning.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp

Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
@@ -2,23 +2,21 @@
 // should NOT poison the array cookie unless the compilation unit is compiled
 // with "-fsanitize-address-poison-custom-array-cookie".
 // Here we test that:
-//   1) w/o sanitizer, the array cookie location IS writable
-//   2) w/sanitizer, the array cookie location IS writeable by default
-//   3) w/sanitizer, the flag "-fno-sanitize-address-poison-custom-array-cookie"
+//   1) w/o ASan, the array cookie location IS writable
+//   2) w/ASan, the array cookie location IS writeable by default
+//   3) w/ASan, the flag "-fno-sanitize-address-poison-custom-array-cookie"
 //  is a NOP (because this is the default) and finally,
-//   4) w/sanitizer AND "-fsanitize-address-poison-custom-array-cookie", the
+//   4) w/ASan AND "-fsanitize-address-poison-custom-array-cookie", the
 //  array cookie location is NOT writable (ASAN successfully stopped it)
 //
-// RUN: %clangxx   %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT
-// RUN: %clangxx_asan  %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT
-// RUN: %clangxx_asan -fno-sanitize-address-poison-custom-array-cookie %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT
-// RUN: %clangxx_asan -fsanitize-address-poison-custom-array-cookie%s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_YES
+// RUN: %clangxx   %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan  %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -fno-sanitize-address-poison-custom-array-cookie %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -fsanitize-address-poison-custom-array-cookie%s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=COOKIE
 
-#include 
-#include 
-#include 
-#include 
 #include 
+#include 
+
 struct Foo {
   void *operator new(size_t s) { return Allocate(s); }
   void *operator new[] (size_t s) { return Allocate(s); }
@@ -32,19 +30,20 @@
 
 void *Foo::allocated;
 
-Foo *getFoo(size_t s) {
-  return new Foo[s];
+Foo *getFoo(size_t n) {
+  return new Foo[n];
 }
 
 int main() {
   Foo *foo = getFoo(10);
-
-  *reinterpret_cast(Foo::allocated) = 42;
-// SANITIZE_YES: AddressSanitizer: heap-buffer-overflow
-// SANITIZE_YES: in main {{.*}}new_array_cookie_with_new_from_class.cpp:[[@LINE-2]]
-// SANITIZE_YES: is located 0 bytes inside of {{18|26}}-byte region
+  fprintf(stderr, "foo  : %p\n", foo);
+  fprintf(stderr, "alloc: %p\n", Foo::allocated);
+  reinterpret_cast(foo)[-1] = 42;
+// COOKIE: AddressSanitizer: heap-buffer-overflow
+// COOKIE: in main {{.*}}new_array_cookie_with_new_from_class.cpp:[[@LINE-2]]
+// COOKIE: is located {{7 bytes inside of 18|15 bytes inside of 26}}-byte region
   printf("Unsurprisingly, we were able to write to the array cookie\n");
-// SANITIZE_NOT: Unsurprisingly, we were able to write to the array cookie
+// CHECK: Unsurprisingly, we were able to write to the array cookie
 
   return 0;
 }
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
@@ -3,12 +3,12 @@
 // RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1  | FileCheck %s --check-prefix=COOKIE
 // RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1  | FileCheck %s --check-prefix=NO_COOKIE
 
-#include 
-#include 
 #include 
-int dtor_counter;
+#include 
+
+int dtor_counter=0;
 struct C {
-  size_t x;
+  int x;
   ~C() {
 dtor_counter++;
 fprintf(stderr, "DTOR %d\n", dtor_counter);
@@ -17,12 +17,11 @@
 
 __attribute__((noinline)) void Delete(C *c) { delete[] c; }
 __attribute__((no_sanitize_address)) void Write42ToCookie(C *c) {
-  size_t *p = reinterpret_cast(c);
-  p[-1] = 42;
+  

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

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

steakhal added a comment.

Great content!
I've got a long list of nits though. Nothing personal :D




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1453
+const RangeSet *NegatedRange = nullptr;
+if (const UnarySymExpr *USE = dyn_cast(Sym)) {
+  if (USE->getOpcode() == UO_Minus) {





Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1454-1456
+  if (USE->getOpcode() == UO_Minus) {
+// Just get the operand when we negate a symbol that is already 
negated.
+// -(-a) == a, ~(~a) == a

The comment says that it would work with both `-` and `~`.
Why do you check against only `-`?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1457-1458
+// -(-a) == a, ~(~a) == a
+SymbolRef NegatedSym = USE->getOperand();
+NegatedRange = getConstraint(State, NegatedSym);
+  }





Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1464
+QualType T = Sym->getType();
 SymbolManager  = State->getSymbolManager();
 SymbolRef NegatedSym =

This `SymMgr` is acquired multiple times. We could hoist it and use it 
everywhere.



Comment at: clang/test/Analysis/constraint_manager_negate.c:16-20
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))

We can probably spare the unimportant stuff.



Comment at: clang/test/Analysis/constraint_manager_negate.c:23-24
+void negate_positive_range(int a) {
+  if (-a <= 0)
+return;
+  // -a: [1, INT_MAX]

Why don't you use `assert(-a > 0)` here? It should be equivalent. Same for the 
rest.



Comment at: clang/test/Analysis/constraint_manager_negate.c:27-29
+  clang_analyzer_eval(a < 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= INT_MIN); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == INT_MIN); // expected-warning{{FALSE}}

You can also query if the bound is sharp, by asking the same question but with 
a slightly different value and expect `UNKNOWN`.
```
  clang_analyzer_eval(a < 0); // expected-warning{{TRUE}}
  clang_analyzer_eval(a < -1); // expected-warning{{UNKNOWN}}
```

I also believe that `a >= INT_MIN` is `TRUE` for all `a` anyway, thus it can be 
omitted.
Checking against equality won't help much either, because we had no equality 
check before, thus it should result in `FALSE` unconditionally.



Comment at: clang/test/Analysis/constraint_manager_negate.c:46
+return;
+  clang_analyzer_eval(-a == INT_MIN); // expected-warning{{TRUE}}
+}

Let's also assert `a == INT_MIN` just for the symmetry.



Comment at: clang/test/Analysis/constraint_manager_negate.c:69-73
+void negate_symexpr(int a, int b) {
+  assert(3 <= a * b && a * b <= 5);
+  clang_analyzer_eval(-(a * b) >= -5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(-(a * b) <= -3); // expected-warning{{TRUE}}
+}

Sweet!!



Comment at: clang/test/Analysis/constraint_manager_negate.c:76
+void negate_unsigned_min(unsigned a) {
+  if (a == UINT_MIN) {
+clang_analyzer_eval(-a == UINT_MIN); // expected-warning{{TRUE}}

Well, this is the third form of expressing constraints.
1) Using asserts.
2) Use ifs but with early returns.
3) Use ifs but embed the code into the true branch.

Please, settle on one method.



Comment at: clang/test/Analysis/constraint_manager_negate.c:87-88
+  if (a == 4u) {
+clang_analyzer_eval(-a == 4u); // expected-warning{{FALSE}}
+clang_analyzer_eval(-a != 4u); // expected-warning{{TRUE}}
+  }

Add an equality comparison which results in `TRUE`.



Comment at: clang/test/Analysis/constraint_manager_negate.c:92-98
+_Static_assert(UINT_MID == -UINT_MID, "");
+void negate_unsigned_mid(unsigned a) {
+  if (a == UINT_MID) {
+clang_analyzer_eval(-a == UINT_MID); // expected-warning{{TRUE}}
+clang_analyzer_eval(-a != UINT_MID); // expected-warning{{FALSE}}
+  }
+}

Leave a comment here that this is the only value besides zero where the negated 
is equal to the original value in the `unsigned int` domain.



Comment at: clang/test/Analysis/constraint_manager_negate.c:100-105
+void negate_unsigned_mid2(unsigned a) {
+  if (a < UINT_MID && a > UINT_MIN) {
+clang_analyzer_eval(-a > UINT_MID); // expected-warning{{TRUE}}
+clang_analyzer_eval(-a < UINT_MID); // expected-warning{{FALSE}}
+  }
+}

I believe the eval query can be sharper than this.


Repository:
  rG LLVM Github 

[PATCH] D121733: Clean pathnames in FileManager.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/lib/Basic/FileManager.cpp:218
+  llvm::sys::path::remove_dots(CleanFilename, /*remove_dot_dot=*/false);
+  Filename = CleanFilename;
+

this is actually breaking the [contract of 
FileEntryRef](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/FileEntry.h#L58-L59),
 is this the point?

```
/// A reference to a \c FileEntry that includes the name of the file as it was
/// accessed by the FileManager's client.
```

I don't see mention of this contract being changed explicitly anywhere, if so 
can you mention it in the commit message and also update the documentation? 
(the same applies to DirectoryEntryRef changes as well)

I am not able to take a detailed look at this at the moment, but this doesn't 
feel like a safe change for all clients. Because people might be relying on 
this contract without explicitly testing the behaviour for "./" in the 
filenames. So tests passing (especially when you're just updating them to not 
have `./`) might not be implying it's safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

[PATCH] D125006: [pseudo] Support parsing variant target symbols.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Grammar.h:188
+// Name must be a valid nonterminal symbol name in the grammar.
+SymbolID findNonterminal(llvm::StringRef Name,
+ const clang::pseudo::GrammarTable );

hokein wrote:
> it is unfortunate that we put this function in the Grammar.h. This could be 
> avoided when we have the generated enum grammar type, but we are not there 
> yet...
This doesn't seem so bad to me, but I'd prefer it to be a method on Grammar I 
think - callers like the ones in the benchmark shouldn't have to worry about 
the GrammarTable object.



Comment at: clang-tools-extra/pseudo/tool/ClangPseudo.cpp:118
+  *ParseableStream, clang::pseudo::ParseParams{*G, LRTable, Arena, 
GSS},
+  clang::pseudo::findNonterminal(StartSymbol, G->table()));
   if (PrintForest)

this means if you pass --start-symbol=typo we'll hit an assertion

I think findNonterminal should probably return Optional instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125006

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

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

It seems to not affect a ton of code, so I don't think there's a huge hurry.

I would like to hear an answer about the question "what is this for?" though, 
and your thoughts on how this relates to the goals in 
https://docs.google.com/presentation/d/1oxNHaVjA9Gn_rTzX6HIpJHP7nXRua_0URXxxJ3oYRq0/edit#slide=id.g71ecd450e_2_812
 (see point 2 in https://reviews.llvm.org/D124613#3496824)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124613

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

[PATCH] D121733: Clean pathnames in FileManager.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

Please allow some time for the clangd team to review this before landing.
Changes to filenames used to cause unintended consequences for us before. We 
switched to using file UIDs since then, but we're still not sure it's not going 
to break us.

Also see other comments, there are a few important issues.




Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:24
+  static auto *Mappings =
+  new std::array, 645>{{
+  {"algorithm", ""},

Don't specify sizes of arrays explicitly. This is error prone.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:546
+  {"include/wordexp.h", ""},
+  {"include/x86intrin.h", ""},
+  {"include/xlocale.h", ""},

Why do we change the order of elements here?
Please revert, this increases the diff and is not relevant to the actual change.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:670
+  }};
+  auto *HeaderMapping = new llvm::StringMap(Mappings->size());
+

This line introduces a memory leak.
Notice how the previous version had a `static` variable.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:677
+  };
+  for (auto  : *Mappings) {
+Canonicalize(p.first);

This function is on a critical path. We don't want to pay for `Canonicalize` on 
every call to it.
Please create a static variable and initialize in a lambda if that's absolutely 
necessary.
```
static auto *Mapping = []{ StringMap *Mapping = new ...; /* init code*/ return 
Mapping; }();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:70-73
+auto ExternCBlockBegin =
+SM.getDecomposedExpansionLoc(LinkSpecDecl->getBeginLoc());
+auto ExternCBlockEnd =
+SM.getDecomposedExpansionLoc(LinkSpecDecl->getEndLoc());

This unconditionally assumes that an encountered LSD will be `extern "C"`. 
However, `extern "C++"` is also possible (and supported by Clang). (And things 
like `extern "Java"` are also possible according to the standard, albeit not 
supported by Clang.)



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:87-88
 const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
-PP->addPPCallbacks(
-::std::make_unique(*this, getLangOpts()));
+  PP->addPPCallbacks(::std::make_unique(
+  *this, getLangOpts(), PP->getSourceManager()));
+}

(: Same here. Is this state that might keep a dangling reference if multiple 
TUs are consumed in sequence?)



Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:92
+ast_matchers::MatchFinder *Finder) {
+  Finder->addMatcher(ast_matchers::translationUnitDecl().bind("TU"), this);
+}

(Perhaps it is worth an explicit comment here that you do this at first glance 
unconventional match because the entire check's logic is calculated **only** on 
the "preprocessor" level.)



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:135
 {"float.h", "cfloat"},
 {"limits.h", "climits"},
 {"locale.h", "clocale"},

POSIX defines a "global" include `limits.h` which gives you the definition of 
macros like `PATH_MAX`. Will such code keep on working if the include is turned 
into `climits`?



Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h:66
+  friend class detail::ExternCRefutationVisitor;
+  std::vector IncludesToBeProcessed;
 };

: You are storing some TU-specific state here (`SourceRange`) that is not 
cleared. What happens if you run `clang-tidy a.cpp b.cpp`, i.e. two TUs in the 
same process execution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D125403: [Serialization] Delta-encode consecutive SourceLocations in TypeLoc

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added a subscriber: mgorny.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Much of the size of PCH/PCM files comes from stored SourceLocations.
These are encoded using (almost) their raw value, VBR-encoded. Absolute
SourceLocations can be relatively large numbers, so this commonly takes
20-30 bits per location.

We can reduce this by exploiting redundancy: many "nearby" SourceLocations are
stored differing only slightly and can be delta-encoded.
Randam-access loading of AST nodes constrains how long these sequences
can be, but we can do it at least within a node that always gets
deserialized as an atomic unit.

TypeLoc is implemented in this patch as it's a relatively small change
that shows most of the API.
This saves ~3.5% of PCH size, I have local changes applying this technique
further that save another 3%, I think it's possible to get to 10% total.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125403

Files:
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTRecordReader.h
  clang/include/clang/Serialization/ASTRecordWriter.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/SourceLocationEncoding.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/unittests/Serialization/CMakeLists.txt
  clang/unittests/Serialization/SourceLocationEncodingTest.cpp

Index: clang/unittests/Serialization/SourceLocationEncodingTest.cpp
===
--- /dev/null
+++ clang/unittests/Serialization/SourceLocationEncodingTest.cpp
@@ -0,0 +1,103 @@
+//===- unittests/Serialization/SourceLocationEncodingTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Serialization/SourceLocationEncoding.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+using LocSeq = SourceLocationSequence;
+
+// Convert a single source location into encoded form and back.
+// If ExpectedEncoded is provided, verify the encoded value too.
+// Loc is the raw (in-memory) form of SourceLocation.
+void roundTrip(SourceLocation::UIntTy Loc,
+   llvm::Optional ExpectedEncoded = llvm::None) {
+  uint64_t ActualEncoded =
+  SourceLocationEncoding::encode(SourceLocation::getFromRawEncoding(Loc));
+  if (ExpectedEncoded)
+ASSERT_EQ(ActualEncoded, *ExpectedEncoded) << "Encoding " << Loc;
+  SourceLocation::UIntTy DecodedEncoded =
+  SourceLocationEncoding::decode(ActualEncoded).getRawEncoding();
+  ASSERT_EQ(DecodedEncoded, Loc) << "Decoding " << ActualEncoded;
+}
+
+// As above, but use sequence encoding for a series of locations.
+void roundTrip(std::vector Locs,
+   std::vector ExpectedEncoded = {}) {
+  std::vector ActualEncoded;
+  {
+LocSeq::Root Seq;
+for (auto L : Locs)
+  ActualEncoded.push_back(SourceLocationEncoding::encode(
+  SourceLocation::getFromRawEncoding(L), Seq));
+if (!ExpectedEncoded.empty())
+  ASSERT_EQ(ActualEncoded, ExpectedEncoded)
+  << "Encoding " << testing::PrintToString(Locs);
+  }
+  std::vector DecodedEncoded;
+  {
+LocSeq::Root Seq;
+for (auto L : ActualEncoded) {
+  SourceLocation Loc = SourceLocationEncoding::decode(L, Seq);
+  DecodedEncoded.push_back(Loc.getRawEncoding());
+}
+ASSERT_EQ(DecodedEncoded, Locs)
+<< "Decoding " << testing::PrintToString(ActualEncoded);
+  }
+}
+
+constexpr SourceLocation::UIntTy MacroBit =
+1 << (sizeof(SourceLocation::UIntTy) * CHAR_BIT - 1);
+constexpr SourceLocation::UIntTy Big = MacroBit >> 1;
+constexpr SourceLocation::UIntTy Biggest = -1;
+
+TEST(SourceLocationEncoding, Individual) {
+  roundTrip(1, 2);
+  roundTrip(100, 200);
+  roundTrip(MacroBit, 1);
+  roundTrip(MacroBit | 5, 11);
+  roundTrip(Big);
+  roundTrip(Big + 1);
+  roundTrip(MacroBit | Big);
+  roundTrip(MacroBit | Big + 1);
+}
+
+TEST(SourceLocationEncoding, Sequence) {
+  roundTrip({1, 2, 3, 3, 2, 1},
+{2, // 1
+ 5, // +2 (+1 of non-raw)
+ 5, // +2
+ 1, // +0
+ 4, // -2
+ 4} // -2
+  );
+  roundTrip({100, 0, 100},
+{200, // 100
+ 0,   // 0
+ 1}   // +0
+  );
+
+  roundTrip({1, Big}, {2, ((Big - 1) << 2) + 1});
+  roundTrip({2, MacroBit | Big}, {4, ((Big - 1) << 2) - 1});
+
+  roundTrip({3, MacroBit | 5, MacroBit | 4, 3},
+{6,  // 3
+ 11, // +5 (+2 of non-raw + set macro bit)
+ 4,  // -2
+   

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource added a comment.

In D124613#3506740 , @thakis wrote:

> Should we revert this, at least for now, until the investigation is complete?

I started typing my message before you sent yours so it was not a direct 
answer. We are working on a fork of LLVM, so if you revert the fix for the 
moment, it will not put us in a bad position anyway. I'll let you judge on 
whether the current problem can remain until my fixing PR is reviewed or 
whether you prefer to revert immediately and for me to make a clean PR with the 
corrected version afterward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124613

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

[PATCH] D125141: [clang][AIX] Don't ignore XCOFF visibility by default

[David Tenty via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd9c1d3cbcb97: [clang][AIX] Dont ignore XCOFF 
visibility by default (authored by daltenty).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D125141?vs=427784=428708#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125141

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.cpp
  clang/test/CodeGen/PowerPC/aix-visibility-inlines-hidden.cpp

Index: clang/test/CodeGen/PowerPC/aix-visibility-inlines-hidden.cpp
===
--- clang/test/CodeGen/PowerPC/aix-visibility-inlines-hidden.cpp
+++ clang/test/CodeGen/PowerPC/aix-visibility-inlines-hidden.cpp
@@ -1,13 +1,13 @@
 // RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -emit-llvm -o - -x c++ %s  | \
-// RUN: FileCheck -check-prefix=NOVISIBILITY-IR %s
+// RUN: FileCheck -check-prefix=VISIBILITY-IR %s
 
 // RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large \
 // RUN:-fvisibility-inlines-hidden -emit-llvm -o - -x c++ %s  | \
-// RUN: FileCheck -check-prefix=NOVISIBILITY-IR %s
+// RUN: FileCheck -check-prefixes=VISIBILITY-IR,HIDDENINLINE-IR %s
 
 // RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden \
 // RUN:-fvisibility default -emit-llvm -o - -x c++ %s  | \
-// RUN: FileCheck -check-prefix=VISIBILITY-IR %s
+// RUN: FileCheck -check-prefixes=VISIBILITY-IR,HIDDENINLINE-IR %s
 
 // RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -mignore-xcoff-visibility -emit-llvm \
 // RUN:-fvisibility-inlines-hidden -fvisibility default -o - -x c++ %s  | \
@@ -30,7 +30,7 @@
   return x;
 }
 
-// VISIBILITY-IR: define linkonce_odr hidden void @_Z1fv()
+// HIDDENINLINE-IR: define linkonce_odr hidden void @_Z1fv()
 // NOVISIBILITY-IR:   define linkonce_odr void @_Z1fv()
 
 // VISIBILITY-IR: define linkonce_odr hidden void @_Z3foov()
Index: clang/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.cpp
===
--- clang/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.cpp
+++ clang/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple powerpc-unknown-aix -emit-llvm -o - -x c++ %s  | \
-// RUN: FileCheck -check-prefix=NOVISIBILITY-IR %s
+// RUN: FileCheck -check-prefix=VISIBILITY-IR %s
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple powerpc-unknown-aix -emit-llvm -round-trip-args -o - -x c++ %s  | \
-// RUN: FileCheck -check-prefix=NOVISIBILITY-IR %s
+// RUN: FileCheck -check-prefix=VISIBILITY-IR %s
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple powerpc-unknown-aix -mignore-xcoff-visibility -fvisibility default -emit-llvm -o - -x c++ %s  | \
 // RUN: FileCheck -check-prefix=NOVISIBILITY-IR %s
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3739,28 +3739,7 @@
 Opts.GNUCVersion = Major * 100 * 100 + Minor * 100 + Patch;
   }
 
-  // In AIX OS, the -mignore-xcoff-visibility is enable by default if there is
-  // no -fvisibility=* option.
-  // This is the reason why '-fvisibility' needs to be always generated:
-  // its absence implies '-mignore-xcoff-visibility'.
-  //
-  // Suppose the original cc1 command line does contain '-fvisibility default':
-  // '-mignore-xcoff-visibility' should not be implied.
-  // * If '-fvisibility' is not generated (as most options with default values
-  //   don't), its absence would imply '-mignore-xcoff-visibility'. This changes
-  //   the command line semantics.
-  // * If '-fvisibility' is generated regardless of its presence and value,
-  //   '-mignore-xcoff-visibility' won't be implied and the command line
-  //   semantics are kept intact.
-  //
-  // When the original cc1 command line does **not** contain '-fvisibility',
-  // '-mignore-xcoff-visibility' is implied. The generated command line will
-  // contain both '-fvisibility default' and '-mignore-xcoff-visibility' and
-  // subsequent calls to `CreateFromArgs`/`generateCC1CommandLine` will always
-  // produce the same arguments.
-
-  if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility) ||
-  !Args.hasArg(OPT_fvisibility)))
+  if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility)))
 Opts.IgnoreXCOFFVisibility = 1;
 
   if (Args.hasArg(OPT_ftrapv)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ 

[clang] d9c1d3c - [clang][AIX] Don't ignore XCOFF visibility by default

[David Tenty via cfe-commits]

Author: David Tenty
Date: 2022-05-11T13:27:48-04:00
New Revision: d9c1d3cbcb9751a6a82cc5e4ada0533cbbc79a1a

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

LOG: [clang][AIX] Don't ignore XCOFF visibility by default

D87451 added -mignore-xcoff-visibility for AIX targets and made it the default 
(which mimicked the behaviour of the XL 16.1 compiler on AIX).

However, ignoring hidden visibility has unwanted side effects and some 
libraries depend on visibility to hide non-ABI facing entities from user 
headers and
reserve the right to change these implementation details based on this 
(https://libcxx.llvm.org/DesignDocs/VisibilityMacros.html). This forces us to 
use
internal linkage fallbacks for these cases on AIX and creates an unwanted 
divergence in implementations on the plaform.

For these reasons, it's preferable to not add -mignore-xcoff-visibility by 
default, which is what this patch does.

Reviewed By: DiggerLin

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/PowerPC/aix-ignore-xcoff-visibility.cpp
clang/test/CodeGen/PowerPC/aix-visibility-inlines-hidden.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5a3b2066d68fc..6d2a1b8885ce9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -298,6 +298,17 @@ Windows Support
   JustMyCode feature. Note, you may need to manually add ``/JMC`` as additional
   compile options in the Visual Studio since it currently assumes clang-cl 
does not support ``/JMC``.
 
+AIX Support
+---
+
+- The driver no longer adds ``-mignore-xcoff-visibility`` by default for AIX
+  targets when no other visibility command-line options are in effect, as
+  ignoring hidden visibility can silently have undesirable side effects (e.g
+  when libraries depend on visibility to hide non-ABI facing entities). The
+  ``-mignore-xcoff-visibility`` option can be manually specified on the
+  command-line to recover the previous behavior if desired.
+
+
 C Language Changes in Clang
 ---
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a9dd4ca65333c..05988cbbf8d7e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5922,9 +5922,7 @@ def stack_protector_buffer_size : Separate<["-"], 
"stack-protector-buffer-size">
   MarshallingInfoInt, "8">;
 def fvisibility : Separate<["-"], "fvisibility">,
   HelpText<"Default type and symbol visibility">,
-  MarshallingInfoVisibility, 
"DefaultVisibility">,
-  // Always emitting because of the relation to `-mignore-xcoff-visibility`.
-  AlwaysEmit;
+  MarshallingInfoVisibility, 
"DefaultVisibility">;
 def ftype_visibility : Separate<["-"], "ftype-visibility">,
   HelpText<"Default type visibility">,
   MarshallingInfoVisibility, 
fvisibility.KeyPath>;

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 847766a58c021..2e205f3429bd4 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3739,28 +3739,7 @@ bool CompilerInvocation::ParseLangArgs(LangOptions 
, ArgList ,
 Opts.GNUCVersion = Major * 100 * 100 + Minor * 100 + Patch;
   }
 
-  // In AIX OS, the -mignore-xcoff-visibility is enable by default if there is
-  // no -fvisibility=* option.
-  // This is the reason why '-fvisibility' needs to be always generated:
-  // its absence implies '-mignore-xcoff-visibility'.
-  //
-  // Suppose the original cc1 command line does contain '-fvisibility default':
-  // '-mignore-xcoff-visibility' should not be implied.
-  // * If '-fvisibility' is not generated (as most options with default values
-  //   don't), its absence would imply '-mignore-xcoff-visibility'. This 
changes
-  //   the command line semantics.
-  // * If '-fvisibility' is generated regardless of its presence and value,
-  //   '-mignore-xcoff-visibility' won't be implied and the command line
-  //   semantics are kept intact.
-  //
-  // When the original cc1 command line does **not** contain '-fvisibility',
-  // '-mignore-xcoff-visibility' is implied. The generated command line will
-  // contain both '-fvisibility default' and '-mignore-xcoff-visibility' and
-  // subsequent calls to `CreateFromArgs`/`generateCC1CommandLine` will always
-  // produce the same arguments.
-
-  if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility) ||
-  !Args.hasArg(OPT_fvisibility)))
+  if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility)))
 

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/docs/ClangOffloadPackager.rst:31-32
+uint32_t Flags;
+StringMap StringData;
+MemoryBufferRef Image;
+  };

yaxunl wrote:
> This makes the file format depend on LLVM version and potentially standard 
> C++ library version.
> 
> If it is consumed by the same version of LLVM, it may be fine.
> 
> However, if it is intended for a generic file format to be consumed by 
> generic offloading language runtimes, it is better to use C-like simple data 
> layout which does not depend on LLVM or standard C++ library.
That format just stores the data before it's serialized to a binary format. The 
binary format is basically just a few headers and a string table. See 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Object/OffloadBinary.h#L91
 for the real format. I didn't want to explain it all in detail here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125165

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

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/docs/ClangOffloadPackager.rst:31-32
+uint32_t Flags;
+StringMap StringData;
+MemoryBufferRef Image;
+  };

This makes the file format depend on LLVM version and potentially standard C++ 
library version.

If it is consumed by the same version of LLVM, it may be fine.

However, if it is intended for a generic file format to be consumed by generic 
offloading language runtimes, it is better to use C-like simple data layout 
which does not depend on LLVM or standard C++ library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125165

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

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource added a comment.

Thanks a lot for pointing out the reproducer, it made the debug a lot easier.
I have pinpointed the problem to an unintended increase in ADL visibility for 
friend classes. I do have a test-case and fix for that. I'll make a last check 
tomorrow to make sure I'm not missing something important and push a 
pull-request.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124613

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

> Any update/further details here?

David, apologies for not getting back to you sooner. The context is D105564 
 which I started working on again recently. I 
was having difficulties finding a solution that also worked for local lambdas. 
I discovered eventually that what I had worked with debug-info that gcc 
generated and realized that gcc was not emitting `auto` for lambdas and decided 
to match gcc's behavior here since we often do that. I think an alternative 
approach would have been to emit `DW_AT_linkage_name` for local lambdas but 
when I dug into that it looked like we were dropping the linkage name several 
step before where would emit `DW_AT_linkage_name` and it was not clear to me 
how easy a fix that was.

I am happy to consider other approaches as well to solving lookup for local 
lambdas for D105564 . Let me know what you 
think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123319

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

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Given that:

- This is not needed for system headers
- Before the change, users had to fix their standards-noncompliant code, but 
with it, users have to change their previously-compiling, standards-compliant 
code
- The change is a no-op for C++20 and up anyways
- There's no warning implemented, so the change is kind of against the spirit 
of clang-cl atm
- The change can't affect a lot of code since we didn't have if for a long time 
(and it only broke one single thing in all of chrome), so erring on less code 
in clang seems better

Should we revert this, at least for now, until the investigation is complete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124613

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

[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Namgoo Lee via Phabricator via cfe-commits]

nlee created this revision.
nlee added a reviewer: lattner.
Herald added a project: All.
nlee requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Returning class or struct by value with const qualifier should be avoided since 
it prevents move semantics.

clang-tidy has readability-const-return-type that catches this case, but it 
also impacts move semantics, not just readability.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125402

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-return-by-const-value.cpp

Index: clang/test/SemaCXX/warn-return-by-const-value.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-return-by-const-value.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fsyntax-only -Wreturn-by-const-value -Wignored-qualifiers -std=c++11 -verify %s
+
+struct S{} s;
+
+const S f1() // expected-warning {{returning by const value prevents move semantics}}
+{
+  return s;
+}
+
+// warn only on top-level const
+const S& f2()
+{
+  return s;
+}
+
+// same as f2()
+const S* f3()
+{
+  return 
+}
+
+// top-level const on primitive type(e.g. pointer) is handled by -Wignored-qualifiers
+S* const f4() // expected-warning {{'const' type qualifier on return type has no effect}}
+{
+  return 
+}
+
+// same as f4()
+const S* const f5() // expected-warning {{'const' type qualifier on return type has no effect}}
+{
+  return 
+}
\ No newline at end of file
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -37,6 +37,7 @@
 CHECK-NEXT:-Wreorder
 CHECK-NEXT:  -Wreorder-ctor
 CHECK-NEXT:  -Wreorder-init-list
+CHECK-NEXT:-Wreturn-by-const-value
 CHECK-NEXT:-Wreturn-type
 CHECK-NEXT:  -Wreturn-type-c-linkage
 CHECK-NEXT:-Wself-assign
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5197,6 +5197,15 @@
   S.Diag(DeclType.Loc, diag::warn_deprecated_volatile_return) << T;
   }
 
+  // const qualifier on by-value return type prevents move semantics
+  if (S.getLangOpts().CPlusPlus11 &&
+  T.isConstQualified() &&
+  T->isStructureOrClassType()) {
+const SourceLocation ConstLoc = D.getDeclSpec().getConstSpecLoc();
+S.Diag(ConstLoc, diag::warn_return_by_const_value)
+<< T << FixItHint::CreateRemoval(ConstLoc);
+  }
+
   // Objective-C ARC ownership qualifiers are ignored on the function
   // return type (by type canonicalization). Complain if this attribute
   // was written here.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6618,6 +6618,10 @@
   InGroup, DefaultIgnore;
 def note_remove_move : Note<"remove std::move call here">;
 
+def warn_return_by_const_value : Warning<
+  "returning by const value prevents move semantics">,
+  InGroup, DefaultIgnore;
+
 def warn_string_plus_int : Warning<
   "adding %0 to a string does not append to the string">,
   InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -566,6 +566,7 @@
 def GNURedeclaredEnum : DiagGroup<"gnu-redeclared-enum">;
 def RedundantMove : DiagGroup<"redundant-move">;
 def Register : DiagGroup<"register", [DeprecatedRegister]>;
+def ReturnByConstValue : DiagGroup<"return-by-const-value">;
 def ReturnTypeCLinkage : DiagGroup<"return-type-c-linkage">;
 def ReturnType : DiagGroup<"return-type", [ReturnTypeCLinkage]>;
 def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy",
@@ -987,6 +988,7 @@
 MultiChar,
 RangeLoopConstruct,
 Reorder,
+ReturnByConstValue,
 ReturnType,
 SelfAssignment,
 SelfMove,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

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

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

Overall looks good to me. I wish some parts would be simpler but it looks like 
sometimes it is not easy to extend the current code and we might need to do 
some refactoring at some point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

[Kees Cook via Phabricator via cfe-commits]

kees added a comment.

In D125142#3505767 , @jfb wrote:

> I think the most relevant post from @rsmith is: 
> https://discourse.llvm.org/t/making-ftrivial-auto-var-init-zero-a-first-class-option/55143/40
>
> He has a prototype: https://reviews.llvm.org/D79249
> I assume he would like someone to pursue it further, it was a good faith 
> attempt at not just demanding. I'd played with it and it needed a few fixes, 
> but overall it was pretty complete. Does someone want to give it a go?

That's a different mode and doesn't seem to be relevant to the current 
situation and this change to drop the -enable flag.

> The fact remains that people have deployed zero init widely (you're likely 
> running multiple zero-init codebases to read this), and they would not ever 
> deploy zero-or-trap init. That's simply a fact.

Right.

> Separately, we'd discussed narrowing the performance gap between pattern and 
> zero-init, @Florian and team had done a bunch of work 2+ years ago, but I 
> don't know how "done" we can claim that work is since I haven't been tracking 
> the work.

Performance is the smaller part of why init=zero is being used so widely. It's 
about stability. Quoting from my earlier thread 
:

> Another driving factor (see below from various vendors/projects), is the
> security stance. Putting non-zero values into most variables types ends
> up making them arguably more dangerous than if they were zero-filled.
> Most notably, sizes and indexes and less likely to be used out of bounds
> if they are zero-initialized. The same holds for bool values that tend
> to indicate success instead of failing safe with a false value. While
> pointers in the non-canonical range are nice, zero tends to be just
> as good. There are certainly exceptions here, but the bulk of the
> historical record on how "uninitialized" variables have been used in
> real world exploitation involve their being non-zero, and analysis of
> those bugs support that conclusion.

This "usually handled safely" state (e.g. existing NULL pointer checks) is 
specifically why Chrome OS switched from init=pattern to init=zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125142

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

[PATCH] D125401: [OpenCL] Do not guard vload/store_half builtins

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
svenvh added a project: clang.
Herald added subscribers: Naghasan, ldrumm, yaxunl.
Herald added a project: All.
svenvh requested review of this revision.
Herald added a subscriber: cfe-commits.

The vload*_half* and vstore*_half* builtins do not require the
cl_khr_fp16 extension: pointers to `half` can be declared without the
extension and the _half variants of vload and vstore should be
available without the extension.

This aligns the guards for these builtins for
`-fdeclare-opencl-builtins` with `opencl-c.h`.

Fixes https://github.com/llvm/llvm-project/issues/55275


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125401

Files:
  clang/lib/Headers/opencl-c-base.h
  clang/lib/Sema/OpenCLBuiltins.td


Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -352,9 +352,22 @@
 let Extension = Fp64TypeExt in {
   def Double: Type<"double",QualType<"Context.DoubleTy">>;
 }
+
+// The half type for builtins that require the cl_khr_fp16 extension.
 let Extension = Fp16TypeExt in {
   def Half  : Type<"half",  QualType<"Context.HalfTy">>;
 }
+
+// Without the cl_khr_fp16 extension, the half type can only be used to declare
+// a pointer.  Define const and non-const pointer types in all address spaces.
+// Use the "__half" alias to allow the TableGen emitter to distinguish the
+// (extensionless) pointee type of these pointer-to-half types from the "half"
+// type defined above that already carries the cl_khr_fp16 extension.
+foreach AS = [PrivateAS, GlobalAS, ConstantAS, LocalAS, GenericAS] in {
+  def "HalfPtr" # AS  : PointerType>, AS>;
+  def "HalfPtrConst" # AS : PointerType>>, AS>;
+}
+
 def Size  : Type<"size_t",QualType<"Context.getSizeType()">>;
 def PtrDiff   : Type<"ptrdiff_t", QualType<"Context.getPointerDiffType()">>;
 def IntPtr: Type<"intptr_t",  QualType<"Context.getIntPtrType()">>;
@@ -877,22 +890,22 @@
 
 multiclass VloadVstoreHalf addrspaces, bit defStores> {
   foreach AS = addrspaces in {
-def : Builtin<"vload_half", [Float, Size, PointerType, 
AS>], Attr.Pure>;
+def : Builtin<"vload_half", [Float, Size, !cast("HalfPtrConst" # 
AS)], Attr.Pure>;
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload_half" # VSize, "vloada_half" # VSize] in {
-def : Builtin, Size, 
PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, 
!cast("HalfPtrConst" # AS)], Attr.Pure>;
   }
 }
 if defStores then {
   foreach rnd = ["", "_rte", "_rtz", "_rtp", "_rtn"] in {
 foreach name = ["vstore_half" # rnd] in {
-  def : Builtin]>;
-  def : Builtin]>;
+  def : Builtin("HalfPtr" # 
AS)]>;
+  def : Builtin("HalfPtr" # 
AS)]>;
 }
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vstore_half" # VSize # rnd, "vstorea_half" # VSize 
# rnd] in {
-def : Builtin, Size, 
PointerType]>;
-def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
!cast("HalfPtr" # AS)]>;
+def : Builtin, Size, 
!cast("HalfPtr" # AS)]>;
   }
 }
   }
Index: clang/lib/Headers/opencl-c-base.h
===
--- clang/lib/Headers/opencl-c-base.h
+++ clang/lib/Headers/opencl-c-base.h
@@ -202,6 +202,9 @@
 typedef double double16 __attribute__((ext_vector_type(16)));
 #endif
 
+// An internal alias for half, for use by OpenCLBuiltins.td.
+#define __half half
+
 #if defined(__OPENCL_CPP_VERSION__)
 #define NULL nullptr
 #elif defined(__OPENCL_C_VERSION__)


Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -352,9 +352,22 @@
 let Extension = Fp64TypeExt in {
   def Double: Type<"double",QualType<"Context.DoubleTy">>;
 }
+
+// The half type for builtins that require the cl_khr_fp16 extension.
 let Extension = Fp16TypeExt in {
   def Half  : Type<"half",  QualType<"Context.HalfTy">>;
 }
+
+// Without the cl_khr_fp16 extension, the half type can only be used to declare
+// a pointer.  Define const and non-const pointer types in all address spaces.
+// Use the "__half" alias to allow the TableGen emitter to distinguish the
+// (extensionless) pointee type of these pointer-to-half types from the "half"
+// type defined above that already carries the cl_khr_fp16 extension.
+foreach AS = [PrivateAS, GlobalAS, ConstantAS, LocalAS, GenericAS] in {
+  def "HalfPtr" # AS  : PointerType>, AS>;
+  def "HalfPtrConst" # AS : PointerType>>, AS>;
+}
+
 def Size  : Type<"size_t",QualType<"Context.getSizeType()">>;
 def PtrDiff   : Type<"ptrdiff_t", QualType<"Context.getPointerDiffType()">>;
 def IntPtr: Type<"intptr_t",  

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a super minor nit in a test file. Can you please add a release 
note for the new diagnostics as well?

I'm happy to land this for you when you're ready, but let me know which email 
address you'd like me to use this time around.




Comment at: clang/test/Preprocessor/ifdef-recover.c:23-29
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}

Just to keep comment styles the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D124447#3506536 , @whisperity 
wrote:

> In D124447#3493996 , @whisperity 
> wrote:
>
>> In D124447#3493446 , 
>> @aaron.ballman wrote:
>>
>>> precommit CI is showing a fair amount of failures that I believe are 
>>> related to your patch.
>>
>> I have no idea what is causing the CI issues, because all the CI issues are 
>> related to unit test libraries removing `const` from fixits and such(??) 
>> which this patch (or any in the current patch set) doesn't even touch, at 
>> all. I did run the test targets locally... it's likely that simply the 
>> rebase and the push happened against an unclean/breaking main branch...
>
> Rebase & rerun against a current `main` branch produces the issues locally 
> for me, too. So far I have no idea what might be causing this, but I'll keep 
> on digging. However, let's continue with discussing the general approach 
> meanwhile. 

Actually, it turned out to be easy after all! The implementation **is** 
backwards compatible //on the command-line// but was not for the 
"machine-driven" tests that are implemented as C++ code and execution scaffold. 
Adding the below commented change to the diff **completely** solved the failing 
tests for me locally (turns out none of the checks were actually running the 
`check()` callback!), but I cannot update a //"Revision"// that I do not own.




Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:60
+
+  MultipassProjectPhase MultipassPhase;
+  /// The directory where multi-pass project-level analysis stores its data to.

⚠ The crucial fix against the unit-test failures!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124447

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

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

xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116
 
+  const bool Foreign = false; // From CTU.
+

martong wrote:
> martong wrote:
> > xazax.hun wrote:
> > > I feel that we use different terms for the imported declarations. 
> > > Sometimes we call them `new`, sometimes `imported`, sometimes `foreign`. 
> > > In case all of these means the same thing, it would be nice to 
> > > standardize on a single way of naming. If there is a subtle difference 
> > > between them, let's document that in a comment. It would be nice if we 
> > > did not need the comment after the declaration but it would be obvious 
> > > from the variable name.
> > Yes, I agree that this should deserver some more explanation. Maybe right 
> > above this declaration?
> > 
> > So, `new` means that a declaration is **created** newly by the ASTImporter.
> > `imported` means it has been imported, but not necessarily `new`. Think 
> > about this case, we import `foo`'s definition.
> > ```
> > // to.cpp
> > void bar() {} // from a.h
> > // from.cpp
> > void bar() {} // from a.h
> > void foo() {
> >   bar();
> > }
> > ```
> > Then `foo` will be `new` and `imported`, `bar` will be `imported` and not 
> > `new`.  
> > `foreign` basically means `imported` and `new`.
> I've just added an explanatory comment for this field.
Foreign means new and imported. But is there a way for a declaration to be new 
and not to be imported? If no, in that case it feels like new and foreign are 
actually the same and we should standardize on a single name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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

[PATCH] D125338: [HLSL] add -D option for dxc mode.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a super minor nit with the test.




Comment at: clang/test/Driver/dxc_D.hlsl:14
+// ERROR-NOT: error:
\ No newline at end of file


You should add the newline back to the end of the test file. (Feel free to do 
that when you land the patch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125338

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446
+}
+const bool BState = State->contains(D);
+if (!BState) { // This is the first time we see this foreign function.

xazax.hun wrote:
> martong wrote:
> > xazax.hun wrote:
> > > So if we see the same foreign function called in multiple contexts, we 
> > > will only queue one of the contexts for the CTU. Is this the intended 
> > > design? 
> > > 
> > > So if I see:
> > > ```
> > > foreign(true);
> > > foreign(false);
> > > ```
> > > 
> > > The new CTU will only evaluate `foreign(true)` but not `foreign(false)`. 
> > This is intentional.
> > ```
> > foreign(true);
> > foreign(false);
> > ```
> > The new CTU will evaluate the following paths in the exploded graph:
> > ```
> > foreign(true); // conservative evaluated
> > foreign(false); // conservative evaluated
> > foreign(true); // inlined
> > foreign(false); // inlined
> > ```
> > The point is to keep bifurcation to a minimum and avoid the exponential 
> > blow up.
> > So, we will never have a path like this:
> > ```
> > foreign(true); // conservative evaluated
> > foreign(false); // inlined
> > ```
> > 
> > Actually, this is the same strategy that we use during the dynamic dispatch 
> > of C++ virtual calls. See `DynamicDispatchBifurcationMap`.
> > 
> > The conservative evaluation happens in the first phase, the inlining in the 
> > second phase (assuming the phase1 inlining option is set to none).
> > The new CTU will evaluate the following paths in the exploded graph:
> > ```
> > foreign(true); // conservative evaluated
> > foreign(false); // conservative evaluated
> > foreign(true); // inlined
> > foreign(false); // inlined
> > ```
> 
> When we encounter `foreign(true)`, we would add the decl to 
> `CTUDispatchBifurcationSet`. So the second time we see a call to the function 
> `foreign(false);`, we will just do the conservative evaluation and will not 
> add the call to the CTU worklist. So how will `foreign(false);` be inlined in 
> the second pass? Do I miss something? 
> 
Oh, I think I now understand. Do you expect `foreign(false);` to be inlined 
after we return from `foreign(true);` in the second pass? Sorry for the 
confusion, in that case it looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446
+}
+const bool BState = State->contains(D);
+if (!BState) { // This is the first time we see this foreign function.

martong wrote:
> xazax.hun wrote:
> > So if we see the same foreign function called in multiple contexts, we will 
> > only queue one of the contexts for the CTU. Is this the intended design? 
> > 
> > So if I see:
> > ```
> > foreign(true);
> > foreign(false);
> > ```
> > 
> > The new CTU will only evaluate `foreign(true)` but not `foreign(false)`. 
> This is intentional.
> ```
> foreign(true);
> foreign(false);
> ```
> The new CTU will evaluate the following paths in the exploded graph:
> ```
> foreign(true); // conservative evaluated
> foreign(false); // conservative evaluated
> foreign(true); // inlined
> foreign(false); // inlined
> ```
> The point is to keep bifurcation to a minimum and avoid the exponential blow 
> up.
> So, we will never have a path like this:
> ```
> foreign(true); // conservative evaluated
> foreign(false); // inlined
> ```
> 
> Actually, this is the same strategy that we use during the dynamic dispatch 
> of C++ virtual calls. See `DynamicDispatchBifurcationMap`.
> 
> The conservative evaluation happens in the first phase, the inlining in the 
> second phase (assuming the phase1 inlining option is set to none).
> The new CTU will evaluate the following paths in the exploded graph:
> ```
> foreign(true); // conservative evaluated
> foreign(false); // conservative evaluated
> foreign(true); // inlined
> foreign(false); // inlined
> ```

When we encounter `foreign(true)`, we would add the decl to 
`CTUDispatchBifurcationSet`. So the second time we see a call to the function 
`foreign(false);`, we will just do the conservative evaluation and will not add 
the call to the CTU worklist. So how will `foreign(false);` be inlined in the 
second pass? Do I miss something? 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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