[PATCH] D106518: [RISCV] Disable EEW=64 for index values when XLEN=32.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106518

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

[PATCH] D115924: [ConstantFolding] Unify handling of load from uniform value

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

In the meantime, I've applied 
https://github.com/llvm/llvm-project/commit/2926d6d335aca7e3f57ac45e6b25b1716e053fb3
 as a targeted fix for the original assertion failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115924

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

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Just a ping. I'd like to have this patch stack loaded somewhen :)


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

https://reviews.llvm.org/D105340

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

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

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

linjamaki added a comment.

Thanks, @yaxunl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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

[clang] 4f103e9 - [NFC] [C++20] [Modules] Add tests for template instantiation in transitively imported module

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2021-12-21T17:37:40+08:00
New Revision: 4f103e956157515dd800951f73ed550b1a0477f4

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

LOG: [NFC] [C++20] [Modules] Add tests for template instantiation in 
transitively imported module

This commit adds two test about template class instantiation in
transitively imported module. They are used as pre-commit tests for
successive patches.

Added: 
clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
clang/test/Modules/module-transtive-instantiation-2.cpp
clang/test/Modules/module-transtive-instantiation.cpp

Modified: 


Removed: 




diff  --git 
a/clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm 
b/clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
new file mode 100644
index 0..ee41b5374613f
--- /dev/null
+++ b/clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
@@ -0,0 +1,8 @@
+export module Templ;
+export template 
+class G {
+public:
+T operator()() {
+return T();
+}
+};

diff  --git a/clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm 
b/clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
new file mode 100644
index 0..63cd595a642dc
--- /dev/null
+++ b/clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
@@ -0,0 +1,6 @@
+export module bar;
+import Templ;
+export template
+int bar() {
+return G()();
+}

diff  --git a/clang/test/Modules/module-transtive-instantiation-2.cpp 
b/clang/test/Modules/module-transtive-instantiation-2.cpp
new file mode 100644
index 0..647971776aa13
--- /dev/null
+++ b/clang/test/Modules/module-transtive-instantiation-2.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm 
--precompile -o %t/Templ.pcm
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  
--precompile -fprebuilt-module-path=%t -o %t/bar.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import bar;
+int foo() {
+G g;// expected-error {{declaration of 'G' must be imported from 
module 'Templ' before it is required}}
+return g();  // 
expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{declaration 
here is not visible}}
+}

diff  --git a/clang/test/Modules/module-transtive-instantiation.cpp 
b/clang/test/Modules/module-transtive-instantiation.cpp
new file mode 100644
index 0..4c095bfbbce6a
--- /dev/null
+++ b/clang/test/Modules/module-transtive-instantiation.cpp
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm 
--precompile -o %t/Templ.pcm
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  
--precompile -fprebuilt-module-path=%t -o %t/bar.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import bar;
+int foo() {
+// FIXME: It shouldn't be an error. Since the `G` is already imported in 
bar.
+return bar(); // 
expected-error@Inputs/module-transtive-instantiation/bar.cppm:5 {{definition of 
'G' must be imported from module 'Templ' before it is required}}
+   // expected-note@-1 {{in instantiation of function 
template specialization 'bar' requested here}}
+   // 
expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{definition 
here is not reachable}}
+}



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

[PATCH] D115510: [clang][OpenMP][DebugInfo] Debug support for variables in shared clause of OpenMP task construct

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro accepted this revision.
djtodoro added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:-4446
+// Generate llvm.dbg.declare for each shared variable and
+// DIExpression = metadata !DIExpression(DW_OP_plus_uconst, OFFSET,
+// DW_OP_deref)

nit: We could improve this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115510

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

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D115967

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

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

[Peter Stys via Phabricator via cfe-commits]

peterstys added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1955-1957
+if (Style.isCSharp() && FormatTok->is(TT_FatArrow) &&
+tryToParseChildBlock())
+  continue;

owenpan wrote:
> peterstys wrote:
> > It seems that this block could be removed altogether, because this is 
> > already done on lines 1964-165, and it's done regardless if it the language 
> > is C# or not.
> Lines 1964-1965 only apply to JavaScript because of line 1958, so lines 
> 1955-1957 cannot be removed.
Oh yes, sorry I missed the Javascript block. I wonder if in that case wouldn't 
be more consistent to add Javascript check on the line 1955 to be consistent 
with how the same block of logic is handled on the lines 1641-1645.


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

https://reviews.llvm.org/D115967

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

[clang] a6f56a6 - Revert "[NFC] [C++20] [Modules] Add tests for template instantiation in transitively imported module"

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2021-12-21T18:34:27+08:00
New Revision: a6f56a622d96de24f183d7eed5a4fa9925c49992

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

LOG: Revert "[NFC] [C++20] [Modules] Add tests for template instantiation in 
transitively imported module"

This reverts commit 4f103e956157515dd800951f73ed550b1a0477f4.

The tests couldn't pass under windows.

Added: 


Modified: 


Removed: 
clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
clang/test/Modules/module-transtive-instantiation-2.cpp
clang/test/Modules/module-transtive-instantiation.cpp



diff  --git 
a/clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm 
b/clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
deleted file mode 100644
index ee41b5374613f..0
--- a/clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
+++ /dev/null
@@ -1,8 +0,0 @@
-export module Templ;
-export template 
-class G {
-public:
-T operator()() {
-return T();
-}
-};

diff  --git a/clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm 
b/clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
deleted file mode 100644
index 63cd595a642dc..0
--- a/clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
+++ /dev/null
@@ -1,6 +0,0 @@
-export module bar;
-import Templ;
-export template
-int bar() {
-return G()();
-}

diff  --git a/clang/test/Modules/module-transtive-instantiation-2.cpp 
b/clang/test/Modules/module-transtive-instantiation-2.cpp
deleted file mode 100644
index 647971776aa13..0
--- a/clang/test/Modules/module-transtive-instantiation-2.cpp
+++ /dev/null
@@ -1,11 +0,0 @@
-// RUN: rm -rf %t
-// RUN: mkdir -p %t
-// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm 
--precompile -o %t/Templ.pcm
-// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  
--precompile -fprebuilt-module-path=%t -o %t/bar.pcm
-// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
-
-import bar;
-int foo() {
-G g;// expected-error {{declaration of 'G' must be imported from 
module 'Templ' before it is required}}
-return g();  // 
expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{declaration 
here is not visible}}
-}

diff  --git a/clang/test/Modules/module-transtive-instantiation.cpp 
b/clang/test/Modules/module-transtive-instantiation.cpp
deleted file mode 100644
index 4c095bfbbce6a..0
--- a/clang/test/Modules/module-transtive-instantiation.cpp
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: rm -rf %t
-// RUN: mkdir -p %t
-// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm 
--precompile -o %t/Templ.pcm
-// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  
--precompile -fprebuilt-module-path=%t -o %t/bar.pcm
-// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
-
-import bar;
-int foo() {
-// FIXME: It shouldn't be an error. Since the `G` is already imported in 
bar.
-return bar(); // 
expected-error@Inputs/module-transtive-instantiation/bar.cppm:5 {{definition of 
'G' must be imported from module 'Templ' before it is required}}
-   // expected-note@-1 {{in instantiation of function 
template specialization 'bar' requested here}}
-   // 
expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{definition 
here is not reachable}}
-}



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

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1955-1957
+if (Style.isCSharp() && FormatTok->is(TT_FatArrow) &&
+tryToParseChildBlock())
+  continue;

peterstys wrote:
> owenpan wrote:
> > peterstys wrote:
> > > It seems that this block could be removed altogether, because this is 
> > > already done on lines 1964-165, and it's done regardless if it the 
> > > language is C# or not.
> > Lines 1964-1965 only apply to JavaScript because of line 1958, so lines 
> > 1955-1957 cannot be removed.
> Oh yes, sorry I missed the Javascript block. I wonder if in that case 
> wouldn't be more consistent to add Javascript check on the line 1955 to be 
> consistent with how the same block of logic is handled on the lines 1641-1645.
I don't think so because here we must parse the optional `async function` part 
first for JavaScript only.


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

https://reviews.llvm.org/D115967

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

[clang] 55d7a12 - [CodeGen] Avoid pointee type access during global var declaration

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-21T11:48:37+01:00
New Revision: 55d7a12b86d81ad3b19f790893f8bf38c817fd61

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

LOG: [CodeGen] Avoid pointee type access during global var declaration

All callers pass in a GlobalVariable, so we can conveniently fetch
the type from there.

Added: 


Modified: 
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 4a8e93e0f4b77..3579761f14293 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -172,7 +172,7 @@ void CodeGenFunction::EmitInvariantStart(llvm::Constant 
*Addr, CharUnits Size) {
 }
 
 void CodeGenFunction::EmitCXXGlobalVarDeclInit(const VarDecl &D,
-   llvm::Constant *DeclPtr,
+   llvm::GlobalVariable *GV,
bool PerformInit) {
 
   const Expr *Init = D.getInit();
@@ -194,15 +194,16 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const 
VarDecl &D,
   // "shared" address space qualifier, but the constructor of StructWithCtor
   // expects "this" in the "generic" address space.
   unsigned ExpectedAddrSpace = getContext().getTargetAddressSpace(T);
-  unsigned ActualAddrSpace = DeclPtr->getType()->getPointerAddressSpace();
+  unsigned ActualAddrSpace = GV->getAddressSpace();
+  llvm::Constant *DeclPtr = GV;
   if (ActualAddrSpace != ExpectedAddrSpace) {
-llvm::Type *LTy = CGM.getTypes().ConvertTypeForMem(T);
-llvm::PointerType *PTy = llvm::PointerType::get(LTy, ExpectedAddrSpace);
+llvm::PointerType *PTy = llvm::PointerType::getWithSamePointeeType(
+GV->getType(), ExpectedAddrSpace);
 DeclPtr = llvm::ConstantExpr::getAddrSpaceCast(DeclPtr, PTy);
   }
 
-  ConstantAddress DeclAddr(DeclPtr, 
DeclPtr->getType()->getPointerElementType(),
-   getContext().getDeclAlign(&D));
+  ConstantAddress DeclAddr(
+  DeclPtr, GV->getValueType(), getContext().getDeclAlign(&D));
 
   if (!T->isReferenceType()) {
 if (getLangOpts().OpenMP && !getLangOpts().OpenMPSimd &&

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h 
b/clang/lib/CodeGen/CodeGenFunction.h
index 679c720f7bd73..481ec2d606b11 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4410,7 +4410,7 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   /// EmitCXXGlobalVarDeclInit - Create the initializer for a C++
   /// variable with global storage.
-  void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::Constant *DeclPtr,
+  void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::GlobalVariable *GV,
 bool PerformInit);
 
   llvm::Function *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee 
Dtor,



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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:52
+// Terminal IDs correspond to the clang TokenKind enum.
+using SymbolID = uint16_t;
+// SymbolID is only 12 bits wide.

If we want strong types, we could use `enum class SymbolID : uint16_t {};`



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:74
+using RuleID = uint16_t;
+// RuleID is only 12 bits wide.
+static constexpr unsigned RuleBits = 12;

Maybe rather "there are at most 2^12 rules"



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:83
+
+  static constexpr unsigned SizeBits = 4;
+  static_assert(SizeBits + SymbolBits <= 16,

// Sequence can be at most 2^4 tokens long



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:110
+// Grammar that describes a programming language, C++ etc.
+// It parses BNF grammar rules, and is a building brick for constructing a
+// grammar-based parser.

nit: building block is more idiomatic



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:110
+// Grammar that describes a programming language, C++ etc.
+// It parses BNF grammar rules, and is a building brick for constructing a
+// grammar-based parser.

sammccall wrote:
> nit: building block is more idiomatic
Nit: it's rather the RuleSpec factories and Grammar::build (which are static) 
that parses BNF grammar.
The Grammar object itself doesn't have this responsibility.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:133
+static llvm::Expected parseRaw(llvm::StringRef Line);
+// Exposed for testing.
+// Inline all _opt symbols.

I think this should be explicit rather than implicit, and so not just "exposed 
for testing" but explicitly called



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:144
+
+  // Lookup non-terminal by name.
+  SymbolID lookupSymbol(llvm::StringRef Name) const;

The implementation also (inefficiently) supports looking up terminals. Intended?



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:154
+  // Dump symbol (terminal or non-terminal) name.
+  llvm::StringRef dumpSymbol(SymbolID) const;
+  std::string dumpRule(RuleID) const;

dump suggests an arbitrary debugging representation, but we rely on this being 
exactly the symbol name. `symbolName(SymbolID)`?

Maybe also doc what the names are (for terminals).
e.g. `Terminals have names like "," (kw_comma) or "OPERATOR" (kw_operator)`.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:156
+  std::string dumpRule(RuleID) const;
+  std::string dumpRuleRecursively(RuleID) const;
+  // Dump all rules of the given nonterminal symbol.

in the prototype the "recursively" versions ended up being much less useful 
than I thought. You may want to drop the complexity



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:171
+
+// Underlying storage of the Grammar.
+struct Table {

Are compiled transition tables etc expected to live here, or is this a purely 
descriptive version of teh grammar?



Comment at: clang/lib/Tooling/Syntax/Pseudo/Grammar.cpp:48
+
+void computeNullability(Table &T) {
+  // A nonterminal is nullable if some rule producing it is nullable.

I don't think it makes sense to compute and store nullability if our plan is to 
build a parser that doesn't support nullable symbols.

Rather we would warn in eliminateOptional() or diagnose() if there are nullable 
rules. (This doesn't require computing the nullability of every rule)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Steffen Larsen via Phabricator via cfe-commits]

steffenlarsen added a comment.

I agree that reusing existing parser logic for this would be preferable, but as 
@aaron.ballman mentions `Parser::ParseSimpleExpressionList` does not give us 
the parameter pack expansion parsing we need. We could potentially unify it 
with the logic from 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseExpr.cpp#L3059
 but on the other hand there could be potential benefits to using 
`Parser::ParseExpressionList` instead.

The primary difference between the behavior of `Parser::ParseExpressionList` 
and what we are looking for is that it will allow initializer lists as 
arguments. Since most (if not all) attributes check the validity of their 
expression arguments, they would likely fail accordingly if they get an 
initializer list, likely specifying they expected an argument of type X. It may 
be a blessing in disguise to parse these arguments as well as we are more 
permissive w.r.t. attribute arguments. Please let me know if there is anything 
fundamentally wrong with this line of logic. One concern I have is that we 
currently run `Actions.CorrectDelayedTyposInExpr` on the expression right after 
parsing it, then we run pack expansion, whereas `Parser::ParseExpressionList` 
only runs the former action in failure case. I am unsure whether or not that 
might pose a problem.

One problem is that if we only use `Parser::ParseExpressionList` to parse the 
arguments of the last argument (if it is variadic) then we get the odd behavior 
that the initializer lists will only be permitted in variadic arguments, which 
arguably is a little obscure. However, upon further investigation the only case 
that stops us from using this for all expression arguments are when the 
attribute has either `VariadicIdentifierArgument` or 
`VariadicParamOrParamIdxArgument` as the first argument. If we assume that 
these variadic expressions are limited to the last argument as well (as they 
are variadic) leaving them as the only argument of the attributes we are 
parsing here, then we can parse expressions as we did before if we are parsing 
one of these arguments or switch to using e.g. `Parser::ParseExpressionList` 
for all expressions of the attribute (even non-variadic) if not. Then we would 
just have to iterate over the produced expressions to make sure no parameter 
pack expansions occur before the last (variadic) argument of the parsed 
attribute.

//Summary of suggested changes://
Split attribute parsing into 3: First case parsing a type (currently exists). 
Second case keeping the current logic for parsing identifiers and single 
assignment-expressions if the attribute has a single argument of type 
`VariadicIdentifierArgument` or `VariadicParamOrParamIdxArgument`. Third case, 
applicable if none of the former ran, parses all expression parameters in one 
using `Parser::ParseExpressionList` and then checks that parameter packs only 
occur in the last variadic argument (if it allows it). Side effect is it allows 
initializer lists in the third case, but attributes are likely to complain 
about unexpected expression types if these are passed.


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

https://reviews.llvm.org/D114439

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

[PATCH] D115856: [syntax][pseudo] Introduce the spec C++ grammar.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:12
+#  - attributes are omitted, which will be handled as comments;
+#  - we don't allow nullable non-terminal symbols (except translation-unit).
+#There are few nullable non-terminals in the spec grammar, they are adjust

translation-unit appears to be non-nullable in the grammar here



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:13
+#  - we don't allow nullable non-terminal symbols (except translation-unit).
+#There are few nullable non-terminals in the spec grammar, they are adjust
+#to be non-nullable;

nit: adjusted



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:27
+# FIXME:
+#   - support annotations (lazy parsing, contextual identifiers)
+#   - empty input should be parsed successfully (special-case it?)

I think we should spell out what this is, or leave out the FIXME



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:28
+#   - support annotations (lazy parsing, contextual identifiers)
+#   - empty input should be parsed successfully (special-case it?)
+#

This probably isn't a FIXME for the grammar right? If we're going with 
non-nullable rules, having the *parser* special-case _ as nullable seems more 
sensible.



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:30
+#
+# start symbols
+_ := translation-unit

Is this "default start symbol"?
I believe we conceptually want to be able to start parsing at other symbols too.



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:33
+
+# A.1 gram.key
+typedef-name := IDENTIFIER

I'd suggest dropping the A.1 part, as it's version-specific

(e.g. gram.basic is currently A.4 at https://eel.is/c++draft/gram, which is a 
pretty common reference)



Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:44
+
+# A.3 grammar.basic
+# !Custom modifications to eliminate optional declaration-seq

nit: gram.basic



Comment at: clang/tools/clang-pseudo/ClangPseudo.cpp:1
+//===-- ClangPseudo.cpp - Clang pseudo parser tool 
===//
+//

Can we use this tool to drive lit tests?



Comment at: clang/tools/clang-pseudo/ClangPseudo.cpp:41
+  auto Diagnostics = Grammar::build(*RSpecs).diagnose();
+  llvm::errs() << (Diagnostics.empty() ? "Parse grammar successfully!\n"
+   : llvm::join(Diagnostics, "\n"));

nit: parse -> parsed



Comment at: clang/tools/clang-pseudo/ClangPseudo.cpp:42
+  llvm::errs() << (Diagnostics.empty() ? "Parse grammar successfully!\n"
+   : llvm::join(Diagnostics, "\n"));
+  return 0;

do you want to dump the grammar? (Behind a dump-grammar subcommand)



Comment at: clang/tools/clang-pseudo/ClangPseudo.cpp:43
+   : llvm::join(Diagnostics, "\n"));
+  return 0;
+}

Diagnostics.empty() ? 0 : 2?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115856

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

[PATCH] D116097: [NFC] [C++20] [Modules] Add tests for template instantiation in transitive imported module

[Chuanqi Xu via Phabricator via cfe-commits]

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

This patch adds two test about template class instantiation in transitively 
imported module. They are used as pre-commit tests for successive patches.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116097

Files:
  clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
  clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
  clang/test/Modules/module-transtive-instantiation-2.cpp
  clang/test/Modules/module-transtive-instantiation.cpp


Index: clang/test/Modules/module-transtive-instantiation.cpp
===
--- /dev/null
+++ clang/test/Modules/module-transtive-instantiation.cpp
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm 
--precompile -o %t/Templ.pcm
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  
--precompile -fprebuilt-module-path=%t -o %t/bar.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import bar;
+int foo() {
+// FIXME: It shouldn't be an error. Since the `G` is already imported in 
bar.
+return bar(); // 
expected-error@Inputs/module-transtive-instantiation/bar.cppm:5 {{definition of 
'G' must be imported from module 'Templ' before it is required}}
+   // expected-note@-1 {{in instantiation of function 
template specialization 'bar' requested here}}
+   // 
expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{definition 
here is not reachable}}
+}
Index: clang/test/Modules/module-transtive-instantiation-2.cpp
===
--- /dev/null
+++ clang/test/Modules/module-transtive-instantiation-2.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm 
--precompile -o %t/Templ.pcm
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  
--precompile -fprebuilt-module-path=%t -o %t/bar.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import bar;
+int foo() {
+G g;// expected-error {{declaration of 'G' must be imported from 
module 'Templ' before it is required}}
+return g();  // 
expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{declaration 
here is not visible}}
+}
Index: clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/module-transtive-instantiation/bar.cppm
@@ -0,0 +1,6 @@
+export module bar;
+import Templ;
+export template
+int bar() {
+return G()();
+}
Index: clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/module-transtive-instantiation/Templ.cppm
@@ -0,0 +1,8 @@
+export module Templ;
+export template 
+class G {
+public:
+T operator()() {
+return T();
+}
+};


Index: clang/test/Modules/module-transtive-instantiation.cpp
===
--- /dev/null
+++ clang/test/Modules/module-transtive-instantiation.cpp
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm --precompile -o %t/Templ.pcm
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  --precompile -fprebuilt-module-path=%t -o %t/bar.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import bar;
+int foo() {
+// FIXME: It shouldn't be an error. Since the `G` is already imported in bar.
+return bar(); // expected-error@Inputs/module-transtive-instantiation/bar.cppm:5 {{definition of 'G' must be imported from module 'Templ' before it is required}}
+   // expected-note@-1 {{in instantiation of function template specialization 'bar' requested here}}
+   // expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{definition here is not reachable}}
+}
Index: clang/test/Modules/module-transtive-instantiation-2.cpp
===
--- /dev/null
+++ clang/test/Modules/module-transtive-instantiation-2.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm --precompile -o %t/Templ.pcm
+// RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  --precompile -fprebuilt-module-path=%t -o %t/bar.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import bar;
+int foo() {
+

[PATCH] D116098: [C++20] [Coroutines] Mark imported module as imported if not exported

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, aaron.ballman, urnathan, iains.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

The intention of this patch is described in the title. I met the problem in the 
original test. The original behavior of the test is clearly incorrect.

And I found this patch could solve the problem. And the original behavior looks 
odd. It only maintain the `Exports` field but not `Imports` field. So I am not 
sure if the behavior was intended or it is just an oversight. I did simple test 
but I am not sure if this change is good.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116098

Files:
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/module-transtive-instantiation.cpp


Index: clang/test/Modules/module-transtive-instantiation.cpp
===
--- clang/test/Modules/module-transtive-instantiation.cpp
+++ clang/test/Modules/module-transtive-instantiation.cpp
@@ -3,11 +3,9 @@
 // RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm 
--precompile -o %t/Templ.pcm
 // RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  
--precompile -fprebuilt-module-path=%t -o %t/bar.pcm
 // RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+// expected-no-diagnostics
 
 import bar;
 int foo() {
-// FIXME: It shouldn't be an error. Since the `G` is already imported in 
bar.
-return bar(); // 
expected-error@Inputs/module-transtive-instantiation/bar.cppm:5 {{definition of 
'G' must be imported from module 'Templ' before it is required}}
-   // expected-note@-1 {{in instantiation of function 
template specialization 'bar' requested here}}
-   // 
expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{definition 
here is not reachable}}
+return bar();
 }
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -387,6 +387,8 @@
   if (!ModuleScopes.empty() && ModuleScopes.back().ModuleInterface) {
 if (ExportLoc.isValid() || getEnclosingExportDecl(Import))
   getCurrentModule()->Exports.emplace_back(Mod, false);
+else
+  getCurrentModule()->Imports.insert(Mod);
   } else if (ExportLoc.isValid()) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface);
   }


Index: clang/test/Modules/module-transtive-instantiation.cpp
===
--- clang/test/Modules/module-transtive-instantiation.cpp
+++ clang/test/Modules/module-transtive-instantiation.cpp
@@ -3,11 +3,9 @@
 // RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/Templ.cppm --precompile -o %t/Templ.pcm
 // RUN: %clang -std=c++20 %S/Inputs/module-transtive-instantiation/bar.cppm  --precompile -fprebuilt-module-path=%t -o %t/bar.pcm
 // RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+// expected-no-diagnostics
 
 import bar;
 int foo() {
-// FIXME: It shouldn't be an error. Since the `G` is already imported in bar.
-return bar(); // expected-error@Inputs/module-transtive-instantiation/bar.cppm:5 {{definition of 'G' must be imported from module 'Templ' before it is required}}
-   // expected-note@-1 {{in instantiation of function template specialization 'bar' requested here}}
-   // expected-note@Inputs/module-transtive-instantiation/Templ.cppm:3 {{definition here is not reachable}}
+return bar();
 }
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -387,6 +387,8 @@
   if (!ModuleScopes.empty() && ModuleScopes.back().ModuleInterface) {
 if (ExportLoc.isValid() || getEnclosingExportDecl(Import))
   getCurrentModule()->Exports.emplace_back(Mod, false);
+else
+  getCurrentModule()->Imports.insert(Mod);
   } else if (ExportLoc.isValid()) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111047: CUDA/HIP: Allow __int128 on the host side

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

linjamaki updated this revision to Diff 395649.
linjamaki added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111047

Files:
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/allow-int128.cu
  clang/test/SemaCUDA/spirv-int128.cu


Index: clang/test/SemaCUDA/spirv-int128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/spirv-int128.cu
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple spirv64 -aux-triple x86_64-unknown-linux-gnu \
+// RUN:   -fcuda-is-device -verify -fsyntax-only %s
+
+#define __device__ __attribute__((device))
+
+__int128 h_glb;
+
+__device__ __int128 d_unused;
+
+// expected-note@+1 {{'d_glb' defined here}}
+__device__ __int128 d_glb;
+
+__device__ __int128 bar() {
+  // expected-error@+1 {{'d_glb' requires 128 bit size '__int128' type 
support, but target 'spirv64' does not support it}}
+  return d_glb;
+}
Index: clang/test/SemaCUDA/allow-int128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/allow-int128.cu
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu \
+// RUN:   -fcuda-is-device -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple nvptx \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu \
+// RUN:   -fcuda-is-device -verify -fsyntax-only %s
+
+// expected-no-diagnostics
+#define __device__ __attribute__((device))
+
+__int128 h_glb;
+__device__ __int128 d_unused;
+__device__ __int128 d_glb;
+__device__ __int128 bar() {
+  return d_glb;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1495,8 +1495,8 @@
   }
   case DeclSpec::TST_int128:
 if (!S.Context.getTargetInfo().hasInt128Type() &&
-!S.getLangOpts().SYCLIsDevice &&
-!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+!(S.getLangOpts().SYCLIsDevice || S.getLangOpts().CUDAIsDevice ||
+  (S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice)))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__int128";
 if (DS.getTypeSpecSign() == TypeSpecifierSign::Unsigned)
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1932,7 +1932,8 @@
   };
 
   auto CheckType = [&](QualType Ty, bool IsRetTy = false) {
-if (LangOpts.SYCLIsDevice || (LangOpts.OpenMP && LangOpts.OpenMPIsDevice))
+if (LangOpts.SYCLIsDevice || (LangOpts.OpenMP && LangOpts.OpenMPIsDevice) 
||
+LangOpts.CUDAIsDevice)
   CheckDeviceType(Ty);
 
 QualType UnqualTy = Ty.getCanonicalType().getUnqualifiedType();


Index: clang/test/SemaCUDA/spirv-int128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/spirv-int128.cu
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple spirv64 -aux-triple x86_64-unknown-linux-gnu \
+// RUN:   -fcuda-is-device -verify -fsyntax-only %s
+
+#define __device__ __attribute__((device))
+
+__int128 h_glb;
+
+__device__ __int128 d_unused;
+
+// expected-note@+1 {{'d_glb' defined here}}
+__device__ __int128 d_glb;
+
+__device__ __int128 bar() {
+  // expected-error@+1 {{'d_glb' requires 128 bit size '__int128' type support, but target 'spirv64' does not support it}}
+  return d_glb;
+}
Index: clang/test/SemaCUDA/allow-int128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/allow-int128.cu
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu \
+// RUN:   -fcuda-is-device -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple nvptx \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu \
+// RUN:   -fcuda-is-device -verify -fsyntax-only %s
+
+// expected-no-diagnostics
+#define __device__ __attribute__((device))
+
+__int128 h_glb;
+__device__ __int128 d_unused;
+__device__ __int128 d_glb;
+__device__ __int128 bar() {
+  return d_glb;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1495,8 +1495,8 @@
   }
   case DeclSpec::TST_int128:
 if (!S.Context.getTargetInfo().hasInt128Type() &&
-!S.getLangOpts().SYCLIsDevice &&
-!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+!(S.getLangOpts().SYCLIsDevice || S.getLangOpts().CUDAIsDevice ||
+  (S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice)))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__int128";
 if (DS.getTypeSpecSign() == TypeSpecifierSign::Unsigned)
Index: clang/lib/Sema/Sema.cpp

[clang] e751d97 - [CodeGen] Avoid some pointer element type accesses

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-21T14:16:28+01:00
New Revision: e751d97863fb48b7dd844e48c0ba564f6970b726

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

LOG: [CodeGen] Avoid some pointer element type accesses

This avoids some pointer element type accesses when compiling
C++ code.

Added: 


Modified: 
clang/lib/CodeGen/Address.h
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGException.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index 0f2d7b1d9e45..37c20291c0e8 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -79,6 +79,18 @@ class Address {
 assert(isValid());
 return Alignment;
   }
+
+  /// Return address with 
diff erent pointer, but same element type and
+  /// alignment.
+  Address withPointer(llvm::Value *NewPointer) const {
+return Address(NewPointer, ElementType, Alignment);
+  }
+
+  /// Return address with 
diff erent alignment, but same pointer and element
+  /// type.
+  Address withAlignment(CharUnits NewAlignment) const {
+return Address(Pointer, ElementType, NewAlignment);
+  }
 };
 
 /// A specialization of Address that requires the address to be an

diff  --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 0df64d4d5d26..a9e25447a91c 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -326,9 +326,9 @@ Address CodeGenFunction::GetAddressOfBaseClass(
   }
 
   // Get the base pointer type.
+  llvm::Type *BaseValueTy = ConvertType((PathEnd[-1])->getType());
   llvm::Type *BasePtrTy =
-  ConvertType((PathEnd[-1])->getType())
-  ->getPointerTo(Value.getType()->getPointerAddressSpace());
+  BaseValueTy->getPointerTo(Value.getType()->getPointerAddressSpace());
 
   QualType DerivedTy = getContext().getRecordType(Derived);
   CharUnits DerivedAlign = CGM.getClassPointerAlignment(Derived);
@@ -342,7 +342,7 @@ Address CodeGenFunction::GetAddressOfBaseClass(
   EmitTypeCheck(TCK_Upcast, Loc, Value.getPointer(),
 DerivedTy, DerivedAlign, SkippedChecks);
 }
-return Builder.CreateBitCast(Value, BasePtrTy);
+return Builder.CreateElementBitCast(Value, BaseValueTy);
   }
 
   llvm::BasicBlock *origBB = nullptr;
@@ -379,7 +379,7 @@ Address CodeGenFunction::GetAddressOfBaseClass(
   VirtualOffset, Derived, VBase);
 
   // Cast to the destination type.
-  Value = Builder.CreateBitCast(Value, BasePtrTy);
+  Value = Builder.CreateElementBitCast(Value, BaseValueTy);
 
   // Build a phi if we needed a null check.
   if (NullCheckValue) {
@@ -406,16 +406,16 @@ CodeGenFunction::GetAddressOfDerivedClass(Address 
BaseAddr,
 
   QualType DerivedTy =
 getContext().getCanonicalType(getContext().getTagDeclType(Derived));
-  unsigned AddrSpace =
-BaseAddr.getPointer()->getType()->getPointerAddressSpace();
-  llvm::Type *DerivedPtrTy = ConvertType(DerivedTy)->getPointerTo(AddrSpace);
+  unsigned AddrSpace = BaseAddr.getAddressSpace();
+  llvm::Type *DerivedValueTy = ConvertType(DerivedTy);
+  llvm::Type *DerivedPtrTy = DerivedValueTy->getPointerTo(AddrSpace);
 
   llvm::Value *NonVirtualOffset =
 CGM.GetNonVirtualBaseClassOffset(Derived, PathBegin, PathEnd);
 
   if (!NonVirtualOffset) {
 // No offset, we can just cast back.
-return Builder.CreateBitCast(BaseAddr, DerivedPtrTy);
+return Builder.CreateElementBitCast(BaseAddr, DerivedValueTy);
   }
 
   llvm::BasicBlock *CastNull = nullptr;
@@ -453,7 +453,7 @@ CodeGenFunction::GetAddressOfDerivedClass(Address BaseAddr,
 Value = PHI;
   }
 
-  return Address(Value, CGM.getClassPointerAlignment(Derived));
+  return Address(Value, DerivedValueTy, CGM.getClassPointerAlignment(Derived));
 }
 
 llvm::Value *CodeGenFunction::GetVTTParameter(GlobalDecl GD,

diff  --git a/clang/lib/CodeGen/CGException.cpp 
b/clang/lib/CodeGen/CGException.cpp
index aff9c77d53c7..91ecbecc843f 100644
--- a/clang/lib/CodeGen/CGException.cpp
+++ b/clang/lib/CodeGen/CGException.cpp
@@ -400,8 +400,8 @@ void CodeGenFunction::EmitAnyExprToExn(const Expr *e, 
Address addr) {
 
   // __cxa_allocate_exception returns a void*;  we need to cast this
   // to the appropriate type for the object.
-  llvm::Type *ty = ConvertTypeForMem(e->getType())->getPointerTo();
-  Address typedAddr = Builder.CreateBitCast(addr, ty);
+  llvm::Type *ty = ConvertTypeForMem(e->getType());
+  Address typedAddr = Builder.CreateElementBitCast(addr, ty);
 
   // FIXME: this isn't quite right!  If there's a final unelided call
   // to a copy constructor, then according to [except.termi

[clang-tools-extra] 6f1a501 - [clangd] Fix typo in test. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-12-21T14:17:25+01:00
New Revision: 6f1a501fddaef72fdc4c7bcccedce0307d111ffb

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

LOG: [clangd] Fix typo in test. NFC

Added: 


Modified: 
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 920e56a21cf7..5cf68c9c8e9c 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -484,7 +484,7 @@ TEST(DiagnosticTest, ClangTidySuppressionComment) {
   // NOLINTEND
 
   // verify no crashes on unmatched nolints.
-  // NOLINTBEIGN
+  // NOLINTBEGIN
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());



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

[PATCH] D111047: CUDA/HIP: Allow __int128 on the host side

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

linjamaki added a comment.

This patch should be ready to land. @tra, could you please commit it to the 
LLVM for us? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111047

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

[clang] 9a05a7b - [CodeGen] Accept Address in CreateLaunderInvariantGroup

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-21T14:43:20+01:00
New Revision: 9a05a7b00ccdbd9edee309834901c619beee8d36

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

LOG: [CodeGen] Accept Address in CreateLaunderInvariantGroup

Add an overload that accepts and returns an Address, as we
generally just want to replace the pointer with a laundered one,
while retaining remaining information.

Added: 


Modified: 
clang/lib/CodeGen/CGBuilder.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprCXX.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index 53537b044f95..5639abf56982 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -344,6 +344,11 @@ class CGBuilderTy : public CGBuilderBaseTy {
ElTy->getElementType(Index),
Addr.getAlignment().alignmentAtOffset(Offset));
   }
+
+  using CGBuilderBaseTy::CreateLaunderInvariantGroup;
+  Address CreateLaunderInvariantGroup(Address Addr) {
+return Addr.withPointer(CreateLaunderInvariantGroup(Addr.getPointer()));
+  }
 };
 
 }  // end namespace CodeGen

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index c3b30de31266..577252fdfeac 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4384,8 +4384,7 @@ LValue CodeGenFunction::EmitLValueForField(LValue base,
 hasAnyVptr(FieldType, getContext()))
   // Because unions can easily skip invariant.barriers, we need to add
   // a barrier every time CXXRecord field with vptr is referenced.
-  addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),
- addr.getAlignment());
+  addr = Builder.CreateLaunderInvariantGroup(addr);
 
 if (IsInPreservedAIRegion ||
 (getDebugInfo() && rec->hasAttr())) {

diff  --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index e9943fb4e210..ca4450a8cf1c 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -1725,8 +1725,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const 
CXXNewExpr *E) {
   // of optimization level.
   if (CGM.getCodeGenOpts().StrictVTablePointers &&
   allocator->isReservedGlobalPlacementOperator())
-result = result.withPointer(
-Builder.CreateLaunderInvariantGroup(result.getPointer()));
+result = Builder.CreateLaunderInvariantGroup(result);
 
   // Emit sanitizer checks for pointer value now, so that in the case of an
   // array it was checked only once and not at each constructor call. We may



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

[PATCH] D116049: [clang-format] Fix SplitEmptyRecord affecting SplitEmptyFunction.

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM, less the clang-format oddities


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116049

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

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

[David Rector via Phabricator via cfe-commits]

davrec added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:59
+   RTy->getCanonicalTypeUnqualified();
+
+  // FIXME: We should probably check the other kinds of nested name specifiers.

steakhal wrote:
> davrec wrote:
> > It occurs to me this needs to be recursive, to check a sequence of 
> > qualifiers, i.e.
> > ```
> >   const Type *LTy = Left->getAsType();
> >   const Type *RTy = Right->getAsType();
> >   if (!LTy || !RTy) 
> > return true;
> >   if (LTy->getCanonicalTypeUnqualified() !=
> >RTy->getCanonicalTypeUnqualified())
> > return false;
> >   return areEquivalentNameSpecifier(Left->getPrefix(), Right->getPrefix());
> > ```
> > 
> > The reason is, if there is a prefix qualifier to this qualifier, we run 
> > into the original problem, e.g.:
> > ```
> > struct Base {
> >   struct Inner { 
> > static const bool value = true;
> >   };
> > };
> > struct Outer1 : Base {};
> > struct Outer2 : Base {};
> > 
> > // We do not want the following to warn, but without checking the prefix of 
> > Inner, 
> > // I believe these will be interpreted as equivalent DeclRefs and will warn:
> > auto sink = Outer1::Inner::value || Outer2::Inner::value;
> > ```
> > 
> You are right, but that would require me to implement all the possible kinds 
> of NNS, in addition to the types.
> And it's not entirely clear to me (yet) what the desired behavior should be 
> for implementing this.
I agree it's not entirely clear what should be the desired behavior in that 
rare case; I suppose it might be appropriate to introduce a `bool 
RecurseToPrefix` param that determines whether you do the while loop or finish 
after one iteration, mostly so that others using this function can ponder the 
decision for themselves.



Comment at: clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp:372
+}
+
 /// Determines whether two statement trees are identical regarding

I would strongly support turning these functions into methods of DeclRefExpr 
and NestedNameSpecifier, not just to avoid the code duplication here but 
because they sure seem to be of general utility to AST users.

You just need to clearly motivate these methods in the documentation, which 
your description for this patch largely does, e.g. something like.:

```
class DeclRefExpr : ... {
  ...
  /// Returns whether this \c DeclRefExpr 
  ///   a. refers to the same canonical declaration as \c other and
  ///   b. if it has a qualifier, that qualifier refers to the same canonical 
  ///   declarations as those of \c other .
  ///
  /// Consider these two DeclRefExprs:
  /// \code
  ///   std::is_same::value
  ///   std::is_same::value;
  /// \endcode
  ///
  /// We can see that the value static members are different, but in fact
  /// the \c getDecl() of these two returns the same VarDecl. (...etc)
  bool isSemanticallyEquivalentTo(const DeclRefExpr *Other) { ... }
};
...
class NestedNameSpecifier {
  ...
  /// Returns whether this refers to the same sequence of identifiers/canonical 
declarations
  /// as \c Other.  (Then I would repeat the std::is_same example here, since 
that
  /// really makes the issue clear.  And also describe what this returns when 
other is nullptr or
  /// when getPrefix() is nullptr while other->getPrefix() is non-null -- maybe 
add a parameter if
  /// its not clear in general what the behavior should be.)
  bool isSemanticallyEquivalentTo(const NestedNameSpecifier *Other, bool 
RecurseToPrefix = true) { ... }
};

```


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

https://reviews.llvm.org/D114622

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

[clang] 6e28b86 - AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

[via cfe-commits]

Author: mydeveloperday
Date: 2021-12-21T13:57:43Z
New Revision: 6e28b86cc629c351b1f5d238146af1b6f7ceb785

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

LOG: AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

https://github.com/llvm/llvm-project/issues/49846

Fixes #49846

AlignConsecutiveDeclarations  is not working for "let" and "const" in JavaScript

let letVariable = 5;
const constVariable = 10;

Reviewed By: owenpan, HazardyKnusperkeks, curdeius

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

Added: 


Modified: 
clang/include/clang/Format/Format.h
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestJS.cpp

Removed: 




diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index d38bc6e3f0e68..0f97e80d425e3 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2669,6 +2669,7 @@ struct FormatStyle {
   bool isCpp() const { return Language == LK_Cpp || Language == LK_ObjC; }
   bool isCSharp() const { return Language == LK_CSharp; }
   bool isJson() const { return Language == LK_Json; }
+  bool isJavaScript() const { return Language == LK_JavaScript; }
 
   /// Language, this format style is targeted at.
   /// \version 3.5

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 5809e4c40070e..e11ba45c1978d 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1826,14 +1826,16 @@ class AnnotatingParser {
 if (Tok.Previous->isOneOf(TT_LeadingJavaAnnotation, Keywords.kw_instanceof,
   Keywords.kw_as))
   return false;
-if (Style.Language == FormatStyle::LK_JavaScript &&
-Tok.Previous->is(Keywords.kw_in))
+if (Style.isJavaScript() && Tok.Previous->is(Keywords.kw_in))
   return false;
 
 // Skip "const" as it does not have an influence on whether this is a name.
 FormatToken *PreviousNotConst = Tok.getPreviousNonComment();
-while (PreviousNotConst && PreviousNotConst->is(tok::kw_const))
-  PreviousNotConst = PreviousNotConst->getPreviousNonComment();
+
+// For javascript const can be like "let" or "var"
+if (!Style.isJavaScript())
+  while (PreviousNotConst && PreviousNotConst->is(tok::kw_const))
+PreviousNotConst = PreviousNotConst->getPreviousNonComment();
 
 if (!PreviousNotConst)
   return false;
@@ -1852,10 +1854,24 @@ class AnnotatingParser {
 PreviousNotConst->is(TT_TypeDeclarationParen))
   return true;
 
-return (!IsPPKeyword &&
-PreviousNotConst->isOneOf(tok::identifier, tok::kw_auto)) ||
-   PreviousNotConst->is(TT_PointerOrReference) ||
-   PreviousNotConst->isSimpleTypeSpecifier();
+// If is a preprocess keyword like #define.
+if (IsPPKeyword)
+  return false;
+
+// int a or auto a.
+if (PreviousNotConst->isOneOf(tok::identifier, tok::kw_auto))
+  return true;
+
+// *a or &a or &&a.
+if (PreviousNotConst->is(TT_PointerOrReference))
+  return true;
+
+// MyClass a;
+if (PreviousNotConst->isSimpleTypeSpecifier())
+  return true;
+
+// const a = in JavaScript.
+return (Style.isJavaScript() && PreviousNotConst->is(tok::kw_const));
   }
 
   /// Determine whether ')' is ending a cast.

diff  --git a/clang/unittests/Format/FormatTestJS.cpp 
b/clang/unittests/Format/FormatTestJS.cpp
index 069b7b19e316e..7b7bccda0805d 100644
--- a/clang/unittests/Format/FormatTestJS.cpp
+++ b/clang/unittests/Format/FormatTestJS.cpp
@@ -2696,5 +2696,115 @@ TEST_F(FormatTestJS, NumericSeparators) {
   verifyFormat("x = 1_000_000 + 12;", "x = 1_000_000   + 12;");
 }
 
+TEST_F(FormatTestJS, AlignConsecutiveDeclarations) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("letletVariable = 5;\n"
+   "double constVariable = 10;",
+   Style);
+
+  verifyFormat("let   letVariable = 5;\n"
+   "const constVariable = 10;",
+   Style);
+
+  verifyFormat("let  letVariable = 5;\n"
+   "static const constVariable = 10;",
+   Style);
+
+  verifyFormat("letletVariable = 5;\n"
+   "static var constVariable = 10;",
+   Style);
+
+  verifyFormat("let letVariable = 5;\n"
+   "var constVariable = 10;",
+   Style);
+
+  verifyFormat("double letVariable = 5;\n"
+   "varconstVariable = 10;",
+   Style);
+
+  verifyFormat("const letVariable = 5;\n"
+   "var   constVariable = 10;",
+   Style);
+
+  verifyFormat(

[PATCH] D115990: AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

[MyDeveloperDay 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 rG6e28b86cc629: AlignConsecutiveDeclarations not working for 
'const' keyword in JavsScript (authored by MyDeveloperDay).

Changed prior to commit:
  https://reviews.llvm.org/D115990?vs=395329&id=395660#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115990

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestJS.cpp

Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2696,5 +2696,115 @@
   verifyFormat("x = 1_000_000 + 12;", "x = 1_000_000   + 12;");
 }
 
+TEST_F(FormatTestJS, AlignConsecutiveDeclarations) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("letletVariable = 5;\n"
+   "double constVariable = 10;",
+   Style);
+
+  verifyFormat("let   letVariable = 5;\n"
+   "const constVariable = 10;",
+   Style);
+
+  verifyFormat("let  letVariable = 5;\n"
+   "static const constVariable = 10;",
+   Style);
+
+  verifyFormat("letletVariable = 5;\n"
+   "static var constVariable = 10;",
+   Style);
+
+  verifyFormat("let letVariable = 5;\n"
+   "var constVariable = 10;",
+   Style);
+
+  verifyFormat("double letVariable = 5;\n"
+   "varconstVariable = 10;",
+   Style);
+
+  verifyFormat("const letVariable = 5;\n"
+   "var   constVariable = 10;",
+   Style);
+
+  verifyFormat("int letVariable = 5;\n"
+   "int constVariable = 10;",
+   Style);
+}
+
+TEST_F(FormatTestJS, AlignConsecutiveAssignments) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  verifyFormat("let letVariable  = 5;\n"
+   "double constVariable = 10;",
+   Style);
+
+  verifyFormat("let letVariable = 5;\n"
+   "const constVariable = 10;",
+   Style);
+
+  verifyFormat("let letVariable= 5;\n"
+   "static const constVariable = 10;",
+   Style);
+
+  verifyFormat("let letVariable  = 5;\n"
+   "static var constVariable = 10;",
+   Style);
+
+  verifyFormat("let letVariable   = 5;\n"
+   "var constVariable = 10;",
+   Style);
+
+  verifyFormat("double letVariable = 5;\n"
+   "var constVariable  = 10;",
+   Style);
+
+  verifyFormat("const letVariable = 5;\n"
+   "var constVariable = 10;",
+   Style);
+
+  verifyFormat("int letVariable   = 5;\n"
+   "int constVariable = 10;",
+   Style);
+}
+
+TEST_F(FormatTestJS, AlignConsecutiveAssignmentsAndDeclarations) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  verifyFormat("letletVariable   = 5;\n"
+   "double constVariable = 10;",
+   Style);
+
+  verifyFormat("let   letVariable   = 5;\n"
+   "const constVariable = 10;",
+   Style);
+
+  verifyFormat("let  letVariable   = 5;\n"
+   "static const constVariable = 10;",
+   Style);
+
+  verifyFormat("letletVariable   = 5;\n"
+   "static var constVariable = 10;",
+   Style);
+
+  verifyFormat("let letVariable   = 5;\n"
+   "var constVariable = 10;",
+   Style);
+
+  verifyFormat("double letVariable   = 5;\n"
+   "varconstVariable = 10;",
+   Style);
+
+  verifyFormat("const letVariable   = 5;\n"
+   "var   constVariable = 10;",
+   Style);
+
+  verifyFormat("int letVariable   = 5;\n"
+   "int constVariable = 10;",
+   Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1826,14 +1826,16 @@
 if (Tok.Previous->isOneOf(TT_LeadingJavaAnnotation, Keywords.kw_instanceof,
   Keywords.kw_as))
   return false;
-if (Style.Language == FormatStyle::LK_JavaScript &&
-Tok.Previous->is(Keywords.kw_in))
+if (Style.isJavaScript() && Tok.Previous->is(Keywords.kw_in))
   return f

[clang] 142e79b - [clang-format] NFC use recently added Style.isJavaScript()

[via cfe-commits]

Author: mydeveloperday
Date: 2021-12-21T14:24:12Z
New Revision: 142e79b8681bae42bebf3eef97fdaa30ce707c67

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

LOG: [clang-format] NFC use recently added Style.isJavaScript()

Improve the readability of these if(Style==FormatStyle::LK_JavsScript) clauses

Added: 


Modified: 
clang/lib/Format/BreakableToken.cpp
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/Format.cpp
clang/lib/Format/FormatTokenLexer.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp

Removed: 




diff  --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index 968b35bfda23..5d03c9811e1b 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -91,7 +91,7 @@ getCommentSplit(StringRef Text, unsigned ContentStartColumn,
   // In JavaScript, some @tags can be followed by {, and machinery that parses
   // these comments will fail to understand the comment if followed by a line
   // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript) {
+  if (Style.isJavaScript()) {
 StringRef::size_type SpaceOffset =
 Text.find_first_of(Blanks, MaxSplitBytes);
 if (SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
@@ -127,8 +127,7 @@ getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 }
 
 // Avoid ever breaking before a @tag or a { in JavaScript.
-if (Style.Language == FormatStyle::LK_JavaScript &&
-SpaceOffset + 1 < Text.size() &&
+if (Style.isJavaScript() && SpaceOffset + 1 < Text.size() &&
 (Text[SpaceOffset + 1] == '{' || Text[SpaceOffset + 1] == '@')) {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
   continue;
@@ -460,8 +459,7 @@ BreakableBlockComment::BreakableBlockComment(
   IndentAtLineBreak = std::max(IndentAtLineBreak, Decoration.size());
 
   // Detect a multiline jsdoc comment and set DelimitersOnNewline in that case.
-  if (Style.Language == FormatStyle::LK_JavaScript ||
-  Style.Language == FormatStyle::LK_Java) {
+  if (Style.isJavaScript() || Style.Language == FormatStyle::LK_Java) {
 if ((Lines[0] == "*" || Lines[0].startswith("* ")) && Lines.size() > 1) {
   // This is a multiline jsdoc comment.
   DelimitersOnNewline = true;
@@ -580,8 +578,7 @@ const llvm::StringSet<>
 };
 
 unsigned BreakableBlockComment::getContentIndent(unsigned LineIndex) const {
-  if (Style.Language != FormatStyle::LK_Java &&
-  Style.Language != FormatStyle::LK_JavaScript)
+  if (Style.Language != FormatStyle::LK_Java && !Style.isJavaScript())
 return 0;
   // The content at LineIndex 0 of a comment like:
   // /** line 0 */

diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 104df6494d8d..4225d6b67b0e 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -422,8 +422,7 @@ bool ContinuationIndenter::mustBreak(const LineState 
&State) {
   // ...
   //   }.bind(...));
   // FIXME: We should find a more generic solution to this problem.
-  !(State.Column <= NewLineColumn &&
-Style.Language == FormatStyle::LK_JavaScript) &&
+  !(State.Column <= NewLineColumn && Style.isJavaScript()) &&
   !(Previous.closesScopeAfterBlock() && State.Column <= NewLineColumn))
 return true;
 
@@ -493,8 +492,8 @@ bool ContinuationIndenter::mustBreak(const LineState 
&State) {
   return true;
   }
 
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) {
+  if (Style.isJavaScript() && Previous.is(tok::r_paren) &&
+  Previous.is(TT_JavaAnnotation)) {
 // Break after the closing parenthesis of TypeScript decorators before
 // functions, getters and setters.
 static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
@@ -510,7 +509,7 @@ bool ContinuationIndenter::mustBreak(const LineState 
&State) {
  Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
 // Don't always break between a JavaScript `function` and the function
 // name.
-Style.Language != FormatStyle::LK_JavaScript) ||
+!Style.isJavaScript()) ||
(Current.is(tok::kw_operator) && !Previous.is(tok::coloncolon))) &&
   !Previous.is(tok::kw_template) && 
State.Stack.back().BreakBeforeParameter)
 return true;
@@ -827,9 +826,8 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState 
&State,
   // is common and should be formatted like a free-standing function. The same
   // goes for wrapping before the lambda 

[PATCH] D116068: [NFC][Clang] Move function implementation of `OpenMPAtomicUpdateChecker` into anonymous namespace

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116068

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

[clang] f44e3fb - [CodeView] Emit S_OBJNAME record

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-12-21T09:26:36-05:00
New Revision: f44e3fbadd15bc851c6e3c2a40ddf5f0a502151a

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

LOG: [CodeView] Emit S_OBJNAME record

Thanks to @zturner for the initial patch!

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

Added: 
clang/test/CodeGenCXX/debug-info-objname.cpp

Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Job.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CMakeLists.txt
llvm/include/llvm/MC/MCTargetOptions.h
llvm/include/llvm/Support/Caching.h
llvm/include/llvm/Support/ToolOutputFile.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Support/Caching.cpp
llvm/test/DebugInfo/COFF/globals.ll
llvm/test/DebugInfo/COFF/multifunction.ll
llvm/test/DebugInfo/COFF/pr28747.ll
llvm/test/DebugInfo/COFF/simple.ll
llvm/test/DebugInfo/COFF/vframe-fpo.ll
llvm/test/MC/AArch64/coff-debug.ll
llvm/test/MC/ARM/coff-debugging-secrel.ll
llvm/test/MC/COFF/cv-compiler-info.ll
llvm/tools/llc/llc.cpp
llvm/tools/llvm-lto2/llvm-lto2.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 664e4998b8dec..d4781b647b877 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -227,6 +227,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// Output filename for the split debug info, not used in the skeleton CU.
   std::string SplitDwarfOutput;
 
+  /// Output filename used in the COFF debug information.
+  std::string ObjectFilenameForDebug;
+
   /// The name of the relocation model to use.
   llvm::Reloc::Model RelocationModel;
 

diff  --git a/clang/include/clang/Driver/Job.h 
b/clang/include/clang/Driver/Job.h
index 8b287638a271d..6e3b51f2a7995 100644
--- a/clang/include/clang/Driver/Job.h
+++ b/clang/include/clang/Driver/Job.h
@@ -204,6 +204,10 @@ class Command {
   /// from the parent process will be used.
   virtual void setEnvironment(llvm::ArrayRef NewEnvironment);
 
+  void replaceArguments(llvm::opt::ArgStringList List) {
+Arguments = std::move(List);
+  }
+
   const char *getExecutable() const { return Executable; }
 
   const llvm::opt::ArgStringList &getArguments() const { return Arguments; }

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3b47512501d39..08e9e1a3432ae 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3802,6 +3802,11 @@ def o : JoinedOrSeparate<["-"], "o">, 
Flags<[NoXarchOption, RenderAsInput,
   CC1Option, CC1AsOption, FC1Option, FlangOption]>,
   HelpText<"Write output to ">, MetaVarName<"">,
   MarshallingInfoString>;
+def object_file_name_EQ : Joined<["-"], "object-file-name=">, 
Flags<[CC1Option, CC1AsOption, CoreOption]>,
+  HelpText<"Set the output  for debug infos">, MetaVarName<"">,
+  MarshallingInfoString>;
+def object_file_name : Separate<["-"], "object-file-name">, Flags<[CC1Option, 
CC1AsOption, CoreOption]>,
+Alias;
 def pagezero__size : JoinedOrSeparate<["-"], "pagezero_size">;
 def pass_exit_codes : Flag<["-", "--"], "pass-exit-codes">, 
Flags<[Unsupported]>;
 def pedantic_errors : Flag<["-", "--"], "pedantic-errors">, 
Group, Flags<[CC1Option]>,

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 3195615ae561c..5e16d3525b383 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -646,6 +646,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
   Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
   Options.DebugStrictDwarf = CodeGenOpts.DebugStrictDwarf;
+  Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug;
 
   return true;
 }
@@ -1583,7 +1584,8 @@ static void runThinLTOBackend(
 return;
 
   auto AddStream = [&](size_t Task) {
-return std::make_unique(std::move(OS));
+return std::make_unique(std::move(OS),
+  CGOpts.ObjectFilenameForDebug);
   };
   lto::Config Conf;
   if (CGOpts.SaveTempsFilePrefix != "") {

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index d76f810f1a7f0..2a3723975568b 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -625,8 +625,9 @@ getFramePointerKind(const ArgList &Args, const llv

[clang] a995cda - [CodeGen] Avoid more pointer element type accesses

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-21T15:52:18+01:00
New Revision: a995cdab190895382448555b7072a50151768534

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

LOG: [CodeGen] Avoid more pointer element type accesses

Added: 


Modified: 
clang/lib/CodeGen/CGClass.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index a9e25447a91c..8f99ff0d50ff 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -127,18 +127,18 @@ CodeGenModule::getDynamicOffsetAlignment(CharUnits 
actualBaseAlign,
 
 Address CodeGenFunction::LoadCXXThisAddress() {
   assert(CurFuncDecl && "loading 'this' without a func declaration?");
-  assert(isa(CurFuncDecl));
+  auto *MD = cast(CurFuncDecl);
 
   // Lazily compute CXXThisAlignment.
   if (CXXThisAlignment.isZero()) {
 // Just use the best known alignment for the parent.
 // TODO: if we're currently emitting a complete-object ctor/dtor,
 // we can always use the complete-object alignment.
-auto RD = cast(CurFuncDecl)->getParent();
-CXXThisAlignment = CGM.getClassPointerAlignment(RD);
+CXXThisAlignment = CGM.getClassPointerAlignment(MD->getParent());
   }
 
-  return Address(LoadCXXThis(), CXXThisAlignment);
+  llvm::Type *Ty = ConvertType(MD->getThisType()->getPointeeType());
+  return Address(LoadCXXThis(), Ty, CXXThisAlignment);
 }
 
 /// Emit the address of a field using a member data pointer.
@@ -286,7 +286,7 @@ ApplyNonVirtualAndVirtualOffset(CodeGenFunction &CGF, 
Address addr,
   }
   alignment = alignment.alignmentAtOffset(nonVirtualOffset);
 
-  return Address(ptr, alignment);
+  return Address(ptr, CGF.Int8Ty, alignment);
 }
 
 Address CodeGenFunction::GetAddressOfBaseClass(
@@ -996,16 +996,8 @@ namespace {
 
   private:
 void emitMemcpyIR(Address DestPtr, Address SrcPtr, CharUnits Size) {
-  llvm::PointerType *DPT = DestPtr.getType();
-  llvm::Type *DBP =
-llvm::Type::getInt8PtrTy(CGF.getLLVMContext(), DPT->getAddressSpace());
-  DestPtr = CGF.Builder.CreateBitCast(DestPtr, DBP);
-
-  llvm::PointerType *SPT = SrcPtr.getType();
-  llvm::Type *SBP =
-llvm::Type::getInt8PtrTy(CGF.getLLVMContext(), SPT->getAddressSpace());
-  SrcPtr = CGF.Builder.CreateBitCast(SrcPtr, SBP);
-
+  DestPtr = CGF.Builder.CreateElementBitCast(DestPtr, CGF.Int8Ty);
+  SrcPtr = CGF.Builder.CreateElementBitCast(SrcPtr, CGF.Int8Ty);
   CGF.Builder.CreateMemCpy(DestPtr, SrcPtr, Size.getQuantity());
 }
 
@@ -2068,8 +2060,8 @@ void CodeGenFunction::EmitCXXConstructorCall(const 
CXXConstructorDecl *D,
 
   if (SlotAS != ThisAS) {
 unsigned TargetThisAS = getContext().getTargetAddressSpace(ThisAS);
-llvm::Type *NewType =
-
ThisPtr->getType()->getPointerElementType()->getPointerTo(TargetThisAS);
+llvm::Type *NewType = llvm::PointerType::getWithSamePointeeType(
+This.getType(), TargetThisAS);
 ThisPtr = getTargetHooks().performAddrSpaceCast(*this, This.getPointer(),
 ThisAS, SlotAS, NewType);
   }
@@ -2507,9 +2499,6 @@ void CodeGenFunction::InitializeVTablePointer(const VPtr 
&Vptr) {
 
   // Apply the offsets.
   Address VTableField = LoadCXXThisAddress();
-  unsigned ThisAddrSpace =
-  VTableField.getPointer()->getType()->getPointerAddressSpace();
-
   if (!NonVirtualOffset.isZero() || VirtualOffset)
 VTableField = ApplyNonVirtualAndVirtualOffset(
 *this, VTableField, NonVirtualOffset, VirtualOffset, Vptr.VTableClass,
@@ -2525,8 +2514,7 @@ void CodeGenFunction::InitializeVTablePointer(const VPtr 
&Vptr) {
   ->getPointerTo(GlobalsAS);
   // vtable field is is derived from `this` pointer, therefore they should be 
in
   // the same addr space. Note that this might not be LLVM address space 0.
-  VTableField = Builder.CreateBitCast(VTableField,
-  
VTablePtrTy->getPointerTo(ThisAddrSpace));
+  VTableField = Builder.CreateElementBitCast(VTableField, VTablePtrTy);
   VTableAddressPoint = Builder.CreateBitCast(VTableAddressPoint, VTablePtrTy);
 
   llvm::StoreInst *Store = Builder.CreateStore(VTableAddressPoint, 
VTableField);



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

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D115231#3197112 , @junaire wrote:

> Thanks for accepting this patch! I would appreciate it if someone is willing 
> to commit this for me since I don't have commit access ;D
>
> You can use:
> Jun Zhang 
> j...@junz.org

It looks like the patch doesn't apply cleanly on current main. Could you rebase 
the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115231

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

Re: [PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

[Kadir Çetinkaya via cfe-commits]

Hi Salman, it looks like patch doesn't have edit access for anyone but you.
I had drafted some comments but can't hit the submit button.
I think you can go to
https://reviews.llvm.org/differential/revision/edit/116085/ and change the
`Editable By` field to `All Users`

On Tue, Dec 21, 2021 at 8:45 AM Salman Javed via Phabricator <
revi...@reviews.llvm.org> wrote:

> salman-javed-nz updated this revision to Diff 395606.
> salman-javed-nz added a comment.
>
> Remove unnecessary `llvm::` qualification.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D116085/new/
>
> https://reviews.llvm.org/D116085
>
> Files:
>   clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
>   clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
>   clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
>   clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp
>
> clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
>   clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116110: Introduce the AttributeMask class

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: nikic, rnk.
Herald added subscribers: ormris, dexonsmith, jdoerfert, hiraditya.
serge-sans-paille requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This class is solely used as a lightweight and clean way to build a set of
attributes to be removed from an AttrBuilder. Previously AttrBuilder was used
both for building and removing, which introduced odd situation like creation of
Attribute with dummy value because the only relevant part was the attribute
kind.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116110

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/IR/Argument.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Scalar/SCCP.cpp
  llvm/unittests/IR/AttributesTest.cpp

Index: llvm/unittests/IR/AttributesTest.cpp
===
--- llvm/unittests/IR/AttributesTest.cpp
+++ llvm/unittests/IR/AttributesTest.cpp
@@ -81,12 +81,12 @@
   AttrBuilder B_align_readonly;
   B_align_readonly.addAttribute(AlignAttr);
   B_align_readonly.addAttribute(Attribute::ReadOnly);
-  AttrBuilder B_align;
+  AttributeMask B_align;
   B_align.addAttribute(AlignAttr);
   AttrBuilder B_stackalign_optnone;
   B_stackalign_optnone.addAttribute(StackAlignAttr);
   B_stackalign_optnone.addAttribute(Attribute::OptimizeNone);
-  AttrBuilder B_stackalign;
+  AttributeMask B_stackalign;
   B_stackalign.addAttribute(StackAlignAttr);
 
   AttributeSet AS = AttributeSet::get(C, B_align_readonly);
Index: llvm/lib/Transforms/Scalar/SCCP.cpp
===
--- llvm/lib/Transforms/Scalar/SCCP.cpp
+++ llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -486,7 +486,7 @@
   // inaccessiblemem_or_argmemonly attributes do not hold any longer. Remove
   // them from both the function and callsites.
   if (ReplacedPointerArg) {
-AttrBuilder AttributesToRemove;
+AttributeMask AttributesToRemove;
 AttributesToRemove.addAttribute(Attribute::ArgMemOnly);
 AttributesToRemove.addAttribute(Attribute::InaccessibleMemOrArgMemOnly);
 F.removeFnAttrs(AttributesToRemove);
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1383,7 +1383,7 @@
 
   for (Attribute A : AL.getFnAttrs()) {
 if (isStatepointDirectiveAttr(A))
-  FnAttrs.remove(A);
+  FnAttrs.removeAttribute(A);
   }
 
   // Just skip parameter and return attributes for now
@@ -2654,7 +2654,7 @@
 template 
 static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH,
   unsigned Index) {
-  AttrBuilder R;
+  AttributeMask R;
   AttributeSet AS = AH.getAttributes().getAttributes(Index);
   if (AS.getDereferenceableBytes())
 R.addAttribute(Attribute::get(Ctx, Attribute::Dereferenceable,
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3664,7 +3664,7 @@
   // will become a non-readonly function after it is instrumented by us. To
   // prevent this code from being optimized out, mark that function
   // non-readonly in advance.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
@@ -5359,7 +5359,7 @@
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -454,7 +454,7 @@
   MDNode *OriginStoreWeights;
   DFSanABIList ABIList;
   DenseMap UnwrappedFnMap;
-  AttrBuilder ReadOnlyNoneAttrs;
+  AttributeMask ReadOnlyNoneAttrs;
 
   /// Memory map parameters used in calculati

[clang] d4de2a4 - [clang][NFC] Refactor coroutine_traits lookup

[Nathan Sidwell via cfe-commits]

Author: Nathan Sidwell
Date: 2021-12-21T10:29:01-05:00
New Revision: d4de2a4d5905beb0602a615593daf003378ddb64

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

LOG: [clang][NFC] Refactor coroutine_traits lookup

To allow transition from the TS-specified
std::experimental::coroutine_traits to the C++20-specified
std::coroutine_traits, we lookup in both places and provide helpful
diagnostics. This refactors the code to avoid separate paths to
std::experimental lookups.

Reviewed By: ChuanqiXu

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

Added: 


Modified: 
clang/lib/Sema/SemaCoroutine.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index d4a919e8c1656..27ba49fb001c0 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1661,45 +1661,53 @@ ClassTemplateDecl 
*Sema::lookupCoroutineTraits(SourceLocation KwLoc,
SourceLocation FuncLoc,
NamespaceDecl *&Namespace) {
   if (!StdCoroutineTraitsCache) {
-NamespaceDecl *CoroNamespace = getStdNamespace();
-LookupResult Result(*this, 
&PP.getIdentifierTable().get("coroutine_traits"),
-FuncLoc, LookupOrdinaryName);
-
-if (!CoroNamespace || !LookupQualifiedName(Result, CoroNamespace)) {
-  /// Look up in namespace std::experimental, for compatibility.
-  /// TODO: Remove this extra lookup when  is
-  /// removed.
-  CoroNamespace = lookupStdExperimentalNamespace();
-  if (!CoroNamespace || !LookupQualifiedName(Result, CoroNamespace)) {
-Diag(KwLoc, diag::err_implied_coroutine_type_not_found)
-<< "std::coroutine_traits";
-return nullptr;
-  }
+// Because coroutines moved from std::experimental in the TS to std in
+// C++20, we look in both places to give users time to transition their
+// TS-specific code to C++20.  Diagnostics are given when the TS usage is
+// discovered.
+// TODO: Become stricter when  is removed.
+
+auto const &TraitIdent = PP.getIdentifierTable().get("coroutine_traits");
+
+NamespaceDecl *StdSpace = getStdNamespace();
+LookupResult ResStd(*this, &TraitIdent, FuncLoc, LookupOrdinaryName);
+bool InStd = StdSpace && LookupQualifiedName(ResStd, StdSpace);
+
+NamespaceDecl *ExpSpace = lookupStdExperimentalNamespace();
+LookupResult ResExp(*this, &TraitIdent, FuncLoc, LookupOrdinaryName);
+bool InExp = ExpSpace && LookupQualifiedName(ResExp, ExpSpace);
+
+if (!InStd && !InExp) {
+  // The goggles, they found nothing!
+  Diag(KwLoc, diag::err_implied_coroutine_type_not_found)
+  << "std::coroutine_traits";
+  return nullptr;
+}
+
+if (!InStd) {
+  // Found only in std::experimental.
   Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
   << "coroutine_traits";
-} else {
-  /// When we found coroutine_traits in std namespace. Make sure there is 
no
-  /// misleading definition in std::experimental namespace.
-  NamespaceDecl *ExpNamespace = lookupStdExperimentalNamespace();
-  LookupResult ExpResult(*this,
- &PP.getIdentifierTable().get("coroutine_traits"),
- FuncLoc, LookupOrdinaryName);
-  if (ExpNamespace && LookupQualifiedName(ExpResult, ExpNamespace)) {
-Diag(KwLoc,
- diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
-Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
-<< "coroutine_traits";
-return nullptr;
-  }
+} else if (InExp) {
+  // Found in std and std::experimental.
+  Diag(KwLoc,
+   diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
+  Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
+  << "coroutine_traits";
+  return nullptr;
 }
 
+// Prefer ::std to std::experimental.
+auto &Result = InStd ? ResStd : ResExp;
+CoroTraitsNamespaceCache = InStd ? StdSpace : ExpSpace;
+
+// coroutine_traits is required to be a class template.
 if (!(StdCoroutineTraitsCache = Result.getAsSingle())) {
   Result.suppressDiagnostics();
   NamedDecl *Found = *Result.begin();
   Diag(Found->getLocation(), diag::err_malformed_std_coroutine_traits);
   return nullptr;
 }
-CoroTraitsNamespaceCache = CoroNamespace;
   }
   Namespace = CoroTraitsNamespaceCache;
   return StdCoroutineTraitsCache;



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

[PATCH] D116029: [clang][NFC] Refactor coroutine_traits lookup

[Nathan Sidwell 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 rGd4de2a4d5905: [clang][NFC] Refactor coroutine_traits lookup 
(authored by urnathan).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D116029?vs=395504&id=395676#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116029

Files:
  clang/lib/Sema/SemaCoroutine.cpp


Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1661,45 +1661,53 @@
SourceLocation FuncLoc,
NamespaceDecl *&Namespace) {
   if (!StdCoroutineTraitsCache) {
-NamespaceDecl *CoroNamespace = getStdNamespace();
-LookupResult Result(*this, 
&PP.getIdentifierTable().get("coroutine_traits"),
-FuncLoc, LookupOrdinaryName);
-
-if (!CoroNamespace || !LookupQualifiedName(Result, CoroNamespace)) {
-  /// Look up in namespace std::experimental, for compatibility.
-  /// TODO: Remove this extra lookup when  is
-  /// removed.
-  CoroNamespace = lookupStdExperimentalNamespace();
-  if (!CoroNamespace || !LookupQualifiedName(Result, CoroNamespace)) {
-Diag(KwLoc, diag::err_implied_coroutine_type_not_found)
-<< "std::coroutine_traits";
-return nullptr;
-  }
+// Because coroutines moved from std::experimental in the TS to std in
+// C++20, we look in both places to give users time to transition their
+// TS-specific code to C++20.  Diagnostics are given when the TS usage is
+// discovered.
+// TODO: Become stricter when  is removed.
+
+auto const &TraitIdent = PP.getIdentifierTable().get("coroutine_traits");
+
+NamespaceDecl *StdSpace = getStdNamespace();
+LookupResult ResStd(*this, &TraitIdent, FuncLoc, LookupOrdinaryName);
+bool InStd = StdSpace && LookupQualifiedName(ResStd, StdSpace);
+
+NamespaceDecl *ExpSpace = lookupStdExperimentalNamespace();
+LookupResult ResExp(*this, &TraitIdent, FuncLoc, LookupOrdinaryName);
+bool InExp = ExpSpace && LookupQualifiedName(ResExp, ExpSpace);
+
+if (!InStd && !InExp) {
+  // The goggles, they found nothing!
+  Diag(KwLoc, diag::err_implied_coroutine_type_not_found)
+  << "std::coroutine_traits";
+  return nullptr;
+}
+
+if (!InStd) {
+  // Found only in std::experimental.
   Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
   << "coroutine_traits";
-} else {
-  /// When we found coroutine_traits in std namespace. Make sure there is 
no
-  /// misleading definition in std::experimental namespace.
-  NamespaceDecl *ExpNamespace = lookupStdExperimentalNamespace();
-  LookupResult ExpResult(*this,
- &PP.getIdentifierTable().get("coroutine_traits"),
- FuncLoc, LookupOrdinaryName);
-  if (ExpNamespace && LookupQualifiedName(ExpResult, ExpNamespace)) {
-Diag(KwLoc,
- diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
-Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
-<< "coroutine_traits";
-return nullptr;
-  }
+} else if (InExp) {
+  // Found in std and std::experimental.
+  Diag(KwLoc,
+   diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
+  Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
+  << "coroutine_traits";
+  return nullptr;
 }
 
+// Prefer ::std to std::experimental.
+auto &Result = InStd ? ResStd : ResExp;
+CoroTraitsNamespaceCache = InStd ? StdSpace : ExpSpace;
+
+// coroutine_traits is required to be a class template.
 if (!(StdCoroutineTraitsCache = Result.getAsSingle())) {
   Result.suppressDiagnostics();
   NamedDecl *Found = *Result.begin();
   Diag(Found->getLocation(), diag::err_malformed_std_coroutine_traits);
   return nullptr;
 }
-CoroTraitsNamespaceCache = CoroNamespace;
   }
   Namespace = CoroTraitsNamespaceCache;
   return StdCoroutineTraitsCache;


Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1661,45 +1661,53 @@
SourceLocation FuncLoc,
NamespaceDecl *&Namespace) {
   if (!StdCoroutineTraitsCache) {
-NamespaceDecl *CoroNamespace = getStdNamespace();
-LookupResult Result(*this, &PP.getIdentifierTable().get("coroutine_traits"),
-FuncLoc, LookupOrdinaryName);
-
-   

[PATCH] D116110: Introduce the AttributeMask class

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 395677.
serge-sans-paille added a comment.

Minor nits


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

https://reviews.llvm.org/D116110

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/IR/Argument.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Scalar/SCCP.cpp
  llvm/unittests/IR/AttributesTest.cpp

Index: llvm/unittests/IR/AttributesTest.cpp
===
--- llvm/unittests/IR/AttributesTest.cpp
+++ llvm/unittests/IR/AttributesTest.cpp
@@ -81,12 +81,12 @@
   AttrBuilder B_align_readonly;
   B_align_readonly.addAttribute(AlignAttr);
   B_align_readonly.addAttribute(Attribute::ReadOnly);
-  AttrBuilder B_align;
+  AttributeMask B_align;
   B_align.addAttribute(AlignAttr);
   AttrBuilder B_stackalign_optnone;
   B_stackalign_optnone.addAttribute(StackAlignAttr);
   B_stackalign_optnone.addAttribute(Attribute::OptimizeNone);
-  AttrBuilder B_stackalign;
+  AttributeMask B_stackalign;
   B_stackalign.addAttribute(StackAlignAttr);
 
   AttributeSet AS = AttributeSet::get(C, B_align_readonly);
Index: llvm/lib/Transforms/Scalar/SCCP.cpp
===
--- llvm/lib/Transforms/Scalar/SCCP.cpp
+++ llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -486,7 +486,7 @@
   // inaccessiblemem_or_argmemonly attributes do not hold any longer. Remove
   // them from both the function and callsites.
   if (ReplacedPointerArg) {
-AttrBuilder AttributesToRemove;
+AttributeMask AttributesToRemove;
 AttributesToRemove.addAttribute(Attribute::ArgMemOnly);
 AttributesToRemove.addAttribute(Attribute::InaccessibleMemOrArgMemOnly);
 F.removeFnAttrs(AttributesToRemove);
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1383,7 +1383,7 @@
 
   for (Attribute A : AL.getFnAttrs()) {
 if (isStatepointDirectiveAttr(A))
-  FnAttrs.remove(A);
+  FnAttrs.removeAttribute(A);
   }
 
   // Just skip parameter and return attributes for now
@@ -2654,7 +2654,7 @@
 template 
 static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH,
   unsigned Index) {
-  AttrBuilder R;
+  AttributeMask R;
   AttributeSet AS = AH.getAttributes().getAttributes(Index);
   if (AS.getDereferenceableBytes())
 R.addAttribute(Attribute::get(Ctx, Attribute::Dereferenceable,
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3664,7 +3664,7 @@
   // will become a non-readonly function after it is instrumented by us. To
   // prevent this code from being optimized out, mark that function
   // non-readonly in advance.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
@@ -5359,7 +5359,7 @@
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -454,7 +454,7 @@
   MDNode *OriginStoreWeights;
   DFSanABIList ABIList;
   DenseMap UnwrappedFnMap;
-  AttrBuilder ReadOnlyNoneAttrs;
+  AttributeMask ReadOnlyNoneAttrs;
 
   /// Memory map parameters used in calculation mapping application addresses
   /// to shadow addresses and origin addresses.
@@ -1121,7 +1121,7 @@
 
   BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", NewF);
   if (F->isVarArg()) {
-NewF->removeFnAttrs(AttrBuilder().addAttribute("split-stack"));
+NewF->removeFnAttrs(AttributeMask().addAttribute("split-stack"));
 CallInst::Create(DFSanVarargWrapperFn,
  IRBuilder<>(BB).CreateGlobalStringPtr(F->getName()), "",
  BB);
In

[clang] abd1cbf - [Clang] Disable debug-info-objname.cpp test on Unix until I sort out the issue.

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-12-21T10:32:43-05:00
New Revision: abd1cbf5e543f0f114d2742e109ead7d7ddbf9c4

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

LOG: [Clang] Disable debug-info-objname.cpp test on Unix until I sort out the 
issue.

Added: 


Modified: 
clang/test/CodeGenCXX/debug-info-objname.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/debug-info-objname.cpp 
b/clang/test/CodeGenCXX/debug-info-objname.cpp
index 07c0618c9a4d..078d2338bcff 100644
--- a/clang/test/CodeGenCXX/debug-info-objname.cpp
+++ b/clang/test/CodeGenCXX/debug-info-objname.cpp
@@ -1,3 +1,5 @@
+// REQUIRES: system-windows
+
 // RUN: cp %s %T/debug-info-objname.cpp
 // RUN: cd %T
 



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

[PATCH] D116110: Introduce the AttributeMask class

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: llvm/include/llvm/IR/Attributes.h:956
+assert(Attribute::isEnumAttrKind(Val) &&
+   "Adding integer/type attribute without an argument!");
+Attrs[Val] = true;

This assert doesn't make sense to me. AttributeMask doesn't accept arguments.



Comment at: llvm/include/llvm/IR/Attributes.h:993
+
+  bool empty() const { return Attrs.none(); }
+  bool hasAttributes() const { return TargetDepAttrs.empty() && Attrs.none(); }

Should this check TargetDepAttrs as well?



Comment at: llvm/include/llvm/IR/Attributes.h:994
+  bool empty() const { return Attrs.none(); }
+  bool hasAttributes() const { return TargetDepAttrs.empty() && Attrs.none(); }
+};

This looks inverted. Isn't this the implementation of empty()?



Comment at: llvm/include/llvm/IR/Attributes.h:1048
 
+  /// Remove the target-dependent attribute to the builder.
+  AttrBuilder &removeAttribute(Attribute A) {

to -> from


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

https://reviews.llvm.org/D116110

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

[clang] 07fe451 - [clang-format] Fix SplitEmptyRecord affecting SplitEmptyFunction.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2021-12-21T16:54:19+01:00
New Revision: 07fe45130546001632e1a6005922c58154e72fe9

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

LOG: [clang-format] Fix SplitEmptyRecord affecting SplitEmptyFunction.

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

Given the style:
```
BraceWrapping
  AfterFunction: true
 SplitEmptyFunction: true
 SplitEmptyRecord: false
...
```

The code that should be like:
```
void f(int ,
   int )
{
}
```

gets the braces merged together:
```
void f(int ,
   int )
{}
```

Reviewed By: MyDeveloperDay

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 8a86b85aca34..3d4c1a4f903b 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -393,11 +393,17 @@ class LineJoiner {
 
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
-  return !Style.BraceWrapping.AfterFunction ||
- (I[1]->First->is(tok::r_brace) &&
-  !Style.BraceWrapping.SplitEmptyRecord)
- ? tryMergeSimpleBlock(I, E, Limit)
- : 0;
+  bool ShouldMerge = false;
+  if (TheLine->First->isOneOf(tok::kw_class, tok::kw_struct)) {
+ShouldMerge = !Style.BraceWrapping.AfterClass ||
+  (I[1]->First->is(tok::r_brace) &&
+   !Style.BraceWrapping.SplitEmptyRecord);
+  } else {
+ShouldMerge = !Style.BraceWrapping.AfterFunction ||
+  (I[1]->First->is(tok::r_brace) &&
+   !Style.BraceWrapping.SplitEmptyFunction);
+  }
+  return ShouldMerge ? tryMergeSimpleBlock(I, E, Limit) : 0;
 }
 // Try to merge a function block with left brace wrapped
 if (I[1]->First->is(TT_FunctionLBrace) &&

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 8287188a9971..11b74580c317 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12200,6 +12200,38 @@ TEST_F(FormatTest, SplitEmptyFunction) {
"}",
Style);
 }
+
+TEST_F(FormatTest, SplitEmptyFunctionButNotRecord) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.SplitEmptyFunction = true;
+  Style.BraceWrapping.SplitEmptyRecord = false;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("class C {};", Style);
+  verifyFormat("struct C {};", Style);
+  verifyFormat("void f(int ,\n"
+   "   int )\n"
+   "{\n"
+   "}",
+   Style);
+  verifyFormat("class C {\n"
+   "  C()\n"
+   "  : (),\n"
+   "bbb()\n"
+   "  {\n"
+   "  }\n"
+   "  void\n"
+   "  m(int ,\n"
+   "int )\n"
+   "  {\n"
+   "  }\n"
+   "};",
+   Style);
+}
+
 TEST_F(FormatTest, KeepShortFunctionAfterPPElse) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;



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

[PATCH] D116049: [clang-format] Fix SplitEmptyRecord affecting SplitEmptyFunction.

[Marek Kurdej 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 rG07fe45130546: [clang-format] Fix SplitEmptyRecord affecting 
SplitEmptyFunction. (authored by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D116049?vs=395477&id=395683#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116049

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12200,6 +12200,38 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, SplitEmptyFunctionButNotRecord) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.SplitEmptyFunction = true;
+  Style.BraceWrapping.SplitEmptyRecord = false;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("class C {};", Style);
+  verifyFormat("struct C {};", Style);
+  verifyFormat("void f(int ,\n"
+   "   int )\n"
+   "{\n"
+   "}",
+   Style);
+  verifyFormat("class C {\n"
+   "  C()\n"
+   "  : (),\n"
+   "bbb()\n"
+   "  {\n"
+   "  }\n"
+   "  void\n"
+   "  m(int ,\n"
+   "int )\n"
+   "  {\n"
+   "  }\n"
+   "};",
+   Style);
+}
+
 TEST_F(FormatTest, KeepShortFunctionAfterPPElse) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -393,11 +393,17 @@
 
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
-  return !Style.BraceWrapping.AfterFunction ||
- (I[1]->First->is(tok::r_brace) &&
-  !Style.BraceWrapping.SplitEmptyRecord)
- ? tryMergeSimpleBlock(I, E, Limit)
- : 0;
+  bool ShouldMerge = false;
+  if (TheLine->First->isOneOf(tok::kw_class, tok::kw_struct)) {
+ShouldMerge = !Style.BraceWrapping.AfterClass ||
+  (I[1]->First->is(tok::r_brace) &&
+   !Style.BraceWrapping.SplitEmptyRecord);
+  } else {
+ShouldMerge = !Style.BraceWrapping.AfterFunction ||
+  (I[1]->First->is(tok::r_brace) &&
+   !Style.BraceWrapping.SplitEmptyFunction);
+  }
+  return ShouldMerge ? tryMergeSimpleBlock(I, E, Limit) : 0;
 }
 // Try to merge a function block with left brace wrapped
 if (I[1]->First->is(TT_FunctionLBrace) &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12200,6 +12200,38 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, SplitEmptyFunctionButNotRecord) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.SplitEmptyFunction = true;
+  Style.BraceWrapping.SplitEmptyRecord = false;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("class C {};", Style);
+  verifyFormat("struct C {};", Style);
+  verifyFormat("void f(int ,\n"
+   "   int )\n"
+   "{\n"
+   "}",
+   Style);
+  verifyFormat("class C {\n"
+   "  C()\n"
+   "  : (),\n"
+   "bbb()\n"
+   "  {\n"
+   "  }\n"
+   "  void\n"
+   "  m(int ,\n"
+   "int )\n"
+   "  {\n"
+   "  }\n"
+   "};",
+   Style);
+}
+
 TEST_F(FormatTest, KeepShortFunctionAfterPPElse) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===

[PATCH] D114966: [clang][deps] Split stat and file content caches

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:359-360
+  findSharedEntryByUID(llvm::vfs::Status Stat) const {
+return SharedCache.getShard(Stat.getName())
+.findEntryByUID(Stat.getUniqueID());
+  }

dexonsmith wrote:
> This doesn't look right to me. UIDs should be sharded independently of the 
> filename they happen to have been reached by; otherwise each filename shard 
> is developing its own idea of what each UID means. Since UID distribution is 
> not uniform, probably the UID shard should be chosen by 
> `hash_value(Stat.getUniqueID()) % NumShards`.
> 
> You could use the same sets of shards for UIDMap and FilenameMap, but since 
> they're independent I'd probably do:
> - UIDCache: sharded by UID: UIDMap and BumpPtrAllocator for entries (and 
> likely anything else tied to content)
> - FilenameCache: sharded by filename: FilenameMap (and perhaps other things 
> tied to filename?)
Hmm, skimming through `RealFileSystem::status`, I saw that it's calling 
`sys::fs::status` with "follow symlinks" enabled. It made sense to me that the 
name stored in `llvm::vfs::Status` would match that and refer the fully 
resolved target entry, not the symlink itself. Seeing as this is not the case, 
I agree the UID itself should be used for choosing the shard.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:23-24
+  auto MaybeFile = getUnderlyingFS().openFileForRead(Filename);
   if (!MaybeFile)
 return MaybeFile.getError();
   auto File = std::move(*MaybeFile);

dexonsmith wrote:
> In what circumstances should this return a cached-error TentativeEntry? Any?
I don't think the distinction matters at this level. Whether failures should be 
cached is a decision that's being made one level up.

I personally prefer having `TentativeEntry` to be "non-fallible" and explicitly 
wrapping the whole thing in `ErrorOr`. That makes it easier for others to know 
what they are working with (i.e. this object cannot represent an error state). 
Eventually, I think this would make sense for the caches and 
`CachedFileSystemEntry` too.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:28-29
   auto MaybeStat = File->status();
   if (!MaybeStat)
 return MaybeStat.getError();
   auto Stat = std::move(*MaybeStat);

dexonsmith wrote:
> Since the file was opened, should we return cached-error TentativeEntry here, 
> rather than an error?
Let's return an error here and create error `CachedFileSystemEntry` one level 
up.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:275
+  const auto &Entry1 = getOrEmplaceSharedEntryForUID(std::move(TEntry));
+  const auto &Entry2 = getOrInsertSharedEntryForFilename(Filename, Entry1);
+  return insertLocalEntryForFilename(Filename, Entry2);

dexonsmith wrote:
> jansvoboda11 wrote:
> > I'm not sure these should be separate. We could end up in situation where 
> > the Filename map contains different entry than the UID map for the same 
> > directory entry. I'm tempted to merge these functions into one and perform 
> > the updates in a single critical section...
> > I'm not sure these should be separate. We could end up in situation where 
> > the Filename map contains different entry than the UID map for the same 
> > directory entry.
> 
> I'm also sure precisely what you mean by "for the same directory entry" in 
> this context; and I don't see what's wrong with the situation I think you're 
> outlining.
> 
> > I'm tempted to merge these functions into one and perform the updates in a 
> > single critical section...
> 
> A single critical section for setting UID and filename at the same time would 
> be hard to get right (and efficient), since UIDs have aliases through other 
> filenames due to different directory paths (dir/../x.h vs x.h) and filesystem 
> links (hard and symbolic).
> 
> Here's the race that I think(?) you're worried about:
> 
> - Worker1 does a tentative stat of "x.h", finds a UID that isn't mapped 
> (UIDX1, but it's ignored...).
> - Worker2 does a tentative stat of "x.h", finds a UID that isn't mapped 
> (UIDX1, but it's ignored...).
> - Worker1 opens "x.h", finds ContentX1+StatX1 (with UIDX1), saves mapping 
> UIDX1 -> ContentX1+StatX1.
> - "x.h" changes.
> - Worker2 opens "x.h", finds ContentX2+StatX2 (with UIDX2), saves mapping 
> UIDX2 -> ContentX2+StatX2.
> - Worker2 saves mapping "x.h" -> ContentX2+StatX2.
> - Both workers move forward with "x.h" -> ContentX2+StatX2.
> 
> IIUC, you're concerned that the mapping UIDX1 -> ContentX1+StatX1 was saved. 
> The side effect is that if a future tentative stat of (e.g.) "y.h" returns 
> UIDX1, then "y.h" will be mapped to ContentX1+StatX1. Is this what concerns 
> you? Why? (Is it something else?)
> 
> The co

[PATCH] D114966: [clang][deps] Split stat and file content caches

[Jan Svoboda via Phabricator via cfe-commits]

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

Erase filenames in temporary `Stat` objects, use `UniqueID` as shard key where 
appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114966

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/unittests/Tooling/DependencyScannerTest.cpp

Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -224,6 +224,8 @@
   EXPECT_TRUE(StatusFull1);
   EXPECT_EQ(StatusMinimized0->getSize(), 17u);
   EXPECT_EQ(StatusFull1->getSize(), 30u);
+  EXPECT_EQ(StatusMinimized0->getName(), StringRef("/mod.h"));
+  EXPECT_EQ(StatusFull1->getName(), StringRef("/mod.h"));
 }
 
 TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately2) {
@@ -245,6 +247,8 @@
   EXPECT_TRUE(StatusMinimized1);
   EXPECT_EQ(StatusFull0->getSize(), 30u);
   EXPECT_EQ(StatusMinimized1->getSize(), 17u);
+  EXPECT_EQ(StatusFull0->getName(), StringRef("/mod.h"));
+  EXPECT_EQ(StatusMinimized1->getName(), StringRef("/mod.h"));
 }
 
 } // end namespace dependencies
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -16,10 +16,10 @@
 using namespace tooling;
 using namespace dependencies;
 
-llvm::ErrorOr
-CachedFileSystemEntry::initFile(StringRef Filename, llvm::vfs::FileSystem &FS) {
+llvm::ErrorOr
+DependencyScanningWorkerFilesystem::readFile(StringRef Filename) {
   // Load the file and its content from the file system.
-  auto MaybeFile = FS.openFileForRead(Filename);
+  auto MaybeFile = getUnderlyingFS().openFileForRead(Filename);
   if (!MaybeFile)
 return MaybeFile.getError();
   auto File = std::move(*MaybeFile);
@@ -34,24 +34,42 @@
 return MaybeBuffer.getError();
   auto Buffer = std::move(*MaybeBuffer);
 
-  OriginalContents = std::move(Buffer);
-  return Stat;
+  // If the file size changed between read and stat, pretend it didn't.
+  if (Stat.getSize() != Buffer->getBufferSize())
+Stat = llvm::vfs::Status::copyWithNewSize(Stat, Buffer->getBufferSize());
+
+  return TentativeEntry(Stat, std::move(Buffer));
 }
 
-void CachedFileSystemEntry::minimizeFile() {
-  assert(OriginalContents && "minimizing missing contents");
+EntryRef DependencyScanningWorkerFilesystem::minimizeIfNecessary(
+const CachedFileSystemEntry &Entry, StringRef Filename) {
+  if (Entry.isError() || Entry.isDirectory() || !shouldMinimize(Filename))
+return EntryRef(/*Minimized=*/false, Filename, Entry);
+
+  CachedFileContents *Contents = Entry.getContents();
+  assert(Contents && "contents not allocated");
+
+  // Double-checked locking.
+  if (Contents->MinimizedAccess.load())
+return EntryRef(/*Minimized=*/true, Filename, Entry);
+
+  std::lock_guard GuardLock(Contents->ValueLock);
+
+  // Double-checked locking.
+  if (Contents->MinimizedAccess.load())
+return EntryRef(/*Minimized=*/true, Filename, Entry);
 
   llvm::SmallString<1024> MinimizedFileContents;
   // Minimize the file down to directives that might affect the dependencies.
   SmallVector Tokens;
-  if (minimizeSourceToDependencyDirectives(OriginalContents->getBuffer(),
+  if (minimizeSourceToDependencyDirectives(Contents->Original->getBuffer(),
MinimizedFileContents, Tokens)) {
 // FIXME: Propagate the diagnostic if desired by the client.
 // Use the original file if the minimization failed.
-MinimizedContentsStorage =
-llvm::MemoryBuffer::getMemBuffer(*OriginalContents);
-MinimizedContentsAccess.store(MinimizedContentsStorage.get());
-return;
+Contents->MinimizedStorage =
+llvm::MemoryBuffer::getMemBuffer(*Contents->Original);
+Contents->MinimizedAccess.store(Contents->MinimizedStorage.get());
+return EntryRef(/*Minimized=*/true, Filename, Entry);
   }
 
   // The contents produced by the minimizer must be null terminated.
@@ -74,16 +92,17 @@
 }
 Mapping[Range.Offset] = Range.Length;
   }
-  PPSkippedRangeMapping = std::move(Mapping);
+  Contents->PPSkippedRangeMapping = std::move(Mapping);
 
-  MinimizedContentsStorage = std::make_unique(
+  Contents->MinimizedStorage = std::make_unique(
   std::move(MinimizedFileContents));
-  // The algorithm in `getOrCreateFileSystemEntry` uses the presence of
-  // minimized contents to decide whether an entry is up-to-date or not.
-  // If it is up-to-date, the skipped range mappings must be already computed.
-  // This is why we nee

[PATCH] D116110: Introduce the AttributeMask class

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Pre-merge checks also report a build failure in 
llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp:360.


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

https://reviews.llvm.org/D116110

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D109885#3202213 , @JonChesterfield 
wrote:

> In D109885#3198375 , @arsenm wrote:
>
>> In D109885#3198340 , 
>> @JonChesterfield wrote:
>>
>>> I'm very mistrusting of mixing a rocm toolchain with a trunk toolchain as 
>>> they're both quite keen on runpath and LD_LIBRARY_PATH to find internal 
>>> components. That makes it very easy to accidentally mix the two together 
>>> into something that doesn't work so personal preference is to rip out the 
>>> /opt/rocm search path for HSA entirely and encourage people to build it 
>>> from source.
>>
>> There are a number of cmake crimes going on in both of these, but this isn't 
>> one of them. LD_LIBRARY_PATH should not be used for any builds to find 
>> components.
>
> The LD_LIBRARY_PATH hazard isn't at compiler build time, it's at application 
> run time. E.g. someone points LD_LIBRARY_PATH at a rocm install to get libhsa 
> and gets that plus some rocm-distributed llvm libraries which don't 
> necessarily match trunk. That's a different hazard to this particular one, 
> except that cmake going in search of /opt/rocm suggests that arbitrary 
> combinations of rocm and trunk will work together when it's closer to zero 
> combinations work.

We should have versioned libraries and well defined ABI breaks. The build 
should know what runtime versions it requires and account when searching


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D109885#3203412 , @arsenm wrote:

> We should have versioned libraries and well defined ABI breaks. The build 
> should know what runtime versions it requires and account when searching

If we're playing that game, ROCm should be strictly an extension of upstream 
LLVM, such that code which accidentally or intentionally picks up parts of ROCm 
while expecting parts of trunk runs correctly. I'd be happy with the easier 
objective of making it difficult to accidentally splice together parts of 
different toolchains at runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D109885#3203444 , @JonChesterfield 
wrote:

> In D109885#3203412 , @arsenm wrote:
>
>> We should have versioned libraries and well defined ABI breaks. The build 
>> should know what runtime versions it requires and account when searching
>
> If we're playing that game, ROCm should be strictly an extension of upstream 
> LLVM, such that code which accidentally or intentionally picks up parts of 
> ROCm while expecting parts of trunk runs correctly. I'd be happy with the 
> easier objective of making it difficult to accidentally splice together parts 
> of different toolchains at runtime.

ROCm has as much to do with llvm as the linux kernel does. The compiler and 
runtime version should not be so tightly linked / the runtime so unstable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

[Alexandre Ganea 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 rGf44e3fbadd15: [CodeView] Emit S_OBJNAME record (authored by 
aganea).

Changed prior to commit:
  https://reviews.llvm.org/D43002?vs=374884&id=395663#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D43002

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Job.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CMakeLists.txt
  clang/test/CodeGenCXX/debug-info-objname.cpp
  llvm/include/llvm/MC/MCTargetOptions.h
  llvm/include/llvm/Support/Caching.h
  llvm/include/llvm/Support/ToolOutputFile.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Support/Caching.cpp
  llvm/test/DebugInfo/COFF/globals.ll
  llvm/test/DebugInfo/COFF/multifunction.ll
  llvm/test/DebugInfo/COFF/pr28747.ll
  llvm/test/DebugInfo/COFF/simple.ll
  llvm/test/DebugInfo/COFF/vframe-fpo.ll
  llvm/test/MC/AArch64/coff-debug.ll
  llvm/test/MC/ARM/coff-debugging-secrel.ll
  llvm/test/MC/COFF/cv-compiler-info.ll
  llvm/tools/llc/llc.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -378,7 +378,7 @@
 std::error_code EC;
 auto S = std::make_unique(Path, EC, sys::fs::OF_None);
 check(EC, Path);
-return std::make_unique(std::move(S));
+return std::make_unique(std::move(S), Path);
   };
 
   auto AddBuffer = [&](size_t Task, std::unique_ptr MB) {
Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -605,6 +605,9 @@
   GetOutputStream(TheTarget->getName(), TheTriple.getOS(), argv[0]);
   if (!Out) return 1;
 
+  // Ensure the filename is passed down to CodeViewDebug.
+  Target->Options.ObjectFilenameForDebug = Out->outputFilename();
+
   std::unique_ptr DwoOut;
   if (!SplitDwarfOutputFile.empty()) {
 std::error_code EC;
Index: llvm/test/MC/COFF/cv-compiler-info.ll
===
--- llvm/test/MC/COFF/cv-compiler-info.ll
+++ llvm/test/MC/COFF/cv-compiler-info.ll
@@ -1,4 +1,6 @@
-; RUN: llc -mtriple i686-pc-windows-msvc < %s | FileCheck %s
+; RUN: llc -mtriple i686-pc-windows-msvc < %s | FileCheck %s --check-prefixes=CHECK,STDOUT
+; RUN: llc -mtriple i686-pc-windows-msvc < %s -o %t
+; RUN: FileCheck %s --input-file=%t --check-prefixes=CHECK,FILE
 ; ModuleID = 'D:\src\scopes\foo.cpp'
 source_filename = "D:\5Csrc\5Cscopes\5Cfoo.cpp"
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
@@ -20,19 +22,23 @@
 ; One .debug$S section should contain an S_COMPILE3 record that identifies the
 ; source language and the version of the compiler based on the DICompileUnit.
 ; CHECK: 	.section	.debug$S,"dr"
+; CHECK:.short  4353# Record kind: S_OBJNAME
+; CHECK-NEXT:   .long   0   # Signature
+; STDOUT-NEXT:  .byte   0   # Object name
+; FILE-NEXT:.asciz	"{{.*}}{{|/}}cv-compiler-info.ll.tmp" # Object name
 ; CHECK: 		.short	4412  # Record kind: S_COMPILE3
-; CHECK: 		.long	1   # Flags and language
-; CHECK: 		.short	7 # CPUType
-; CHECK: 		.short	4 # Frontend version
-; CHECK: 		.short	0
-; CHECK: 		.short	0
-; CHECK: 		.short	0
-; CHECK: 		.short	[[BACKEND_VERSION:[0-9]+]]  # Backend version
-; CHECK: 		.short	0
-; CHECK: 		.short	0
-; CHECK: 		.short	0
-; CHECK: 		.asciz	"clang version 4.0.0 "  # Null-terminated compiler version string
-; CHECK-NOT: .short	4412  # Record kind: S_COMPILE3
+; CHECK-NEXT:   .long	1   # Flags and language
+; CHECK-NEXT: 	.short	7 # CPUType
+; CHECK-NEXT: 	.short	4 # Frontend version
+; CHECK-NEXT: 	.short	0
+; CHECK-NEXT: 	.short	0
+; CHECK-NEXT: 	.short	0
+; CHECK-NEXT: 	.short	[[BACKEND_VERSION:[0-9]+]]  # Backend version
+; CHECK-NEXT: 	.short	0
+; CHECK-NEXT: 	.short	0
+; CHECK-NEXT: 	.short	0
+; CHECK-NEXT: 	.asciz	"clang version 4.0.0 "  # Null-terminated compiler version string
+; CHECK-NOT:.short	4412  # Record kind: S_COMPILE3
 !1 = !DIFile(filename: "D:\5Csrc\5Cscopes\5Cfoo.cpp", directory: "D:\5Csrc\5Cscopes\5Cclang")
 !2 = !{}
 !7 = !{i32 2, !"CodeView", i32 1}
Index: llvm/test/MC/ARM/coff-debugging-secrel.ll
===
--- llvm/test/MC/ARM/coff-debugging-secrel.ll
+++ llvm/test/MC/A

[clang] 36ea986 - [clang-format] Remove unnecessary qualifications. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2021-12-21T17:02:26+01:00
New Revision: 36ea9861e3b58a715434cc2ec5ee2ebf7053bbc9

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

LOG: [clang-format] Remove unnecessary qualifications. NFC.

Added: 


Modified: 
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 11b74580c317..eeb2fd04aae3 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -262,8 +262,8 @@ TEST_F(FormatTest, RemovesEmptyLines) {
"}",
getGoogleStyle()));
 
-  auto CustomStyle = clang::format::getLLVMStyle();
-  CustomStyle.BreakBeforeBraces = clang::format::FormatStyle::BS_Custom;
+  auto CustomStyle = getLLVMStyle();
+  CustomStyle.BreakBeforeBraces = FormatStyle::BS_Custom;
   CustomStyle.BraceWrapping.AfterNamespace = true;
   CustomStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   EXPECT_EQ("namespace N\n"



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

[clang-tools-extra] 1a92952 - [clangd] Return error for textdocument/outgoingCalls rather than success

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-12-21T17:06:59+01:00
New Revision: 1a929525e86a20d0b3455a400d0dbed40b325a13

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

LOG: [clangd] Return error for textdocument/outgoingCalls rather than success

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 93382466160c..18539877ec97 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1208,13 +1208,6 @@ void ClangdLSPServer::onCallHierarchyIncomingCalls(
   Server->incomingCalls(Params.item, std::move(Reply));
 }
 
-void ClangdLSPServer::onCallHierarchyOutgoingCalls(
-const CallHierarchyOutgoingCallsParams &Params,
-Callback> Reply) {
-  // FIXME: To be implemented.
-  Reply(std::vector{});
-}
-
 void ClangdLSPServer::onInlayHints(const InlayHintsParams &Params,
Callback> Reply) {
   Server->inlayHints(Params.textDocument.uri.file(), std::move(Reply));
@@ -1478,7 +1471,6 @@ void ClangdLSPServer::bindMethods(LSPBinder &Bind,
   Bind.method("typeHierarchy/resolve", this, 
&ClangdLSPServer::onResolveTypeHierarchy);
   Bind.method("textDocument/prepareCallHierarchy", this, 
&ClangdLSPServer::onPrepareCallHierarchy);
   Bind.method("callHierarchy/incomingCalls", this, 
&ClangdLSPServer::onCallHierarchyIncomingCalls);
-  Bind.method("callHierarchy/outgoingCalls", this, 
&ClangdLSPServer::onCallHierarchyOutgoingCalls);
   Bind.method("textDocument/selectionRange", this, 
&ClangdLSPServer::onSelectionRange);
   Bind.method("textDocument/documentLink", this, 
&ClangdLSPServer::onDocumentLink);
   Bind.method("textDocument/semanticTokens/full", this, 
&ClangdLSPServer::onSemanticTokens);



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

[PATCH] D112073: [PowerPC] Emit warning when SP is clobbered by asm

[Quinn Pham via Phabricator via cfe-commits]

quinnp updated this revision to Diff 395687.
quinnp added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adressing review comments. Added register alias "sp" for the register "r1".
Added a front end testcase to test the location information and all of the
aliases for "r1". Merged the back end test cases so that we are testing both
"r1" and "x1" in 32bit and 64bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112073

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Misc/ppc-inline-asm-clobber-warning.c
  llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
  llvm/lib/Target/PowerPC/PPCRegisterInfo.h
  llvm/test/CodeGen/PowerPC/inline-asm-clobber-warning.ll

Index: llvm/test/CodeGen/PowerPC/inline-asm-clobber-warning.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/inline-asm-clobber-warning.ll
@@ -0,0 +1,22 @@
+; RUN: llc < %s -verify-machineinstrs -mtriple=powerpc-unknown-unkown \
+; RUN:   -mcpu=pwr7 2>&1 | FileCheck %s
+; RUN: llc < %s -verify-machineinstrs -mtriple=powerpc64-unknown-unkown \
+; RUN:   -mcpu=pwr7 2>&1 | FileCheck %s
+
+define void @test_r1_clobber() {
+entry:
+  call void asm sideeffect "nop", "~{r1}"()
+  ret void
+}
+
+; CHECK: warning: inline asm clobber list contains reserved registers: R1
+; CHECK-NEXT: note: Reserved registers on the clobber list may not be preserved across the asm statement, and clobbering them may lead to undefined behaviour.
+
+define void @test_x1_clobber() {
+entry:
+  call void asm sideeffect "nop", "~{x1}"()
+  ret void
+}
+
+; CHECK: warning: inline asm clobber list contains reserved registers: X1
+; CHECK-NEXT: note: Reserved registers on the clobber list may not be preserved across the asm statement, and clobbering them may lead to undefined behaviour.
Index: llvm/lib/Target/PowerPC/PPCRegisterInfo.h
===
--- llvm/lib/Target/PowerPC/PPCRegisterInfo.h
+++ llvm/lib/Target/PowerPC/PPCRegisterInfo.h
@@ -91,6 +91,8 @@
   void adjustStackMapLiveOutMask(uint32_t *Mask) const override;
 
   BitVector getReservedRegs(const MachineFunction &MF) const override;
+  bool isAsmClobberable(const MachineFunction &MF,
+MCRegister PhysReg) const override;
   bool isCallerPreservedPhysReg(MCRegister PhysReg,
 const MachineFunction &MF) const override;
 
Index: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
===
--- llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
+++ llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
@@ -390,6 +390,18 @@
   return Reserved;
 }
 
+bool PPCRegisterInfo::isAsmClobberable(const MachineFunction &MF,
+   MCRegister PhysReg) const {
+  // We cannot use getReservedRegs() to find the registers that are not asm
+  // clobberable because there are some reserved registers which can be
+  // clobbered by inline asm. For example, when LR is clobbered, the register is
+  // saved and restored. We will hardcode the registers that are not asm
+  // cloberable in this function.
+
+  // The stack pointer (R1/X1) is not clobberable by inline asm
+  return PhysReg != PPC::R1 && PhysReg != PPC::X1;
+}
+
 bool PPCRegisterInfo::requiresFrameIndexScavenging(const MachineFunction &MF) const {
   const PPCSubtarget &Subtarget = MF.getSubtarget();
   const PPCInstrInfo *InstrInfo =  Subtarget.getInstrInfo();
Index: clang/test/Misc/ppc-inline-asm-clobber-warning.c
===
--- /dev/null
+++ clang/test/Misc/ppc-inline-asm-clobber-warning.c
@@ -0,0 +1,38 @@
+/// This test checks that the warning includes the location in the C source
+/// file that contains the inline asm. Although this warning is emitted in llvm
+/// it cannot be tested from IR as it does not have that location information at
+/// that stage.
+
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang --target=powerpc-unknown-unknown -mcpu=pwr7 \
+// RUN:   -c %s -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang --target=powerpc64-unknown-unknown -mcpu=pwr7 \
+// RUN:   -c %s -o /dev/null 2>&1 | FileCheck %s
+
+void test_r1_clobber() {
+  __asm__("nop":::"r1");
+}
+
+// CHECK:  ppc-inline-asm-clobber-warning.c:14:11: warning: inline asm clobber list contains reserved registers: R1 [-Winline-asm]
+// CHECK-NEXT:   __asm__("nop":::"r1");
+// CHECK-NEXT:   ^
+// CHECK-NEXT: ppc-inline-asm-clobber-warning.c:14:11: note: Reserved registers on the clobber list may not be preserved across the asm statement, and clobbering them may lead to undefined behaviour.
+
+void test_1_clobber() {
+  __asm__("nop":::"1");
+}
+
+// CHECK:  ppc-inline-asm-clobber-warning.c:23:11: warning: inline asm clobber list contains reserved registers: R1 [-Winline-asm]
+// CHECK-N

[PATCH] D116113: Add LLDB synthetic child and summary scripts for llvm::SmallVector, llvm::Optional, llvm::ErrorOr and llvm::Expected.

[Greg Clayton via Phabricator via cfe-commits]

clayborg created this revision.
clayborg added reviewers: labath, teemperor, aprantl.
Herald added subscribers: usaxena95, arphaman.
clayborg requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This allows us to see the contents of the SmallVector just like std::vector and 
also adds a summary string that shows the size of the array.

For llvm::Optional, the value will show as llvm::None when it has no value, or 
it will show the value of the type T.

For llvm::Error or, it will show the std::error_code as a structure or the 
value if it has a value.

For llvm::Expected, it will show the dynamic classname of the contained error, 
or the value.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116113

Files:
  clang/utils/ClangDataFormat.py

Index: clang/utils/ClangDataFormat.py
===
--- clang/utils/ClangDataFormat.py
+++ clang/utils/ClangDataFormat.py
@@ -22,140 +22,408 @@
 import lldb
 
 def __lldb_init_module(debugger, internal_dict):
-	debugger.HandleCommand("type summary add -F ClangDataFormat.SourceLocation_summary clang::SourceLocation")
-	debugger.HandleCommand("type summary add -F ClangDataFormat.QualType_summary clang::QualType")
-	debugger.HandleCommand("type summary add -F ClangDataFormat.StringRef_summary llvm::StringRef")
+debugger.HandleCommand("type summary add -F ClangDataFormat.SourceLocation_summary clang::SourceLocation")
+debugger.HandleCommand("type summary add -F ClangDataFormat.QualType_summary clang::QualType")
+debugger.HandleCommand("type summary add -F ClangDataFormat.StringRef_summary llvm::StringRef")
+debugger.HandleCommand("type summary add -F ClangDataFormat.Optional_summary -x 'llvm::Optional<.*>'")
+debugger.HandleCommand("type summary add -F ClangDataFormat.SmallVector_summary -x 'llvm::SmallVector<.*>'")
+debugger.HandleCommand("type summary add -F ClangDataFormat.Expected_summary -x 'llvm::Expected<.*>'")
+debugger.HandleCommand("type summary add -F ClangDataFormat.ErrorOr_summary -x 'llvm::ErrorOr<.*>'")
+debugger.HandleCommand("type synthetic add -l ClangDataFormat.Optional -x 'llvm::Optional<.*>'")
+debugger.HandleCommand("type synthetic add -l ClangDataFormat.SmallVector -x 'llvm::SmallVector<.*>'")
+debugger.HandleCommand("type synthetic add -l ClangDataFormat.Expected -x 'llvm::Expected<.*>'")
+debugger.HandleCommand("type synthetic add -l ClangDataFormat.ErrorOr -x 'llvm::ErrorOr<.*>'")
 
-def SourceLocation_summary(srcloc, internal_dict):
-	return SourceLocation(srcloc).summary()
+def SourceLocation_summary(valobj, internal_dict):
+return SourceLocation(valobj).summary()
 
-def QualType_summary(qualty, internal_dict):
-	return QualType(qualty).summary()
+def QualType_summary(valobj, internal_dict):
+return QualType(valobj).summary()
 
-def StringRef_summary(strref, internal_dict):
-	return StringRef(strref).summary()
+def StringRef_summary(valobj, internal_dict):
+return StringRef(valobj).summary()
+
+def Optional_summary(valobj, internal_dict):
+return Optional(valobj, internal_dict).summary()
+
+def SmallVector_summary(valobj, internal_dict):
+return SmallVector(valobj, internal_dict).summary()
+
+def Expected_summary(valobj, internal_dict):
+return Expected(valobj, internal_dict).summary()
+
+def ErrorOr_summary(valobj, internal_dict):
+return ErrorOr(valobj, internal_dict).summary()
 
 class SourceLocation(object):
-	def __init__(self, srcloc):
-		self.srcloc = srcloc
-		self.ID = srcloc.GetChildAtIndex(0).GetValueAsUnsigned()
-		self.frame = srcloc.GetFrame()
-	
-	def offset(self):
-		return getValueFromExpression(self.srcloc, ".getOffset()").GetValueAsUnsigned()
-
-	def isInvalid(self):
-		return self.ID == 0
-
-	def isMacro(self):
-		return getValueFromExpression(self.srcloc, ".isMacroID()").GetValueAsUnsigned()
-
-	def isLocal(self, srcmgr_path):
-		return self.frame.EvaluateExpression("(%s).isLocalSourceLocation(%s)" % (srcmgr_path, getExpressionPath(self.srcloc))).GetValueAsUnsigned()
-
-	def getPrint(self, srcmgr_path):
-		print_str = getValueFromExpression(self.srcloc, ".printToString(%s)" % srcmgr_path)
-		return print_str.GetSummary()
-
-	def summary(self):
-		if self.isInvalid():
-			return ""
-		srcmgr_path = findObjectExpressionPath("clang::SourceManager", self.frame)
-		if srcmgr_path:
-			return "%s (offset: %d, %s, %s)" % (self.getPrint(srcmgr_path), self.offset(), "macro" if self.isMacro() else "file", "local" if self.isLocal(srcmgr_path) else "loaded")
-		return "(offset: %d, %s)" % (self.offset(), "macro" if self.isMacro() else "file")
+def __init__(self, srcloc):
+self.srcloc = srcloc
+self.ID = srcloc.GetChildAtIndex(0).GetValueAsUnsigned()
+self.frame = srcloc.GetFrame()
+
+def offset(self):
+return getValueFromExpression(self.srcloc, ".getOffset()").GetValueAsUnsigned()
+
+def isInvalid(se

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

[David Rector via Phabricator via cfe-commits]

davrec added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp:372
+}
+
 /// Determines whether two statement trees are identical regarding

davrec wrote:
> I would strongly support turning these functions into methods of DeclRefExpr 
> and NestedNameSpecifier, not just to avoid the code duplication here but 
> because they sure seem to be of general utility to AST users.
> 
> You just need to clearly motivate these methods in the documentation, which 
> your description for this patch largely does, e.g. something like.:
> 
> ```
> class DeclRefExpr : ... {
>   ...
>   /// Returns whether this \c DeclRefExpr 
>   ///   a. refers to the same canonical declaration as \c other and
>   ///   b. if it has a qualifier, that qualifier refers to the same canonical 
>   ///   declarations as those of \c other .
>   ///
>   /// Consider these two DeclRefExprs:
>   /// \code
>   ///   std::is_same::value
>   ///   std::is_same::value;
>   /// \endcode
>   ///
>   /// We can see that the value static members are different, but in fact
>   /// the \c getDecl() of these two returns the same VarDecl. (...etc)
>   bool isSemanticallyEquivalentTo(const DeclRefExpr *Other) { ... }
> };
> ...
> class NestedNameSpecifier {
>   ...
>   /// Returns whether this refers to the same sequence of 
> identifiers/canonical declarations
>   /// as \c Other.  (Then I would repeat the std::is_same example here, since 
> that
>   /// really makes the issue clear.  And also describe what this returns when 
> other is nullptr or
>   /// when getPrefix() is nullptr while other->getPrefix() is non-null -- 
> maybe add a parameter if
>   /// its not clear in general what the behavior should be.)
>   bool isSemanticallyEquivalentTo(const NestedNameSpecifier *Other, bool 
> RecurseToPrefix = true) { ... }
> };
> 
> ```
Also if doing this it might be nice to add `isSyntacticallyEquivalentTo(other)` 
methods alongside the semantic versions, defined the same except they don't 
desugar the type nor get the underlying NamespaceDecl from a 
NamespaceAliasDecl.  This would be simple and, given that you found that two 
NestedNameSpecifiers representing the exact same syntax are nonetheless 
different pointers, it could be very useful for AST users concerned with syntax 
rather than semantics.

(Btw note the DeclRefExpr's syntactic method should still use 
getDecl()->getCanonicalDecl(), since, confusingly, getCanonicalDecl simply 
fetches the FirstDecl from the various redeclarations, rather than doing any 
kind of desugaring as getCanonicalType does.  And fortunately I don't think it 
is necessary to manually "desugar" the canonical decl in the semantic version, 
since a DeclRefExpr's getDecl is always a ValueDecl, which I don't think can 
ever be "sugar" for some underlying decl, as say a NamespaceAliasDecl is for a 
NamespaceDecl -- so that part should be fine.)



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

https://reviews.llvm.org/D114622

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

[PATCH] D116110: Introduce the AttributeMask class

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 395692.
serge-sans-paille added a comment.
Herald added subscribers: foad, kerbowa, nhaehnle, jvesely, arsenm.

remvove useless assert, update comment, fix build


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

https://reviews.llvm.org/D116110

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/IR/Argument.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Scalar/SCCP.cpp
  llvm/unittests/IR/AttributesTest.cpp

Index: llvm/unittests/IR/AttributesTest.cpp
===
--- llvm/unittests/IR/AttributesTest.cpp
+++ llvm/unittests/IR/AttributesTest.cpp
@@ -81,12 +81,12 @@
   AttrBuilder B_align_readonly;
   B_align_readonly.addAttribute(AlignAttr);
   B_align_readonly.addAttribute(Attribute::ReadOnly);
-  AttrBuilder B_align;
+  AttributeMask B_align;
   B_align.addAttribute(AlignAttr);
   AttrBuilder B_stackalign_optnone;
   B_stackalign_optnone.addAttribute(StackAlignAttr);
   B_stackalign_optnone.addAttribute(Attribute::OptimizeNone);
-  AttrBuilder B_stackalign;
+  AttributeMask B_stackalign;
   B_stackalign.addAttribute(StackAlignAttr);
 
   AttributeSet AS = AttributeSet::get(C, B_align_readonly);
Index: llvm/lib/Transforms/Scalar/SCCP.cpp
===
--- llvm/lib/Transforms/Scalar/SCCP.cpp
+++ llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -486,7 +486,7 @@
   // inaccessiblemem_or_argmemonly attributes do not hold any longer. Remove
   // them from both the function and callsites.
   if (ReplacedPointerArg) {
-AttrBuilder AttributesToRemove;
+AttributeMask AttributesToRemove;
 AttributesToRemove.addAttribute(Attribute::ArgMemOnly);
 AttributesToRemove.addAttribute(Attribute::InaccessibleMemOrArgMemOnly);
 F.removeFnAttrs(AttributesToRemove);
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1383,7 +1383,7 @@
 
   for (Attribute A : AL.getFnAttrs()) {
 if (isStatepointDirectiveAttr(A))
-  FnAttrs.remove(A);
+  FnAttrs.removeAttribute(A);
   }
 
   // Just skip parameter and return attributes for now
@@ -2654,7 +2654,7 @@
 template 
 static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH,
   unsigned Index) {
-  AttrBuilder R;
+  AttributeMask R;
   AttributeSet AS = AH.getAttributes().getAttributes(Index);
   if (AS.getDereferenceableBytes())
 R.addAttribute(Attribute::get(Ctx, Attribute::Dereferenceable,
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3664,7 +3664,7 @@
   // will become a non-readonly function after it is instrumented by us. To
   // prevent this code from being optimized out, mark that function
   // non-readonly in advance.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
@@ -5359,7 +5359,7 @@
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -454,7 +454,7 @@
   MDNode *OriginStoreWeights;
   DFSanABIList ABIList;
   DenseMap UnwrappedFnMap;
-  AttrBuilder ReadOnlyNoneAttrs;
+  AttributeMask ReadOnlyNoneAttrs;
 
   /// Memory map parameters used in calculation mapping application addresses
   /// to shadow addresses and origin addresses.
@@ -1121,7 +1121,7 @@
 
   BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", NewF);
   if (F->isVarArg()) {
-NewF->removeFnAttrs(AttrBuilder().addAttribute("split-stack"));
+NewF->removeFnAttrs(AttributeMask().addAttribute("split-

[PATCH] D116110: Introduce the AttributeMask class

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 395694.

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

https://reviews.llvm.org/D116110

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/IR/Argument.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Scalar/SCCP.cpp
  llvm/unittests/IR/AttributesTest.cpp

Index: llvm/unittests/IR/AttributesTest.cpp
===
--- llvm/unittests/IR/AttributesTest.cpp
+++ llvm/unittests/IR/AttributesTest.cpp
@@ -81,12 +81,12 @@
   AttrBuilder B_align_readonly;
   B_align_readonly.addAttribute(AlignAttr);
   B_align_readonly.addAttribute(Attribute::ReadOnly);
-  AttrBuilder B_align;
+  AttributeMask B_align;
   B_align.addAttribute(AlignAttr);
   AttrBuilder B_stackalign_optnone;
   B_stackalign_optnone.addAttribute(StackAlignAttr);
   B_stackalign_optnone.addAttribute(Attribute::OptimizeNone);
-  AttrBuilder B_stackalign;
+  AttributeMask B_stackalign;
   B_stackalign.addAttribute(StackAlignAttr);
 
   AttributeSet AS = AttributeSet::get(C, B_align_readonly);
Index: llvm/lib/Transforms/Scalar/SCCP.cpp
===
--- llvm/lib/Transforms/Scalar/SCCP.cpp
+++ llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -486,7 +486,7 @@
   // inaccessiblemem_or_argmemonly attributes do not hold any longer. Remove
   // them from both the function and callsites.
   if (ReplacedPointerArg) {
-AttrBuilder AttributesToRemove;
+AttributeMask AttributesToRemove;
 AttributesToRemove.addAttribute(Attribute::ArgMemOnly);
 AttributesToRemove.addAttribute(Attribute::InaccessibleMemOrArgMemOnly);
 F.removeFnAttrs(AttributesToRemove);
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1383,7 +1383,7 @@
 
   for (Attribute A : AL.getFnAttrs()) {
 if (isStatepointDirectiveAttr(A))
-  FnAttrs.remove(A);
+  FnAttrs.removeAttribute(A);
   }
 
   // Just skip parameter and return attributes for now
@@ -2654,7 +2654,7 @@
 template 
 static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH,
   unsigned Index) {
-  AttrBuilder R;
+  AttributeMask R;
   AttributeSet AS = AH.getAttributes().getAttributes(Index);
   if (AS.getDereferenceableBytes())
 R.addAttribute(Attribute::get(Ctx, Attribute::Dereferenceable,
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3664,7 +3664,7 @@
   // will become a non-readonly function after it is instrumented by us. To
   // prevent this code from being optimized out, mark that function
   // non-readonly in advance.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
@@ -5359,7 +5359,7 @@
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -454,7 +454,7 @@
   MDNode *OriginStoreWeights;
   DFSanABIList ABIList;
   DenseMap UnwrappedFnMap;
-  AttrBuilder ReadOnlyNoneAttrs;
+  AttributeMask ReadOnlyNoneAttrs;
 
   /// Memory map parameters used in calculation mapping application addresses
   /// to shadow addresses and origin addresses.
@@ -1121,7 +1121,7 @@
 
   BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", NewF);
   if (F->isVarArg()) {
-NewF->removeFnAttrs(AttrBuilder().addAttribute("split-stack"));
+NewF->removeFnAttrs(AttributeMask().addAttribute("split-stack"));
 CallInst::Create(DFSanVarargWrapperFn,
  IRBuilder<>(BB).CreateGlobalStringPtr(F->getName()), "",
  

[PATCH] D116110: Introduce the AttributeMask class

[serge via Phabricator via cfe-commits]

serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added inline comments.



Comment at: llvm/include/llvm/IR/Attributes.h:993
+
+  bool empty() const { return Attrs.none(); }
+  bool hasAttributes() const { return TargetDepAttrs.empty() && Attrs.none(); }

nikic wrote:
> Should this check TargetDepAttrs as well?
I matched the implementation in AttrBuilder, which looks odd to me too.


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

https://reviews.llvm.org/D116110

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

[clang] 450dddd - [clang-format] Remove unnecessary qualifications. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2021-12-21T17:53:42+01:00
New Revision: 450cb769b680cd3547877f5d64d40cde16ba

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

LOG: [clang-format] Remove unnecessary qualifications. NFC.

Added: 


Modified: 
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index eeb2fd04aae3..a787e26ccd5a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -21590,7 +21590,7 @@ TEST_F(ReplacementTest, FormatCodeAfterReplacements) {
tooling::Replacement(Context.Sources, Context.getLocation(ID, 4, 13), 1,
 "nullptr")});
 
-  format::FormatStyle Style = format::getLLVMStyle();
+  FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
   auto FormattedReplaces = formatReplacements(Code, Replaces, Style);
   EXPECT_TRUE(static_cast(FormattedReplaces))
@@ -21619,7 +21619,7 @@ TEST_F(ReplacementTest, SortIncludesAfterReplacement) {
   {tooling::Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 0,
 "#include \"b.h\"\n")});
 
-  format::FormatStyle Style = format::getLLVMStyle();
+  FormatStyle Style = getLLVMStyle();
   Style.SortIncludes = FormatStyle::SI_CaseSensitive;
   auto FormattedReplaces = formatReplacements(Code, Replaces, Style);
   EXPECT_TRUE(static_cast(FormattedReplaces))
@@ -21638,7 +21638,7 @@ TEST_F(FormatTest, FormatSortsUsingDeclarations) {
 }
 
 TEST_F(FormatTest, UTF8CharacterLiteralCpp03) {
-  format::FormatStyle Style = format::getLLVMStyle();
+  FormatStyle Style = getLLVMStyle();
   Style.Standard = FormatStyle::LS_Cpp03;
   // cpp03 recognize this string as identifier u8 and literal character 'a'
   EXPECT_EQ("auto c = u8 'a';", format("auto c = u8'a';", Style));
@@ -21718,7 +21718,7 @@ TEST_F(FormatTest, StructuredBindings) {
   EXPECT_EQ("auto const &&[x, y]{expr};",
 format("auto  const  &&  [x,y]  {expr};"));
 
-  format::FormatStyle Spaces = format::getLLVMStyle();
+  FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInSquareBrackets = true;
   verifyFormat("auto [ a, b ] = f();", Spaces);
   verifyFormat("auto &&[ a, b ] = f();", Spaces);



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

[clang] 00ec441 - [Clang] debug-info-objname.cpp test: explictly encode a x86 target when using %clang_cl to avoid falling back to a native CPU triple.

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-12-21T11:54:19-05:00
New Revision: 00ec441253048f5e30540ea26bb0a28c42a5fc18

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

LOG: [Clang] debug-info-objname.cpp test: explictly encode a x86 target when 
using %clang_cl to avoid falling back to a native CPU triple.

Added: 


Modified: 
clang/test/CodeGenCXX/debug-info-objname.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/debug-info-objname.cpp 
b/clang/test/CodeGenCXX/debug-info-objname.cpp
index 078d2338bcff7..1a9a6cfa2081e 100644
--- a/clang/test/CodeGenCXX/debug-info-objname.cpp
+++ b/clang/test/CodeGenCXX/debug-info-objname.cpp
@@ -1,34 +1,32 @@
-// REQUIRES: system-windows
-
 // RUN: cp %s %T/debug-info-objname.cpp
 // RUN: cd %T
 
 // No output file provided, input file is relative, we emit an absolute path 
(MSVC behavior).
-// RUN: %clang_cl /c /Z7 -nostdinc debug-info-objname.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -nostdinc 
debug-info-objname.cpp
 // RUN: llvm-pdbutil dump -all debug-info-objname.obj | FileCheck %s 
--check-prefix=ABSOLUTE
 
 // No output file provided, input file is absolute, we emit an absolute path 
(MSVC behavior).
-// RUN: %clang_cl /c /Z7 -nostdinc -- %T/debug-info-objname.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -nostdinc -- 
%T/debug-info-objname.cpp
 // RUN: llvm-pdbutil dump -all debug-info-objname.obj | FileCheck %s 
--check-prefix=ABSOLUTE
 
 // The output file is provided as an absolute path, we emit an absolute path.
-// RUN: %clang_cl /c /Z7 -nostdinc /Fo%T/debug-info-objname.obj -- 
%T/debug-info-objname.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -nostdinc 
/Fo%T/debug-info-objname.obj -- %T/debug-info-objname.cpp
 // RUN: llvm-pdbutil dump -all debug-info-objname.obj | FileCheck %s 
--check-prefix=ABSOLUTE
 
 // The output file is provided as relative path, -working-dir is provided, we 
emit an absolute path.
-// RUN: %clang_cl /c /Z7 -nostdinc -working-dir=%T debug-info-objname.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -nostdinc 
-working-dir=%T debug-info-objname.cpp
 // RUN: llvm-pdbutil dump -all debug-info-objname.obj | FileCheck %s 
--check-prefix=ABSOLUTE
 
 // The input file name is relative and we specify -fdebug-compilation-dir, we 
emit a relative path.
-// RUN: %clang_cl /c /Z7 -nostdinc -fdebug-compilation-dir=. 
debug-info-objname.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -nostdinc 
-fdebug-compilation-dir=. debug-info-objname.cpp
 // RUN: llvm-pdbutil dump -all debug-info-objname.obj | FileCheck %s 
--check-prefix=RELATIVE
 
 // Ensure /FA emits an .asm file which contains the path to the final .obj, 
not the .asm
-// RUN: %clang_cl /c /Z7 -nostdinc -fdebug-compilation-dir=. /FA 
debug-info-objname.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -nostdinc 
-fdebug-compilation-dir=. /FA debug-info-objname.cpp
 // RUN: cat debug-info-objname.asm | FileCheck %s --check-prefix=ASM
 
 // Same thing for -save-temps
-// RUN: %clang_cl /c /Z7 -nostdinc -fdebug-compilation-dir=. 
/clang:-save-temps debug-info-objname.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -nostdinc 
-fdebug-compilation-dir=. /clang:-save-temps debug-info-objname.cpp
 // RUN: cat debug-info-objname.asm | FileCheck %s --check-prefix=ASM
 
 int main() {



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

[PATCH] D116110: Introduce the AttributeMask class

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: llvm/include/llvm/IR/Attributes.h:992
+  bool empty() const { return Attrs.none(); }
+  bool hasAttributes() const { return !TargetDepAttrs.empty() && Attrs.any(); }
+};

Shouldn't this be `||`?



Comment at: llvm/include/llvm/IR/Attributes.h:993
+
+  bool empty() const { return Attrs.none(); }
+  bool hasAttributes() const { return TargetDepAttrs.empty() && Attrs.none(); }

serge-sans-paille wrote:
> nikic wrote:
> > Should this check TargetDepAttrs as well?
> I matched the implementation in AttrBuilder, which looks odd to me too.
Where is the method used? Can we just drop it?


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

https://reviews.llvm.org/D116110

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

[clang] cd407f6 - [Clang] Fix build by restricting debug-info-objname.cpp test to x86.

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-12-21T12:22:25-05:00
New Revision: cd407f6e52b09cce2bef24c74b7f36fedc94991b

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

LOG: [Clang] Fix build by restricting debug-info-objname.cpp test to x86.

See: https://lab.llvm.org/buildbot/#/builders/188/builds/7188

Added: 


Modified: 
clang/test/CodeGenCXX/debug-info-objname.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/debug-info-objname.cpp 
b/clang/test/CodeGenCXX/debug-info-objname.cpp
index 1a9a6cfa2081..d80d805b8b41 100644
--- a/clang/test/CodeGenCXX/debug-info-objname.cpp
+++ b/clang/test/CodeGenCXX/debug-info-objname.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: x86-registered-target
 // RUN: cp %s %T/debug-info-objname.cpp
 // RUN: cd %T
 



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

[PATCH] D116016: [Clang] [PowerPC] Emit module flag for current float abi

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D116016#3203005 , @nemanjai wrote:

> We should not be emitting the attribute in modules that do not have any use 
> of `long double`.

Right, otherwise it would be a bit unfortunate that a library not using long 
double has to be compiled differently to use ThinLTO with other programs, since 
the module flags metadata's behavior is `Error` (1).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116016

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

[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

D115961  and D115964 
 were the next two pieces of this, both have 
now landed.

The last bit is honoring call site parameter attributes on indirect calls 
outside the SCC.  I'll post that separately and then close this review because 
everything will have been covered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

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

[PATCH] D116110: Introduce the AttributeMask class

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 395713.
serge-sans-paille added a comment.

Remove `empty()` method and only keep `hasAttributes`


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

https://reviews.llvm.org/D116110

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/IR/Argument.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Scalar/SCCP.cpp
  llvm/unittests/IR/AttributesTest.cpp

Index: llvm/unittests/IR/AttributesTest.cpp
===
--- llvm/unittests/IR/AttributesTest.cpp
+++ llvm/unittests/IR/AttributesTest.cpp
@@ -81,12 +81,12 @@
   AttrBuilder B_align_readonly;
   B_align_readonly.addAttribute(AlignAttr);
   B_align_readonly.addAttribute(Attribute::ReadOnly);
-  AttrBuilder B_align;
+  AttributeMask B_align;
   B_align.addAttribute(AlignAttr);
   AttrBuilder B_stackalign_optnone;
   B_stackalign_optnone.addAttribute(StackAlignAttr);
   B_stackalign_optnone.addAttribute(Attribute::OptimizeNone);
-  AttrBuilder B_stackalign;
+  AttributeMask B_stackalign;
   B_stackalign.addAttribute(StackAlignAttr);
 
   AttributeSet AS = AttributeSet::get(C, B_align_readonly);
Index: llvm/lib/Transforms/Scalar/SCCP.cpp
===
--- llvm/lib/Transforms/Scalar/SCCP.cpp
+++ llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -486,7 +486,7 @@
   // inaccessiblemem_or_argmemonly attributes do not hold any longer. Remove
   // them from both the function and callsites.
   if (ReplacedPointerArg) {
-AttrBuilder AttributesToRemove;
+AttributeMask AttributesToRemove;
 AttributesToRemove.addAttribute(Attribute::ArgMemOnly);
 AttributesToRemove.addAttribute(Attribute::InaccessibleMemOrArgMemOnly);
 F.removeFnAttrs(AttributesToRemove);
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1383,7 +1383,7 @@
 
   for (Attribute A : AL.getFnAttrs()) {
 if (isStatepointDirectiveAttr(A))
-  FnAttrs.remove(A);
+  FnAttrs.removeAttribute(A);
   }
 
   // Just skip parameter and return attributes for now
@@ -2654,7 +2654,7 @@
 template 
 static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH,
   unsigned Index) {
-  AttrBuilder R;
+  AttributeMask R;
   AttributeSet AS = AH.getAttributes().getAttributes(Index);
   if (AS.getDereferenceableBytes())
 R.addAttribute(Attribute::get(Ctx, Attribute::Dereferenceable,
@@ -2666,7 +2666,7 @@
 if (AS.hasAttribute(Attr))
   R.addAttribute(Attr);
 
-  if (!R.empty())
+  if (R.hasAttributes())
 AH.setAttributes(AH.getAttributes().removeAttributesAtIndex(Ctx, Index, R));
 }
 
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3664,7 +3664,7 @@
   // will become a non-readonly function after it is instrumented by us. To
   // prevent this code from being optimized out, mark that function
   // non-readonly in advance.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
@@ -5359,7 +5359,7 @@
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
-  AttrBuilder B;
+  AttributeMask B;
   B.addAttribute(Attribute::ReadOnly)
   .addAttribute(Attribute::ReadNone)
   .addAttribute(Attribute::WriteOnly)
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -454,7 +454,7 @@
   MDNode *OriginStoreWeights;
   DFSanABIList ABIList;
   DenseMap UnwrappedFnMap;
-  AttrBuilder ReadOnlyNoneAttrs;
+  AttributeMask ReadOnlyNoneAttrs;
 
   /// Memory map parameters used in calculation mapping application addresses
   /// to shadow addresses and origin addresses.
@@ -1121,7 +1121,7 @@
 
   BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", NewF);
 

[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames abandoned this revision.
reames added a comment.

This has been replaced by a series of smaller patches implementing individual 
parts.

The last of which - parameter attributes on indirect calls - is out for review 
as D116118 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

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

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

In D115804#3202479 , @sepavloff wrote:

> In D115804#3201681 , @spatel wrote:
>
>> In D115804#3201044 , @craig.topper 
>> wrote:
>>
>>> What's the plan for constrained intrinsics versions of these intrinsics? 
>>> The IRBuilder calls for CreateFPToSI and CreateFPToUI are strict FP aware, 
>>> but this new code isn't.
>>
>> Not sure. cc'ing @kpn @sepavloff @andrew.w.kaylor  
>> The saturating intrinsics implement non-standard behavior for C languages 
>> AFAIK, so we might want to warn if someone tries to use 
>> "-fno-strict-float-cast-overflow" and "-ffp-exception-behavior=strict" at 
>> the same time? Or we try to support that corner case by adding even more FP 
>> intrinsics?
>
> Conversion `float`->`int` depends on rounding mode. At least on some ML cores 
> this conversion is made with saturating semantics. So ability to specify 
> rounding mode would be useful for such targets.

What about the idea of using operand bundles? I think the idea there was to 
avoid having to add new intrinsics for every target-specific floating point 
intrinsic. But maybe the idea will work here as well?

I think we definitely need to warn when -ffp-exception-behavior=strict or 
=maytrap is given and incompatible options are also used with it. In that case 
we should also be disabling the -ffp-exception-behavior option and the related 
#pragmas plus warning about the disabling. We already disable+warn when our 
backend doesn't support strictfp, we can do the same here.

If someone in the future wants to implement the support then that'll be nice. 
But in the meantime we shouldn't be silently letting people use strictfp plus a 
contradictory option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115804

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

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan updated this revision to Diff 395724.
Jake-Egan added a comment.

Added `-fintegrated-as` option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

Files:
  clang/test/ASTMerge/anonymous-fields/test.cpp
  clang/test/ASTMerge/codegen-body/test.c
  clang/test/ASTMerge/injected-class-name-decl/test.cpp
  clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
  clang/test/ClangScanDeps/resource_directory.c
  clang/test/lit.cfg.py
  llvm/test/LTO/X86/remangle_intrinsics.ll
  llvm/test/lit.cfg.py
  llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -1131,7 +1131,11 @@
   EXPECT_STREQ(String1, *Extracted3);
 }
 
+#if defined(_AIX) && defined(__64BIT__)
+TEST(DWARFDebugInfo, DISABLED_TestEmptyStringOffsets) {
+#else
 TEST(DWARFDebugInfo, TestEmptyStringOffsets) {
+#endif
   Triple Triple = getNormalizedDefaultTargetTriple();
   if (!isObjectEmissionSupported(Triple))
 return;
@@ -1160,7 +1164,11 @@
   DwarfContext->getDWARFObj().getStrOffsetsSection().Data.empty());
 }
 
+#if defined(_AIX) && defined(__64BIT__)
+TEST(DWARFDebugInfo, DISABLED_TestRelations) {
+#else
 TEST(DWARFDebugInfo, TestRelations) {
+#endif
   Triple Triple = getNormalizedDefaultTargetTriple();
   if (!isObjectEmissionSupported(Triple))
 return;
@@ -1347,7 +1355,11 @@
   EXPECT_FALSE(DefaultDie.getSibling().isValid());
 }
 
+#if defined(_AIX) && defined(__64BIT__)
+TEST(DWARFDebugInfo, DISABLED_TestChildIterators) {
+#else
 TEST(DWARFDebugInfo, TestChildIterators) {
+#endif
   Triple Triple = getNormalizedDefaultTargetTriple();
   if (!isObjectEmissionSupported(Triple))
 return;
@@ -1456,7 +1468,11 @@
   EXPECT_EQ(CUDie.begin(), CUDie.end());
 }
 
+#if defined(_AIX) && defined(__64BIT__)
+TEST(DWARFDebugInfo, DISABLED_TestAttributeIterators) {
+#else
 TEST(DWARFDebugInfo, TestAttributeIterators) {
+#endif
   Triple Triple = getNormalizedDefaultTargetTriple();
   if (!isObjectEmissionSupported(Triple))
 return;
@@ -1518,7 +1534,11 @@
   EXPECT_EQ(E, ++I);
 }
 
+#if defined(_AIX) && defined(__64BIT__)
+TEST(DWARFDebugInfo, DISABLED_TestFindRecurse) {
+#else
 TEST(DWARFDebugInfo, TestFindRecurse) {
+#endif
   Triple Triple = getNormalizedDefaultTargetTriple();
   if (!isObjectEmissionSupported(Triple))
 return;
@@ -1732,7 +1752,11 @@
   // Test
 }
 
+#if defined(_AIX) && defined(__64BIT__)
+TEST(DWARFDebugInfo, DISABLED_TestFindAttrs) {
+#else
 TEST(DWARFDebugInfo, TestFindAttrs) {
+#endif
   Triple Triple = getNormalizedDefaultTargetTriple();
   if (!isObjectEmissionSupported(Triple))
 return;
@@ -1795,7 +1819,11 @@
   EXPECT_EQ(DieMangled, toString(NameOpt, ""));
 }
 
+#if defined(_AIX) && defined(__64BIT__)
+TEST(DWARFDebugInfo, DISABLED_TestImplicitConstAbbrevs) {
+#else
 TEST(DWARFDebugInfo, TestImplicitConstAbbrevs) {
+#endif
   Triple Triple = getNormalizedDefaultTargetTriple();
   if (!isObjectEmissionSupported(Triple))
 return;
Index: llvm/test/lit.cfg.py
===
--- llvm/test/lit.cfg.py
+++ llvm/test/lit.cfg.py
@@ -405,3 +405,22 @@
 
 if "MemoryWithOrigins" in config.llvm_use_sanitizer:
 config.available_features.add('use_msan_with_origins')
+
+def exclude_unsupported_files_for_aix(dirname):
+   for filename in os.listdir(dirname):
+   source_path = os.path.join( dirname, filename)
+   if os.path.isdir(source_path):
+   continue
+   f = open(source_path, 'r')
+   try:
+  data = f.read()
+  # 64-bit object files are not supported on AIX, so exclude the tests.
+  if ('-emit-obj' in data or '-filetype=obj' in data) and '64' in config.target_triple:
+config.excludes += [ filename ]
+   finally:
+  f.close()
+
+if 'aix' in config.target_triple:
+for directory in ('/CodeGen/X86', '/DebugInfo', '/DebugInfo/X86', '/DebugInfo/Generic', '/LTO/X86', '/Linker'):
+exclude_unsupported_files_for_aix(config.test_source_root + directory)
+
Index: llvm/test/LTO/X86/remangle_intrinsics.ll
===
--- llvm/test/LTO/X86/remangle_intrinsics.ll
+++ llvm/test/LTO/X86/remangle_intrinsics.ll
@@ -1,3 +1,4 @@
+; UNSUPPORTED: powerpc64-ibm-aix
 ; RUN: llvm-as < %s > %t1
 ; RUN: llvm-as < %p/Inputs/remangle_intrinsics.ll > %t2
 ; RUN: llvm-lto %t1 %t2 | FileCheck %s
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -243,3 +243,23 @@
 # Add a vendor-specific feature.
 if config.clang_vendor_uti:
 config.available_features.add('clang-vendor=' + config.clang_vendor_uti)
+
+def exclud

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan marked an inline comment as done.
Jake-Egan added inline comments.



Comment at: clang/test/ClangScanDeps/modules-full-by-mod-name.cpp:1
+// UNSUPPORTED: powerpc64-ibm-aix
 // RUN: rm -rf %t.dir

shchenz wrote:
> Jake-Egan wrote:
> > jsji wrote:
> > > there is no `fmodule-format=obj` here, why are we failing here?
> > > 
> > They have the same error as the other tests:
> > 
> > ```
> > 64-bit XCOFF object files are not supported yet.
> > ```
> Which RUN line causes the `64-bit XCOFF object files are not supported yet` 
> assertion? NO `-emit-obj` or `fmodule-format=obj` found in all the RUN lines.
```
// RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format 
experimental-full \
// RUN:   -mode preprocess-minimized-sources -module-name=header1 >> %t.result
```
It fails here. `t.result` contains a module using the `-fmodule-format=obj` 
option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

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

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 395727.
paulkirth added a comment.

- [misexpect] Add missing tests and modify diagnostic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,290 @@
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profil

[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, patch basically looks good.  Minor requests and an observation for 
possible follow-up work.




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2247
   // Otherwise, evaluate and record it.
-  if (const Expr *size = vat->getSizeExpr()) {
+  if (const Expr *SizeExpr = vat->getSizeExpr()) {
 // It's possible that we might have emitted this already,

Please continue to use lowercase names for local variables for consistency with 
the surrounding code.  The rename to `sizeExpr` is good; you can then rename 
`Size` to `size` or `sizeValue`.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2259
   //   greater than zero.
-  if (SanOpts.has(SanitizerKind::VLABound) &&
-  size->getType()->isSignedIntegerType()) {
+  if (SanOpts.has(SanitizerKind::VLABound) && SEType->isIntegerType()) 
{
 SanitizerScope SanScope(this);

Sema requires the bound expression to have integral type, so you don't need to 
do this.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2275
   // undefined behavior to have a negative bound.
-  entry = Builder.CreateIntCast(Size, SizeTy, /*signed*/ false);
+  MapEntry = Builder.CreateIntCast(Size, SizeTy, /*signed*/ false);
 }

This would be a different bug, but should UBSan also be doing a bounds check if 
the type is larger than `size_t`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116048

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

[PATCH] D113530: [wip] [analyzer] support use-after-free checking with parameter annotation

[Chris D'Angelo via Phabricator via cfe-commits]

chrisdangelo updated this revision to Diff 395726.
chrisdangelo edited the summary of this revision.
chrisdangelo added a comment.

This change removes the previous ownership_takes_param work. This change 
augments the existing Ownership attribute so that it is now applicable to 
function parameters.

Theses changes have been used to run the analyzer on a large C / C++ project. 
The tests have been run locally and successfully with `ninja 
check-clang-analysis` and `ninja check-clang`.

This diff is functional but is not fully complete. As of this iteration, these 
are the current tasks unresolved...

1. This change does not fully validate correct usage in SemaDeclAttr.cpp. More 
can be done to validate that ownership attributes are not used in ways that are 
in conflict with one another. This conflict can occur with a combination of 
ownership attributes applied to the function declaration simultaneously with 
parameter annotations.

2. We are interested in deprecating the use of function based ownership 
attributes in favor of using function parameter annotations. This change does 
not currently provide deprecation warnings.

3. This change does not test semantic analysis of ownership parameters 
(malloc-annotations.c).

4. This change does not fully test static analysis of ownership parameters 
(attr-ownership.c/cpp).


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

https://reviews.llvm.org/D113530

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/malloc-annotations.c

Index: clang/test/Analysis/malloc-annotations.c
===
--- clang/test/Analysis/malloc-annotations.c
+++ clang/test/Analysis/malloc-annotations.c
@@ -16,6 +16,10 @@
__attribute((ownership_holds(malloc, 1, 2)));
 void __attribute((ownership_returns(malloc, 1))) *my_malloc2(size_t);
 void __attribute((ownership_holds(malloc, 1))) my_hold(void *);
+void my_free_first_parameter(void * __attribute__((ownership_takes(malloc;
+void my_free_second_parameter(void *, void * __attribute__((ownership_takes(malloc;
+void my_free_two_parameters(void * __attribute__((ownership_takes(malloc))), void * __attribute__((ownership_takes(malloc;
+void __attribute((ownership_takes(malloc, 1))) my_free_both_params_via_param_and_func(void *, void * __attribute__((ownership_takes(malloc;
 
 // Duplicate attributes are silly, but not an error.
 // Duplicate attribute has no extra effect.
@@ -273,3 +277,49 @@
   my_freeBoth(p, q);
 }
 
+int *testFreeFirstParameterUseAfterFree() {
+  int *p = malloc(4);
+
+  my_free_first_parameter(p);
+  return p; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testFreeSecondParameterUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_second_parameter(p1, p2);
+  return p2; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testFreeSecondParameterNoUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_second_parameter(p1, p2);
+  return p1;
+}
+
+int *testFreeFirstOfTwoParametersUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_two_parameters(p1, p2);
+  return p1; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testFreeSecondOfTwoParametersUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_two_parameters(p1, p2);
+  return p2; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testAuthorMayUseFuncDeclAndParamOwnershipAttr() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+  
+  my_free_both_params_via_param_and_func(p1, p2);
+  return p2; // expected-warning{{Use of memory after it is freed}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -301,11 +301,7 @@
  check::NewAllocator, check::PostStmt,
  check::PostObjCMessage, check::Location, eval::Assume> {
 public:
-  /// In pessimistic mode, the checker assumes that it does not know which
-  /// functions might free the memory.
-  /// In optimistic mode, the checker assumes that all user-defined functions
-  /// which might free a pointer are annotated.
-  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
+  DefaultBool IsOptimisticAnalyzerOptionEnabled;
 
   DefaultBool ShouldRegisterNoOwnershipChangeVisitor;
 
@@ -448,6 +444,18 @@
   /// if the macro value could be determined, and if yes the value itself.
   mutable Optional KernelZeroSizePtrValue;
 
+  LLVM_NODISCARD
+  ProgramStateRef ownershipFuncAttr(const CallEvent &Call,
+CheckerContext &C,
+const FunctionDecl *FD,
+

[PATCH] D116113: Add LLDB synthetic child and summary scripts for llvm::SmallVector, llvm::Optional, llvm::ErrorOr and llvm::Expected.

[walter erquinigo via Phabricator via cfe-commits]

wallace added a comment.

thanks for doing this. Just a few minor comments and i think this is good to go




Comment at: clang/utils/ClangDataFormat.py:215-218
+if self.hasVal:
+result = self.value.GetIndexOfChildWithName(name)
+return None if result == 4294967295 else result
+return None

it's safe to return 0 here, as it'll be only invoked when there's a value, so 
there's a single child with index 0



Comment at: clang/utils/ClangDataFormat.py:221-223
+if self.hasVal:
+return self.value.GetChildAtIndex(index)
+return None

wouldn' just

  return self.value

be enough?

index is always going to be 0 if called



Comment at: clang/utils/ClangDataFormat.py:279
+
+class Expected(object):
+def __init__(self, valobj, internal_dict):

god bless you



Comment at: clang/utils/ClangDataFormat.py:331-335
+# We compute the right value to use as the lldb.SBValue to use in
+# self.value in the update() method so we can just use that object to
+# get our asnwers
+result = self.value.GetIndexOfChildWithName(name)
+return None if result == 4294967295 else result

you can just return 0



Comment at: clang/utils/ClangDataFormat.py:409-410
+# get our asnwers
+result = self.value.GetIndexOfChildWithName(name)
+return None if result == 4294967295 else result
+

ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116113

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

[PATCH] D115429: [Clang] Implement the rest of __builtin_elementwise_* functions.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D115429#3201713 , @junaire wrote:

> Hi, @aaron.ballman  I'm sorry for not updating the patch in time because I'm 
> preparing for my school final exam :-(
> One thing I want to mention is that `__builtin_elementwise_roundeven` is 
> actually been added in the RFC during the code review. You can find it in 
> D111529 .
>
>> "Prevailing rounding mode" is not super-useful, other than as a spelling for 
>> round-to-nearest-ties-to-even (IEEE 754 default rounding). Outside of a 
>> FENV_ACCESS ON context, there's not even really a notion of "prevailing 
>> rounding mode" to appeal to. I assume the intent is for this to lower to 
>> e.g. x86 ROUND* with the dynamic rounding-mode immediate.
>>
>> I would recommend adding __builtin_elementwise_roundeven(T x) instead, which 
>> would statically bind IEEE default rounding (following TS 18661-1 naming) 
>> without having to appeal to prevailing rounding mode, and can still lower to 
>> ROUND* on x86 outside of FENV_ACCESS ON contexts, which is the norm for 
>> vector code (and FRINTN unconditionally on armv8). I think we can punt on 
>> rint/nearbyint for now, and add them in the future if there's a need.

Yes, this was a change from the RFC after some feedback to the patch.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3137-3160
   case Builtin::BI__builtin_elementwise_ceil: {
 Value *Op0 = EmitScalarExpr(E->getArg(0));
 Value *Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::ceil, Op0,
  nullptr, "elt.ceil");
 return RValue::get(Result);
   }
+  case Builtin::BI__builtin_elementwise_floor: {

aaron.ballman wrote:
> It's starting to seem like we should have a helper function that takes the 
> intrinsic to create and the name to give the operation; WDYT?
Sounds good! Might be good to split the `emitUnaryBuiltin` changes off into a 
separate change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115429

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

[PATCH] D113530: [wip] [analyzer] support use-after-free checking with parameter annotation

[Chris D'Angelo via Phabricator via cfe-commits]

chrisdangelo updated this revision to Diff 395734.
chrisdangelo added a comment.

This change includes clang-format edits.


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

https://reviews.llvm.org/D113530

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/malloc-annotations.c

Index: clang/test/Analysis/malloc-annotations.c
===
--- clang/test/Analysis/malloc-annotations.c
+++ clang/test/Analysis/malloc-annotations.c
@@ -16,6 +16,10 @@
__attribute((ownership_holds(malloc, 1, 2)));
 void __attribute((ownership_returns(malloc, 1))) *my_malloc2(size_t);
 void __attribute((ownership_holds(malloc, 1))) my_hold(void *);
+void my_free_first_parameter(void *__attribute__((ownership_takes(malloc;
+void my_free_second_parameter(void *, void *__attribute__((ownership_takes(malloc;
+void my_free_two_parameters(void *__attribute__((ownership_takes(malloc))), void *__attribute__((ownership_takes(malloc;
+void __attribute((ownership_takes(malloc, 1))) my_free_both_params_via_param_and_func(void *, void *__attribute__((ownership_takes(malloc;
 
 // Duplicate attributes are silly, but not an error.
 // Duplicate attribute has no extra effect.
@@ -273,3 +277,49 @@
   my_freeBoth(p, q);
 }
 
+int *testFreeFirstParameterUseAfterFree() {
+  int *p = malloc(4);
+
+  my_free_first_parameter(p);
+  return p; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testFreeSecondParameterUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_second_parameter(p1, p2);
+  return p2; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testFreeSecondParameterNoUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_second_parameter(p1, p2);
+  return p1;
+}
+
+int *testFreeFirstOfTwoParametersUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_two_parameters(p1, p2);
+  return p1; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testFreeSecondOfTwoParametersUseAfterFree() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_two_parameters(p1, p2);
+  return p2; // expected-warning{{Use of memory after it is freed}}
+}
+
+int *testAuthorMayUseFuncDeclAndParamOwnershipAttr() {
+  int *p1 = malloc(4);
+  int *p2 = malloc(4);
+
+  my_free_both_params_via_param_and_func(p1, p2);
+  return p2; // expected-warning{{Use of memory after it is freed}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -301,11 +301,7 @@
  check::NewAllocator, check::PostStmt,
  check::PostObjCMessage, check::Location, eval::Assume> {
 public:
-  /// In pessimistic mode, the checker assumes that it does not know which
-  /// functions might free the memory.
-  /// In optimistic mode, the checker assumes that all user-defined functions
-  /// which might free a pointer are annotated.
-  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
+  DefaultBool IsOptimisticAnalyzerOptionEnabled;
 
   DefaultBool ShouldRegisterNoOwnershipChangeVisitor;
 
@@ -448,6 +444,16 @@
   /// if the macro value could be determined, and if yes the value itself.
   mutable Optional KernelZeroSizePtrValue;
 
+  LLVM_NODISCARD
+  ProgramStateRef ownershipFuncAttr(const CallEvent &Call, CheckerContext &C,
+const FunctionDecl *FD,
+ProgramStateRef S) const;
+
+  LLVM_NODISCARD
+  ProgramStateRef ownershipParamAttr(const CallEvent &Call, CheckerContext &C,
+ const FunctionDecl *FD,
+ ProgramStateRef S) const;
+
   /// Process C++ operator new()'s allocation, which is the part of C++
   /// new-expression that goes before the constructor.
   LLVM_NODISCARD
@@ -1080,7 +1086,7 @@
   ReallocatingMemFnMap.lookup(Call))
 return true;
 
-  if (!ShouldIncludeOwnershipAnnotatedFunctions)
+  if (!IsOptimisticAnalyzerOptionEnabled)
 return false;
 
   const auto *Func = dyn_cast(Call.getDecl());
@@ -1396,33 +1402,109 @@
   C.addTransition(State);
 }
 
-void MallocChecker::checkOwnershipAttr(const CallEvent &Call,
-   CheckerContext &C) const {
+static const FunctionDecl *functionDeclForCall(const CallEvent &Call,
+   CheckerContext &C) {
   ProgramStateRef State = C.getState();
   const auto *CE = dyn_cast_or_null(Call.getOriginExpr());
   if (!CE)
-return;
+return nullptr;
+
   const FunctionDecl *FD = C.getCalleeDecl(CE);
   if (!FD)
-return;
-  if (ShouldIncludeOwnershipAnnotatedFunctions ||
-  C

[PATCH] D116110: Introduce the AttributeMask class

[Nikita Popov via Phabricator via cfe-commits]

nikic added a reviewer: aeubanks.
nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

This looks good to me, but I'd recommend waiting a bit before landing in case 
there's further input, as this is a non-trivial API change.

I think separating AttrBuilder (which stores attribute keys and values) from 
AttributeMask (which only stores keys) makes sense conceptually. With the 
current API, it's not really obvious that the attribute values are completely 
ignored for removal operations -- on the surface it looks like the attribute 
would only get removed if both key and value match, but that's not how it 
actually works.

This should also give more flexibility in optimizing AttrBuilder for it's 
primary use case (which is adding attributes).


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

https://reviews.llvm.org/D116110

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

[PATCH] D115670: Implement some constexpr vector unary operators, fix boolean-ops

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added subscribers: bmahjour, anhtuyen, qiongsiwu1.
qiongsiwu1 added a comment.

Hi everyone! We are encountering crashes in some of our `altivec` test cases 
due to this change. `clang` crashes when we have an instance of a `struct` or 
`union` type, and we assign the result of a unary operator `++` or `--` with a 
vector operand to a field of the instance. If we are assigning the result to 
just a vector type, the compiler does not crash.

Here is a reproducer. To reproduce the crash, use command `clang -target 
powerpc-unknown-unknown -maltivec small.c`.

  // small.c
  #include 
  
  // struct type
  typedef struct result
  {
  vector signed short result_vec;
  } RESULT;
  
  vector signed short op_nine = { 9, 9, 9, 9, 9, 9, 9, 9 };
  
  int main() {
  // Instance of the struct type
  RESULT r;
  
  // This line below crashes - we are assigning the result of the unary 
operator to a struct field. 
  r.result_vec = ++op_nine;
  for (int i = 0; i < 8; ++i) {
  printf("%d\t", r.result_vec[i]);
  }
  printf("\n");
  
  // This case works fine. The compiler does not crash. 
  /*vector signed short op_10 = ++op_nine;
  for (int i = 0; i < 8; ++i) {
  printf("%d\t", op_10[i]);
  }
  printf("\n"); */
  }

We discovered this on AIX and we can reproduce this on Linux on Power. The 
issue is reproducible on Intel based Macs as well. Here is a stack dump from 
MacOS.

  Assertion failed: (isVector() && "Invalid accessor"), function 
getVectorLength, file 
/Users/qiongsiwu/workspace/community/llvm-project/clang/include/clang/AST/APValue.h,
 line 498.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: 
/Users/qiongsiwu/workspace/community/llvm-project/build/bin/clang-14 -cc1 
-triple powerpc-unknown-unknown -emit-obj -mrelax-all --mrelax-relocations 
-disable-free -clear-ast-before-backend -main-file-name small.c 
-mrelocation-model static -mframe-pointer=all -fmath-errno -ffp-contract=on 
-fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu ppc 
-target-feature +altivec -mfloat-abi hard -debugger-tuning=gdb 
-target-linker-version 650.9 
-fcoverage-compilation-dir=/Users/qiongsiwu/workspace/13359 -resource-dir 
/Users/qiongsiwu/workspace/community/llvm-project/build/lib/clang/14.0.0 -I 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/
 -fdebug-compilation-dir=/Users/qiongsiwu/workspace/13359 -ferror-limit 19 
-fno-signed-char -fgnuc-version=4.2.1 -fcolor-diagnostics -faddrsig 
-D__GCC_HAVE_DWARF2_CFI_ASM=1 -o 
/var/folders/gh/5hw84m4x2hv4csmhpp8y0268gn/T/small-e9e02b.o -x c small.c
  1.small.c:23:5: current parser token 'return'
  2.small.c:12:12: parsing function body 'main'
  3.small.c:12:12: in compound statement ('{}')
  Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH 
or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
  0  clang-14 0x000107d8d1f7 
llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 39
  1  clang-14 0x000107d8bfb8 llvm::sys::RunSignalHandlers() 
+ 248
  2  clang-14 0x000107d8d840 SignalHandler(int) + 272
  3  libsystem_platform.dylib 0x7ff80662fe2d _sigtramp + 29
  4  libsystem_platform.dylib 0x7ff7ba7632d0 _sigtramp + 
18446744072435741888
  5  libsystem_c.dylib0x7ff806566d10 abort + 123
  6  libsystem_c.dylib0x7ff8065660be err + 0
  7  clang-14 0x00010c208443 (anonymous 
namespace)::VectorExprEvaluator::VisitUnaryOperator(clang::UnaryOperator 
const*) (.cold.13) + 35
  8  clang-14 0x00010a40b1d7 (anonymous 
namespace)::VectorExprEvaluator::VisitUnaryOperator(clang::UnaryOperator 
const*) + 2375

On AIX, the compiler can produce the following AST during `EvaluateVector` 
before the crash.

  [EvaluateVector]
  BinaryOperator 0x11173e148 '__vector short' '='
  |-MemberExpr 0x11173e0e0 '__vector short' lvalue .result_vec 0x111739f40
  | `-DeclRefExpr 0x11173a5a0 'RESULT':'struct result' lvalue Var 0x11173a520 
'r' 'RESULT':'struct result'
  `-UnaryOperator 0x11173e130 '__vector short' prefix '++'
`-DeclRefExpr 0x11173e110 '__vector short' lvalue Var 0x11173a070 'op_nine' 
'__vector short'
  [EvaluateVector]
  UnaryOperator 0x11173e130 '__vector short' prefix '++'
  `-DeclRefExpr 0x11173e110 '__vector short' lvalue Var 0x11173a070 'op_nine' 
'__vector short'

The compiler crashes when trying to deal with `0x11173a070`. It is expecting a 
vector kind (`APValue::ValueKind::Vector`), but the actual kind is 
`APValue::ValueKind::LValue`, and the compiler crashes inside 
`SubExprValue.getVectorLength()`.

Could you help us take a look? Thanks a lot! FYI @bmahjour @anhtuyen


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE L

[PATCH] D113530: [wip] [analyzer] support use-after-free checking with parameter annotation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I'm really glad it actually works!

> We are interested in deprecating the use of function based ownership 
> attributes in favor of using function parameter annotations. This change does 
> not currently provide deprecation warnings.

Do we also need to convert `ownership_holds` to parameter attribute? I think it 
doesn't make sense to deprecate until we convert all of them.

> This change does not fully test static analysis of ownership parameters 
> (malloc-annotations.c).

I think it makes sense to simply copy that file and replace function attributes 
with parameter attributes and see if it still passes.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1405
 
-void MallocChecker::checkOwnershipAttr(const CallEvent &Call,
-   CheckerContext &C) const {
+static const FunctionDecl *functionDeclForCall(const CallEvent &Call,
+   CheckerContext &C) {

`Call.getDecl()` is intended to be the ultimate source of truth on this subject.


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

https://reviews.llvm.org/D113530

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

[PATCH] D115471: [clang] number labels in asm goto strings after tied inputs

[Bill Wendling via Phabricator via cfe-commits]

void accepted this revision.
void added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/docs/LanguageExtensions.rst:1474
 
+When using tied-outputs (ie. outputs that are inputs and outputs, not just
+outputs) with the `+r` constraint, there is a hidden input that's created

nit: s/ie./i.e./


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115471

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

[PATCH] D116022: [clang][dataflow] Add support for terminating virtual destructors

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:50
+  std::unique_ptr Cfg;
+  llvm::DenseMap StmtToBlock;
+};

xazax.hun wrote:
> There is a special class for this at `clang/lib/Analysis/CFGStmtMap.cpp`. 
> That also does some special handling of the terminators. I wonder if we need 
> to do something similar here (or just use that class as is?).
The only downside I see is that CFGStmtMap needs a ParentMap, which is not 
cheap to make, but we don't need. Maybe we need to make it optional in 
CFGStmtMap?



Comment at: 
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h:103
 std::vector>
-runTypeErasedDataflowAnalysis(const CFG &Cfg,
+runTypeErasedDataflowAnalysis(const ControlFlowContext &Ctx,
   TypeErasedDataflowAnalysis &Analysis,

I'd suggest a more verbose name like `CFCtx`, since `Ctx` is most often used in 
Clang for `ASTContext`. (Here and elsewhere in the patch.)



Comment at: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp:1
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/AST/ASTContext.h"

Please add a license comment.



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:47
+auto Ctx = ControlFlowContext::build(nullptr, Body, Result.Context);
+assert(Ctx);
 

Use `llvm::cantFail` to unwrap the `Expected`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116022

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

[PATCH] D116059: [Clang][CFG] check children statements of asm goto

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:826
+// it as potentially uninitialized for those cases where it's used on
+// an indirect path, where it's not guaranteed to be defined.
 vals[VD] = MayUninitialized;

nickdesaulniers wrote:
> efriedma wrote:
> > The old and the new code here seem to be doing very different things.  It 
> > isn't clear why.
> > 
> > Actually, I'm not sure why we're checking whether the variable is 
> > initialized.  Whether the variable is initialized going into the inline asm 
> > isn't really related to whether it's initialized when we exit the inline 
> > asm.  For register outputs, the output isn't even in the same register as 
> > the old value of the variable.
> I suspect we're now eagerly marking variables initialized by visiting the 
> child nodes, which we weren't doing previously.
> 
> See also: https://reviews.llvm.org/D71314.
I'm very confused as well. You're basically going from a strong statement - 
(This variable is initialized) - to a weaker statement - (This variable may be 
uninitialized). That doesn't make a lot of sense to me. Could you explain it 
better in the comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116059

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

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

Updated the review's edit permissions. Sorry about that, @kadircet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116085

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

[PATCH] D116022: [clang][dataflow] Add support for terminating virtual destructors

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

> How resilient is this pattern matching?

Possibly it is not very robust. However it is very important for a certain 
style of precondition verification macros used by various Google projects. I 
can't find an exact open source copy, but they are generally similar to this 
one, but usually use a ternary instead of an if statement:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/re2/src/util/logging.h

Also similar to this one, but with an object that has a noreturn destructor 
instead of inline assembly that causes a trap:

https://source.chromium.org/chromium/chromium/src/+/main:base/check.h

If it wasn't for this "curious" coding pattern that is stamped into many 
function thanks to the macro, we wouldn't be bothering with this pattern 
matching at this point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116022

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

[PATCH] D116022: [clang][dataflow] Add support for terminating virtual destructors

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

xazax.hun added a comment.

In D116022#3205478 , @gribozavr2 
wrote:

>> How resilient is this pattern matching?
>
> Possibly it is not very robust. However it is very important for a certain 
> style of precondition verification macros used by various Google projects. I 
> can't find an exact open source copy, but they are generally similar to this 
> one, but usually use a ternary instead of an if statement:
>
> https://source.chromium.org/chromium/chromium/src/+/main:third_party/re2/src/util/logging.h
>
> Also similar to this one, but with an object that has a noreturn destructor 
> instead of inline assembly that causes a trap:
>
> https://source.chromium.org/chromium/chromium/src/+/main:base/check.h
>
> If it wasn't for this "curious" coding pattern that is stamped into many 
> function thanks to the macro, we wouldn't be bothering with this pattern 
> matching at this point.

I see. This makes sense as a temporary workaround, but in the long term I'd 
love to see a more general solution. I'm ok with this as is, but I'd love to 
see more tests that (possibly) does not provide the expected results along some 
TODOs and some comments about the specific coding patterns this is targeting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116022

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

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

Good that you renamed the method.


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

https://reviews.llvm.org/D115967

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

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: melver, nickdesaulniers, t.p.northover.
Herald added subscribers: pengfei, kristof.beyls.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The Linux kernel has a make macro called cc-option that invokes the
compiler with an option in isolation to see if it is supported before
adding it to CFLAGS. The exit code of the compiler is used to determine
if the flag is supported and should be added to the compiler invocation.

A call to cc-option with '-mno-outline-atomics' was added to prevent
linking errors with newer GCC versions but this call succeeds with a
non-AArch64 target because there is no warning from clang with
'-mno-outline-atomics', just '-moutline-atomics'. Because the call
succeeds and adds '-mno-outline-atomics' to the compiler invocation,
there is a warning from LLVM because the 'outline-atomics target
feature is only supported by the AArch64 backend.

$ echo | clang -target x86_64 -moutline-atomics -Werror -x c -c -o /dev/null -
clang-14: error: The 'x86_64' architecture does not support -moutline-atomics; 
flag ignored [-Werror,-Woption-ignored]

$ echo $?
1

$ echo | clang -target x86_64 -mno-outline-atomics -Werror -x c -c -o /dev/null 
-
'-outline-atomics' is not a recognized feature for this target (ignoring 
feature)

$ echo $?
0

This does not match GCC's behavior, which errors when the flag is added
to a non-AArch64 target.

$ echo | gcc -moutline-atomics -x c -c -o /dev/null -
gcc: error: unrecognized command-line option ‘-moutline-atomics’; did you mean 
‘-finline-atomics’?

$ echo | gcc -mno-outline-atomics -x c -c -o /dev/null -
gcc: error: unrecognized command-line option ‘-mno-outline-atomics’; did you 
mean ‘-fno-inline-atomics’?

$ echo | aarch64-linux-gnu-gcc -moutline-atomics -x c -c -o /dev/null -

$ echo | aarch64-linux-gnu-gcc -mno-outline-atomics -x c -c -o /dev/null -

To get closer to  GCC's behavior, issue a warning when
'-mno-outline-atomics' is used without an AArch64 triple and do not add
'{-,+}outline-atomic" to the list of target features in these cases.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116128

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/x86-outline-atomics.c


Index: clang/test/Driver/x86-outline-atomics.c
===
--- /dev/null
+++ clang/test/Driver/x86-outline-atomics.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s 
-check-prefix=CHECK-OUTLINE-ATOMICS
+// CHECK-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-NO-OUTLINE-ATOMICS
+// CHECK-NO-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-NOT: "-target-feature" "-outline-atomics"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7007,18 +7007,18 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_moutline_atomics,
options::OPT_mno_outline_atomics)) {
-if (A->getOption().matches(options::OPT_moutline_atomics)) {
-  // Option -moutline-atomics supported for AArch64 target only.
-  if (!Triple.isAArch64()) {
-D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
-<< Triple.getArchName();
-  } else {
+// Option -moutline-atomics supported for AArch64 target only.
+if (!Triple.isAArch64()) {
+  D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
+  << Triple.getArchName() << A->getOption().getName();
+} else {
+  if (A->getOption().matches(options::OPT_moutline_atomics)) {
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back("+outline-atomics");
+  } else {
+CmdArgs.push_back("-target-feature");
+CmdArgs.push_back("-outline-atomics");
   }
-} else {
-  CmdArgs.push_back("-target-feature");
-  CmdArgs.push_back("-outline-atomics");
 }
   } else if (Triple.isAArch64() &&
  getToolChain().IsAArch64OutlineAtomicsDefault(Args)) {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -559,7 +559,7 @@
   InGroup;
 
 def warn_drv_moutline_atomics_unsupported_opt : Warning<
-  "'%0' does not support '-mou

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

If there are more qualified people to review and accept this, please add them, 
I was unsure of who else to add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116128

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

[PATCH] D116110: Introduce the AttributeMask class

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

dexonsmith added a comment.

In D116110#3205416 , @nikic wrote:

> This looks good to me, but I'd recommend waiting a bit before landing in case 
> there's further input, as this is a non-trivial API change.

(Maybe valuable to start a quick thread on llvm-dev to get more eyes on this? 
(I'm not suggesting that's required, just pointing out the option...))

> I think separating AttrBuilder (which stores attribute keys and values) from 
> AttributeMask (which only stores keys) makes sense conceptually. With the 
> current API, it's not really obvious that the attribute values are completely 
> ignored for removal operations -- on the surface it looks like the attribute 
> would only get removed if both key and value match, but that's not how it 
> actually works.
>
> This should also give more flexibility in optimizing AttrBuilder for it's 
> primary use case (which is adding attributes).

FWIW, I agree that this approach seems good. (SGTM, not LGTM, only because I 
haven't reviewed the patch details.)


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

https://reviews.llvm.org/D116110

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

[clang] d26520f - [Clang] Own the CommandLineArgs in CodeGenOptions

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-12-21T17:41:35-05:00
New Revision: d26520f6f78785b0c4c296a8a992f2adb656c6ec

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

LOG: [Clang] Own the CommandLineArgs in CodeGenOptions

Fixes PR52704 : https://github.com/llvm/llvm-project/issues/52704

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

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/lib/Driver/Job.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/MC/MCTargetOptions.h

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index d4781b647b877..33ec03a171362 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -398,7 +398,7 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// Executable and command-line used to create a given CompilerInvocation.
   /// Most of the time this will be the full -cc1 command.
   const char *Argv0 = nullptr;
-  ArrayRef CommandLineArgs;
+  std::vector CommandLineArgs;
 
   /// The minimum hotness value a diagnostic needs in order to be included in
   /// optimization diagnostics.

diff  --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp
index 5b87106b6565d..f63763effaffe 100644
--- a/clang/lib/Driver/Job.cpp
+++ b/clang/lib/Driver/Job.cpp
@@ -388,6 +388,8 @@ int CC1Command::Execute(ArrayRef> 
Redirects,
   Argv.push_back(getExecutable());
   Argv.append(getArguments().begin(), getArguments().end());
   Argv.push_back(nullptr);
+  Argv.pop_back(); // The terminating null element shall not be part of the
+   // slice (main() behavior).
 
   // This flag simply indicates that the program couldn't start, which isn't
   // applicable here.

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 106da642f3619..b71addd84bfd9 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4518,7 +4518,7 @@ bool CompilerInvocation::CreateFromArgsImpl(
 
   // Store the command-line for using in the CodeView backend.
   Res.getCodeGenOpts().Argv0 = Argv0;
-  Res.getCodeGenOpts().CommandLineArgs = CommandLineArgs;
+  append_range(Res.getCodeGenOpts().CommandLineArgs, CommandLineArgs);
 
   FixupInvocation(Res, Diags, Args, DashX);
 

diff  --git a/llvm/include/llvm/MC/MCTargetOptions.h 
b/llvm/include/llvm/MC/MCTargetOptions.h
index b006bb321819b..3510eeca89538 100644
--- a/llvm/include/llvm/MC/MCTargetOptions.h
+++ b/llvm/include/llvm/MC/MCTargetOptions.h
@@ -65,7 +65,7 @@ class MCTargetOptions {
   std::string COFFOutputFilename;
 
   const char *Argv0 = nullptr;
-  ArrayRef CommandLineArgs;
+  ArrayRef CommandLineArgs;
 
   /// Additional paths to search for `.include` directives when using the
   /// integrated assembler.



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

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

[Alexandre Ganea 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 rGd26520f6f787: [Clang] Own the CommandLineArgs in 
CodeGenOptions (authored by aganea).

Changed prior to commit:
  https://reviews.llvm.org/D116011?vs=395359&id=395749#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116011

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/Driver/Job.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/MC/MCTargetOptions.h


Index: llvm/include/llvm/MC/MCTargetOptions.h
===
--- llvm/include/llvm/MC/MCTargetOptions.h
+++ llvm/include/llvm/MC/MCTargetOptions.h
@@ -65,7 +65,7 @@
   std::string COFFOutputFilename;
 
   const char *Argv0 = nullptr;
-  ArrayRef CommandLineArgs;
+  ArrayRef CommandLineArgs;
 
   /// Additional paths to search for `.include` directives when using the
   /// integrated assembler.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4518,7 +4518,7 @@
 
   // Store the command-line for using in the CodeView backend.
   Res.getCodeGenOpts().Argv0 = Argv0;
-  Res.getCodeGenOpts().CommandLineArgs = CommandLineArgs;
+  append_range(Res.getCodeGenOpts().CommandLineArgs, CommandLineArgs);
 
   FixupInvocation(Res, Diags, Args, DashX);
 
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -388,6 +388,8 @@
   Argv.push_back(getExecutable());
   Argv.append(getArguments().begin(), getArguments().end());
   Argv.push_back(nullptr);
+  Argv.pop_back(); // The terminating null element shall not be part of the
+   // slice (main() behavior).
 
   // This flag simply indicates that the program couldn't start, which isn't
   // applicable here.
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -398,7 +398,7 @@
   /// Executable and command-line used to create a given CompilerInvocation.
   /// Most of the time this will be the full -cc1 command.
   const char *Argv0 = nullptr;
-  ArrayRef CommandLineArgs;
+  std::vector CommandLineArgs;
 
   /// The minimum hotness value a diagnostic needs in order to be included in
   /// optimization diagnostics.


Index: llvm/include/llvm/MC/MCTargetOptions.h
===
--- llvm/include/llvm/MC/MCTargetOptions.h
+++ llvm/include/llvm/MC/MCTargetOptions.h
@@ -65,7 +65,7 @@
   std::string COFFOutputFilename;
 
   const char *Argv0 = nullptr;
-  ArrayRef CommandLineArgs;
+  ArrayRef CommandLineArgs;
 
   /// Additional paths to search for `.include` directives when using the
   /// integrated assembler.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4518,7 +4518,7 @@
 
   // Store the command-line for using in the CodeView backend.
   Res.getCodeGenOpts().Argv0 = Argv0;
-  Res.getCodeGenOpts().CommandLineArgs = CommandLineArgs;
+  append_range(Res.getCodeGenOpts().CommandLineArgs, CommandLineArgs);
 
   FixupInvocation(Res, Diags, Args, DashX);
 
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -388,6 +388,8 @@
   Argv.push_back(getExecutable());
   Argv.append(getArguments().begin(), getArguments().end());
   Argv.push_back(nullptr);
+  Argv.pop_back(); // The terminating null element shall not be part of the
+   // slice (main() behavior).
 
   // This flag simply indicates that the program couldn't start, which isn't
   // applicable here.
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -398,7 +398,7 @@
   /// Executable and command-line used to create a given CompilerInvocation.
   /// Most of the time this will be the full -cc1 command.
   const char *Argv0 = nullptr;
-  ArrayRef CommandLineArgs;
+  std::vector CommandLineArgs;
 
   /// The minimum hotness value a diagnostic needs in order to be included in
   /// optimization diagnostics.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Bill Wendling via Phabricator via cfe-commits]

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

Approved with one change.




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

nickdesaulniers wrote:
> jyknight wrote:
> > nickdesaulniers wrote:
> > > pengfei wrote:
> > > > jyknight wrote:
> > > > > pengfei wrote:
> > > > > > If my above assumption is true, I think we can't replace the `X` 
> > > > > > with `i` here.
> > > > > > Besides, I'm confused on the indirect labels list. Are they the 
> > > > > > labels of `bar` or `foo`?
> > > > > I don't see a a problem with using "i" everywhere -- all blockaddress 
> > > > > are going to be immediate values no matter whether they're an 
> > > > > indirect target or not.
> > > > > 
> > > > > The indirect labels list is only referring to labels in the current 
> > > > > function.
> > > > > 
> > > > > This test is confusing, but, it is a test for llvm-diff, so that's 
> > > > > okay or maybe even intended. (It can't actually possibly jump to 
> > > > > @bar:%return or @bar:%t_no, because nothing ever gets the address of 
> > > > > those labels. It does get the similarly-named labels in @foo, but it 
> > > > > can't jump to those either, since they're in a different function.)
> > > > Thanks for the explanation. My point is the test3 above intended to use 
> > > > `X` to indicate the destination is not in the indirect labels list. For 
> > > > consistency, we should use `X` here too, since the @foo:%return etc. 
> > > > are not in the list either. Or we don't need to use `X` in test3.
> > > The "indirect destination list" for the `callbr` in `@bar` is the `[label 
> > > %return, label %t_no]`. Both operands have corresponding `blockaddress` 
> > > arguments. So they //should not// use `X` in this case.
> > I don't see why the correct constraint to use should be related at all to 
> > whether the blockaddress argument corresponds to a label in the indirect 
> > label list or not.
> > 
> > Using something other than "X" should probably always be preferred, since 
> > presumably the instruction you're emitting has requirements. (Unless of 
> > course you don't actually use the argument, or only use it in a comment, or 
> > something like that...in which case "X" is fine.)
> > 
> > But, FTR, in this test, the blockaddress is for a label in a //different// 
> > function ("@foo") than the function we're in ("@bar"), which is what 
> > pengfei was pointing out.
> > I don't see why the correct constraint to use should be related at all to 
> > whether the blockaddress argument corresponds to a label in the indirect 
> > label list or not.
> > Using something other than "X" should probably always be preferred
> 
> Note: child patch D115311 only changes the goto label list for `asm goto`; it 
> does not change labels in the input list.
> 
> > But, FTR, in this test, the blockaddress is for a label in a different 
> > function ("@foo") than the function we're in ("@bar"), which is what 
> > pengfei was pointing out.
> 
> Ah, I missed that.  @void can you clarify; `@bar` looks exactly like `@foo` 
> to me; was `@bar` copy-pasted from `@foo` without the `blockaddress`es being 
> updated to refer to `@bar`? I don't really understand the intent of this 
> test; I don't understand why `llvm-diff` has output for `@bar` but not 
> `@foo`.  If there's a difference, I'm having trouble spotting it.
It's a copy-and-paste error, and I'm not sure how it could have worked, given 
that LLVM's tools should abort when it sees the bad `blockaddress`. IOW, it 
should be `@bar`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410

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

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: clang/lib/Driver/Job.cpp:391
   Argv.push_back(nullptr);
+  Argv.pop_back(); // The terminating null element shall not be part of the
+   // slice (main() behavior).

I changed this to match the behavior of the //main function//, ie. `argv[argc]` 
is guaranteed to be a null pointer. Please let me know if you disagree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116011

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

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/x86-outline-atomics.c:1-7
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s 
-check-prefix=CHECK-OUTLINE-ATOMICS
+// CHECK-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-NO-OUTLINE-ATOMICS
+// CHECK-NO-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-NOT: "-target-feature" "-outline-atomics"

If `clang/test/Driver/aarch64-features.c` is only testing outline atomics, I 
wonder if this test should actually go in there? @t.p.northover , thoughts? 
Otherwise LGTM and thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116128

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

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: clang/lib/Driver/Job.cpp:390-392
   Argv.push_back(nullptr);
+  Argv.pop_back(); // The terminating null element shall not be part of the
+   // slice (main() behavior).

If we push `nullptr` and then pop it right after that, why do we push it in the 
first place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116011

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

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/test/Driver/x86-outline-atomics.c:1-7
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s 
-check-prefix=CHECK-OUTLINE-ATOMICS
+// CHECK-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-NO-OUTLINE-ATOMICS
+// CHECK-NO-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-NOT: "-target-feature" "-outline-atomics"

nickdesaulniers wrote:
> If `clang/test/Driver/aarch64-features.c` is only testing outline atomics, I 
> wonder if this test should actually go in there? @t.p.northover , thoughts? 
> Otherwise LGTM and thanks for the patch!
I did consider that initially but:

1. It does appear to test a few other AArch64 specific things.
2. It seemed weird to add an x86_64 test to an AArch64 file.

No strong opinions though, I'll go along with whatever works and makes the 
majority happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116128

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

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added inline comments.



Comment at: clang/lib/Driver/Job.cpp:390-392
   Argv.push_back(nullptr);
+  Argv.pop_back(); // The terminating null element shall not be part of the
+   // slice (main() behavior).

aheejin wrote:
> If we push `nullptr` and then pop it right after that, why do we push it in 
> the first place?
Simply to guarantee that we will have a `nullptr` element after the end of the 
container. `.pop_back()` only decrements the size, the `nullptr` value will 
remain there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116011

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

[PATCH] D116011: [Clang] Own the CommandLineArgs in CodeGenOptions

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added inline comments.



Comment at: clang/lib/Driver/Job.cpp:390-392
   Argv.push_back(nullptr);
+  Argv.pop_back(); // The terminating null element shall not be part of the
+   // slice (main() behavior).

aganea wrote:
> aheejin wrote:
> > If we push `nullptr` and then pop it right after that, why do we push it in 
> > the first place?
> Simply to guarantee that we will have a `nullptr` element after the end of 
> the container. `.pop_back()` only decrements the size, the `nullptr` value 
> will remain there.
Frankly a better fix would be to change the API of `ExecuteCC1Tool` to take an 
`ArrayRef` and not rely on internal behavior of `SmallVector`. Maybe we should 
do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116011

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

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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



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

void wrote:
> nickdesaulniers wrote:
> > jyknight wrote:
> > > nickdesaulniers wrote:
> > > > pengfei wrote:
> > > > > jyknight wrote:
> > > > > > pengfei wrote:
> > > > > > > If my above assumption is true, I think we can't replace the `X` 
> > > > > > > with `i` here.
> > > > > > > Besides, I'm confused on the indirect labels list. Are they the 
> > > > > > > labels of `bar` or `foo`?
> > > > > > I don't see a a problem with using "i" everywhere -- all 
> > > > > > blockaddress are going to be immediate values no matter whether 
> > > > > > they're an indirect target or not.
> > > > > > 
> > > > > > The indirect labels list is only referring to labels in the current 
> > > > > > function.
> > > > > > 
> > > > > > This test is confusing, but, it is a test for llvm-diff, so that's 
> > > > > > okay or maybe even intended. (It can't actually possibly jump to 
> > > > > > @bar:%return or @bar:%t_no, because nothing ever gets the address 
> > > > > > of those labels. It does get the similarly-named labels in @foo, 
> > > > > > but it can't jump to those either, since they're in a different 
> > > > > > function.)
> > > > > Thanks for the explanation. My point is the test3 above intended to 
> > > > > use `X` to indicate the destination is not in the indirect labels 
> > > > > list. For consistency, we should use `X` here too, since the 
> > > > > @foo:%return etc. are not in the list either. Or we don't need to use 
> > > > > `X` in test3.
> > > > The "indirect destination list" for the `callbr` in `@bar` is the 
> > > > `[label %return, label %t_no]`. Both operands have corresponding 
> > > > `blockaddress` arguments. So they //should not// use `X` in this case.
> > > I don't see why the correct constraint to use should be related at all to 
> > > whether the blockaddress argument corresponds to a label in the indirect 
> > > label list or not.
> > > 
> > > Using something other than "X" should probably always be preferred, since 
> > > presumably the instruction you're emitting has requirements. (Unless of 
> > > course you don't actually use the argument, or only use it in a comment, 
> > > or something like that...in which case "X" is fine.)
> > > 
> > > But, FTR, in this test, the blockaddress is for a label in a 
> > > //different// function ("@foo") than the function we're in ("@bar"), 
> > > which is what pengfei was pointing out.
> > > I don't see why the correct constraint to use should be related at all to 
> > > whether the blockaddress argument corresponds to a label in the indirect 
> > > label list or not.
> > > Using something other than "X" should probably always be preferred
> > 
> > Note: child patch D115311 only changes the goto label list for `asm goto`; 
> > it does not change labels in the input list.
> > 
> > > But, FTR, in this test, the blockaddress is for a label in a different 
> > > function ("@foo") than the function we're in ("@bar"), which is what 
> > > pengfei was pointing out.
> > 
> > Ah, I missed that.  @void can you clarify; `@bar` looks exactly like `@foo` 
> > to me; was `@bar` copy-pasted from `@foo` without the `blockaddress`es 
> > being updated to refer to `@bar`? I don't really understand the intent of 
> > this test; I don't understand why `llvm-diff` has output for `@bar` but not 
> > `@foo`.  If there's a difference, I'm having trouble spotting it.
> It's a copy-and-paste error, and I'm not sure how it could have worked, given 
> that LLVM's tools should abort when it sees the bad `blockaddress`. IOW, it 
> should be `@bar`.
> I'm not sure how it could have worked, given that LLVM's tools should abort 
> when it sees the bad blockaddress

The function referred to in the `blockaddress` does not need to be scoped to 
the same function; otherwise I don't think you could store the address of a 
label in a global variable (without shenanigans like converting it to a 
integer).

>  IOW, it should be @bar.

```
diff --git a/llvm/test/tools/llvm-diff/callbr.ll 
b/llvm/test/tools/llvm-diff/callbr.ll
index f925606c11cf..8a26f3529d43 100644
--- a/llvm/test/tools/llvm-diff/callbr.ll
+++ b/llvm/test/tools/llvm-diff/callbr.ll
@@ -25,7 +25,7 @@ return:
 
 define void @bar() {
 entry:
-  callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* 
blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
+  callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* 
blockaddress(@bar, %t_no), i8* blockaddress(@bar, %return))
   to label %asm.fallthrough [label %return, label %t_no]
 
 as

[clang] 5bb5142 - Revert [CodeView] Emit S_OBJNAME record

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-12-21T19:02:14-05:00
New Revision: 5bb5142e80c9c6eb1a948d6d2ff4834e4e69741f

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

LOG: Revert [CodeView] Emit S_OBJNAME record

Also revert all subsequent fixes:
- abd1cbf5e543f0f114d2742e109ead7d7ddbf9c4 [Clang] Disable 
debug-info-objname.cpp test on Unix until I sort out the issue.
- 00ec441253048f5e30540ea26bb0a28c42a5fc18 [Clang] debug-info-objname.cpp test: 
explictly encode a x86 target when using %clang_cl to avoid falling back to a 
native CPU triple.
- cd407f6e52b09cce2bef24c74b7f36fedc94991b [Clang] Fix build by restricting 
debug-info-objname.cpp test to x86.

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Job.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CMakeLists.txt
llvm/include/llvm/MC/MCTargetOptions.h
llvm/include/llvm/Support/Caching.h
llvm/include/llvm/Support/ToolOutputFile.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Support/Caching.cpp
llvm/test/DebugInfo/COFF/globals.ll
llvm/test/DebugInfo/COFF/multifunction.ll
llvm/test/DebugInfo/COFF/pr28747.ll
llvm/test/DebugInfo/COFF/simple.ll
llvm/test/DebugInfo/COFF/vframe-fpo.ll
llvm/test/MC/AArch64/coff-debug.ll
llvm/test/MC/ARM/coff-debugging-secrel.ll
llvm/test/MC/COFF/cv-compiler-info.ll
llvm/tools/llc/llc.cpp
llvm/tools/llvm-lto2/llvm-lto2.cpp

Removed: 
clang/test/CodeGenCXX/debug-info-objname.cpp



diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 33ec03a171362..960aa419b490e 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -227,9 +227,6 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// Output filename for the split debug info, not used in the skeleton CU.
   std::string SplitDwarfOutput;
 
-  /// Output filename used in the COFF debug information.
-  std::string ObjectFilenameForDebug;
-
   /// The name of the relocation model to use.
   llvm::Reloc::Model RelocationModel;
 

diff  --git a/clang/include/clang/Driver/Job.h 
b/clang/include/clang/Driver/Job.h
index 6e3b51f2a7995..8b287638a271d 100644
--- a/clang/include/clang/Driver/Job.h
+++ b/clang/include/clang/Driver/Job.h
@@ -204,10 +204,6 @@ class Command {
   /// from the parent process will be used.
   virtual void setEnvironment(llvm::ArrayRef NewEnvironment);
 
-  void replaceArguments(llvm::opt::ArgStringList List) {
-Arguments = std::move(List);
-  }
-
   const char *getExecutable() const { return Executable; }
 
   const llvm::opt::ArgStringList &getArguments() const { return Arguments; }

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 08e9e1a3432ae..3b47512501d39 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3802,11 +3802,6 @@ def o : JoinedOrSeparate<["-"], "o">, 
Flags<[NoXarchOption, RenderAsInput,
   CC1Option, CC1AsOption, FC1Option, FlangOption]>,
   HelpText<"Write output to ">, MetaVarName<"">,
   MarshallingInfoString>;
-def object_file_name_EQ : Joined<["-"], "object-file-name=">, 
Flags<[CC1Option, CC1AsOption, CoreOption]>,
-  HelpText<"Set the output  for debug infos">, MetaVarName<"">,
-  MarshallingInfoString>;
-def object_file_name : Separate<["-"], "object-file-name">, Flags<[CC1Option, 
CC1AsOption, CoreOption]>,
-Alias;
 def pagezero__size : JoinedOrSeparate<["-"], "pagezero_size">;
 def pass_exit_codes : Flag<["-", "--"], "pass-exit-codes">, 
Flags<[Unsupported]>;
 def pedantic_errors : Flag<["-", "--"], "pedantic-errors">, 
Group, Flags<[CC1Option]>,

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 5e16d3525b383..3195615ae561c 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -646,7 +646,6 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
   Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
   Options.DebugStrictDwarf = CodeGenOpts.DebugStrictDwarf;
-  Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug;
 
   return true;
 }
@@ -1584,8 +1583,7 @@ static void runThinLTOBackend(
 return;
 
   auto AddStream = [&](size_t Task) {
-return std::make_unique(std::move(OS),
-  CGOpts.ObjectFilenameForDebug);
+return std::make_unique(std::move(OS));
   };

[clang] a282ea4 - Reland - [CodeView] Emit S_OBJNAME record

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2021-12-21T19:02:14-05:00
New Revision: a282ea4898efe2b2e57a93b44e90c9e497520cfb

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

LOG: Reland - [CodeView] Emit S_OBJNAME record

Reland integrates build fixes & further review suggestions.

Thanks to @zturner for the initial S_OBJNAME patch!

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

Added: 
clang/test/CodeGenCXX/debug-info-objname.cpp

Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Job.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CMakeLists.txt
llvm/include/llvm/MC/MCTargetOptions.h
llvm/include/llvm/Support/Caching.h
llvm/include/llvm/Support/ToolOutputFile.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Support/Caching.cpp
llvm/test/DebugInfo/COFF/globals.ll
llvm/test/DebugInfo/COFF/multifunction.ll
llvm/test/DebugInfo/COFF/pr28747.ll
llvm/test/DebugInfo/COFF/simple.ll
llvm/test/DebugInfo/COFF/vframe-fpo.ll
llvm/test/MC/AArch64/coff-debug.ll
llvm/test/MC/ARM/coff-debugging-secrel.ll
llvm/test/MC/COFF/cv-compiler-info.ll
llvm/tools/llc/llc.cpp
llvm/tools/llvm-lto2/llvm-lto2.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 960aa419b490e..33ec03a171362 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -227,6 +227,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// Output filename for the split debug info, not used in the skeleton CU.
   std::string SplitDwarfOutput;
 
+  /// Output filename used in the COFF debug information.
+  std::string ObjectFilenameForDebug;
+
   /// The name of the relocation model to use.
   llvm::Reloc::Model RelocationModel;
 

diff  --git a/clang/include/clang/Driver/Job.h 
b/clang/include/clang/Driver/Job.h
index 8b287638a271d..6e3b51f2a7995 100644
--- a/clang/include/clang/Driver/Job.h
+++ b/clang/include/clang/Driver/Job.h
@@ -204,6 +204,10 @@ class Command {
   /// from the parent process will be used.
   virtual void setEnvironment(llvm::ArrayRef NewEnvironment);
 
+  void replaceArguments(llvm::opt::ArgStringList List) {
+Arguments = std::move(List);
+  }
+
   const char *getExecutable() const { return Executable; }
 
   const llvm::opt::ArgStringList &getArguments() const { return Arguments; }

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3b47512501d39..08e9e1a3432ae 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3802,6 +3802,11 @@ def o : JoinedOrSeparate<["-"], "o">, 
Flags<[NoXarchOption, RenderAsInput,
   CC1Option, CC1AsOption, FC1Option, FlangOption]>,
   HelpText<"Write output to ">, MetaVarName<"">,
   MarshallingInfoString>;
+def object_file_name_EQ : Joined<["-"], "object-file-name=">, 
Flags<[CC1Option, CC1AsOption, CoreOption]>,
+  HelpText<"Set the output  for debug infos">, MetaVarName<"">,
+  MarshallingInfoString>;
+def object_file_name : Separate<["-"], "object-file-name">, Flags<[CC1Option, 
CC1AsOption, CoreOption]>,
+Alias;
 def pagezero__size : JoinedOrSeparate<["-"], "pagezero_size">;
 def pass_exit_codes : Flag<["-", "--"], "pass-exit-codes">, 
Flags<[Unsupported]>;
 def pedantic_errors : Flag<["-", "--"], "pedantic-errors">, 
Group, Flags<[CC1Option]>,

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 3195615ae561c..5e16d3525b383 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -646,6 +646,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
   Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
   Options.DebugStrictDwarf = CodeGenOpts.DebugStrictDwarf;
+  Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug;
 
   return true;
 }
@@ -1583,7 +1584,8 @@ static void runThinLTOBackend(
 return;
 
   auto AddStream = [&](size_t Task) {
-return std::make_unique(std::move(OS));
+return std::make_unique(std::move(OS),
+  CGOpts.ObjectFilenameForDebug);
   };
   lto::Config Conf;
   if (CGOpts.SaveTempsFilePrefix != "") {

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index d76f810f1a7f0..2a3723975568b 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/

[clang] b9f6e09 - [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

[Owen Pan via cfe-commits]

Author: Owen Pan
Date: 2021-12-21T16:42:52-08:00
New Revision: b9f6e09b850acf2ade04f22fa423e7cc0f075e62

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

LOG: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

Move the handling of brace wrapping after => from unwrapped line
parser to token annotator and clean up the parser.

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 0a51a53da400a..505a7250572b4 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3725,6 +3725,9 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
 return true;
 
   if (Style.isCSharp()) {
+if (Left.is(TT_FatArrow) && Right.is(tok::l_brace) &&
+Style.BraceWrapping.AfterFunction)
+  return true;
 if (Right.is(TT_CSharpNamedArgumentColon) ||
 Left.is(TT_CSharpNamedArgumentColon))
   return false;

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index c2ae691bfd3d3..6010d276af05c 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1637,19 +1637,9 @@ void UnwrappedLineParser::parseStructuralElement(bool 
IsTopLevel) {
   break;
 }
 case tok::equal:
-  // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType
-  // TT_FatArrow. They always start an expression or a child block if
-  // followed by a curly brace.
-  if (FormatTok->is(TT_FatArrow)) {
-nextToken();
-if (FormatTok->is(tok::l_brace)) {
-  // C# may break after => if the next character is a newline.
-  if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) {
-// calling `addUnwrappedLine()` here causes odd parsing errors.
-FormatTok->MustBreakBefore = true;
-  }
-  parseChildBlock();
-}
+  if ((Style.isJavaScript() || Style.isCSharp()) &&
+  FormatTok->is(TT_FatArrow)) {
+tryToParseChildBlock();
 break;
   }
 
@@ -1725,7 +1715,7 @@ bool UnwrappedLineParser::tryToParsePropertyAccessor() {
   // Try to parse the property accessor:
   // 
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties
   Tokens->setPosition(StoredPosition);
-  if (!IsTrivialPropertyAccessor && Style.BraceWrapping.AfterFunction == true)
+  if (!IsTrivialPropertyAccessor && Style.BraceWrapping.AfterFunction)
 addUnwrappedLine();
   nextToken();
   do {
@@ -1940,18 +1930,15 @@ bool UnwrappedLineParser::tryToParseBracedList() {
   return true;
 }
 
-bool UnwrappedLineParser::tryToParseCSharpLambda() {
-  // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType
-  // TT_FatArrow. They always start an expression or a child block if
-  // followed by a curly brace.
+bool UnwrappedLineParser::tryToParseChildBlock() {
+  assert(Style.isJavaScript() || Style.isCSharp());
+  assert(FormatTok->is(TT_FatArrow));
+  // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType TT_FatArrow.
+  // They always start an expression or a child block if followed by a curly
+  // brace.
   nextToken();
   if (FormatTok->isNot(tok::l_brace))
 return false;
-  // C# may break after => if the next character is a newline.
-  if (Style.BraceWrapping.AfterFunction) {
-// calling `addUnwrappedLine()` here causes odd parsing errors.
-FormatTok->MustBreakBefore = true;
-  }
   parseChildBlock();
   return true;
 }
@@ -1964,24 +1951,15 @@ bool UnwrappedLineParser::parseBracedList(bool 
ContinueOnSemicolons,
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
   // replace this by using parseAssignmentExpression() inside.
   do {
-if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
-  if (tryToParseCSharpLambda())
-continue;
+if (Style.isCSharp() && FormatTok->is(TT_FatArrow) &&
+tryToParseChildBlock())
+  continue;
 if (Style.isJavaScript()) {
   if (FormatTok->is(Keywords.kw_function) ||
   FormatTok->startsSequence(Keywords.kw_async, Keywords.kw_function)) {
 tryToParseJSFunction();
 continue;
   }
-  if (FormatTok->is(TT_FatArrow)) {
-nextToken();
-// Fat arrows can be followed by simple expressions or by child blocks
-// in curly braces.
-if (FormatTok->is(tok::l_brace)) {
-  parseChildBlock();
-  continue;
-}
-  }
   if (FormatTok->is(tok::l_brace)) {
 //

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

[Owen Pan 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 rGb9f6e09b850a: [clang-format][NFC] Handle wrapping after 
=> in mustBreakBefore() (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D115967?vs=395544&id=395773#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115967

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h

Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -138,7 +138,7 @@
   // https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/where-generic-type-constraint
   void parseCSharpGenericTypeConstraint();
   bool tryToParseLambda();
-  bool tryToParseCSharpLambda();
+  bool tryToParseChildBlock();
   bool tryToParseLambdaIntroducer();
   bool tryToParsePropertyAccessor();
   void tryToParseJSFunction();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1637,19 +1637,9 @@
   break;
 }
 case tok::equal:
-  // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType
-  // TT_FatArrow. They always start an expression or a child block if
-  // followed by a curly brace.
-  if (FormatTok->is(TT_FatArrow)) {
-nextToken();
-if (FormatTok->is(tok::l_brace)) {
-  // C# may break after => if the next character is a newline.
-  if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) {
-// calling `addUnwrappedLine()` here causes odd parsing errors.
-FormatTok->MustBreakBefore = true;
-  }
-  parseChildBlock();
-}
+  if ((Style.isJavaScript() || Style.isCSharp()) &&
+  FormatTok->is(TT_FatArrow)) {
+tryToParseChildBlock();
 break;
   }
 
@@ -1725,7 +1715,7 @@
   // Try to parse the property accessor:
   // https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties
   Tokens->setPosition(StoredPosition);
-  if (!IsTrivialPropertyAccessor && Style.BraceWrapping.AfterFunction == true)
+  if (!IsTrivialPropertyAccessor && Style.BraceWrapping.AfterFunction)
 addUnwrappedLine();
   nextToken();
   do {
@@ -1940,18 +1930,15 @@
   return true;
 }
 
-bool UnwrappedLineParser::tryToParseCSharpLambda() {
-  // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType
-  // TT_FatArrow. They always start an expression or a child block if
-  // followed by a curly brace.
+bool UnwrappedLineParser::tryToParseChildBlock() {
+  assert(Style.isJavaScript() || Style.isCSharp());
+  assert(FormatTok->is(TT_FatArrow));
+  // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType TT_FatArrow.
+  // They always start an expression or a child block if followed by a curly
+  // brace.
   nextToken();
   if (FormatTok->isNot(tok::l_brace))
 return false;
-  // C# may break after => if the next character is a newline.
-  if (Style.BraceWrapping.AfterFunction) {
-// calling `addUnwrappedLine()` here causes odd parsing errors.
-FormatTok->MustBreakBefore = true;
-  }
   parseChildBlock();
   return true;
 }
@@ -1964,24 +1951,15 @@
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
   // replace this by using parseAssignmentExpression() inside.
   do {
-if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
-  if (tryToParseCSharpLambda())
-continue;
+if (Style.isCSharp() && FormatTok->is(TT_FatArrow) &&
+tryToParseChildBlock())
+  continue;
 if (Style.isJavaScript()) {
   if (FormatTok->is(Keywords.kw_function) ||
   FormatTok->startsSequence(Keywords.kw_async, Keywords.kw_function)) {
 tryToParseJSFunction();
 continue;
   }
-  if (FormatTok->is(TT_FatArrow)) {
-nextToken();
-// Fat arrows can be followed by simple expressions or by child blocks
-// in curly braces.
-if (FormatTok->is(tok::l_brace)) {
-  parseChildBlock();
-  continue;
-}
-  }
   if (FormatTok->is(tok::l_brace)) {
 // Could be a method inside of a braced list `{a() { return 1; }}`.
 if (tryToParseBracedList())
@@ -1996,12 +1974,6 @@
   return !HasError;
 }
 switch (FormatTok->Tok.getKind()) {
-case tok::caret:
-  nextToken();
-  if (FormatTok->is(tok::l_brace)) {
-parseChildBlock();
-  }
-  break;
 case tok::l_square:
   if (Style.isCSharp())
 parseSquare();
@@ -2093,7 +2065,7 @@
   break;
 case tok::equal:
 

[PATCH] D113530: [wip] [analyzer] support use-after-free checking with parameter annotation

[Chris D'Angelo via Phabricator via cfe-commits]

chrisdangelo added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1405
 
-void MallocChecker::checkOwnershipAttr(const CallEvent &Call,
-   CheckerContext &C) const {
+static const FunctionDecl *functionDeclForCall(const CallEvent &Call,
+   CheckerContext &C) {

NoQ wrote:
> `Call.getDecl()` is intended to be the ultimate source of truth on this 
> subject.
Thank you. This function definition has been removed and the call-site has 
changed to...

```
  const Decl *D = Call.getDecl();
  const FunctionDecl *FD = dyn_cast_or_null(D);
  if (!FD)
return;
```

... I plan to push the above changes soon to Differential. The tests pass 
successfully using `ninja check-clang-analysis`.

Previously, `functionDeclForCall` mimicked the existing solution using `const 
FunctionDecl *FD = C.getCalleeDecl(CE);`. I'm not sure what that existing 
solution provided differently from `Call.getDecl()`.




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

https://reviews.llvm.org/D113530

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

[PATCH] D116090: [X86][MS-InlineAsm] Use exact conditions to recognize MS global variables

[Kan Shengchen via Phabricator via cfe-commits]

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

Thank you for the fix! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116090

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