[PATCH] D147969: Add InsertBraces to ChromiumStyle

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

RFC https://groups.google.com/g/llvm-dev/c/wka1Bnrd-aU?pli=1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147969

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

[clang] 86946eb - [clang] Add test for CWG1822

[Vlad Serebrennikov via cfe-commits]

Author: Vlad Serebrennikov
Date: 2023-04-11T11:08:22+03:00
New Revision: 86946ebb796c4ccde85b34aa52964e9aadabc692

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

LOG: [clang] Add test for CWG1822

[[https://wg21.link/p1787 | P1787]]: CWG1822 is resolved by specifying that the 
body of a lambda remains in the surrounding (function parameter) scope.
Wording: A parameter-declaration-clause P introduces a function parameter scope 
that includes P. <...> If P is associated with a lambda-declarator, its scope 
extends to the end of the compound-statement in the lambda-expression. 
([basic.scope.param])

Reviewed By: #clang-language-wg, shafik

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

Added: 


Modified: 
clang/test/CXX/drs/dr18xx.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index a2a1a5cae0741..55f79295934c8 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -50,6 +50,15 @@ namespace dr1815 { // dr1815: no
 #endif
 }
 
+namespace dr1822 { // dr1822: yes
+#if __cplusplus >= 201103L
+  int a;
+  auto x = [] (int a) {
+#pragma clang __debug dump a // CHECK: ParmVarDecl
+  };
+#endif
+}
+
 namespace dr1872 { // dr1872: 9
 #if __cplusplus >= 201103L
   template struct A : T {

diff  --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 4621e6e851cdf..6e234f63fc015 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -10739,7 +10739,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg1822";>1822
 CD6
 Lookup of parameter names in lambda-expressions
-Unknown
+Yes
   
   
 https://wg21.link/cwg1823";>1823



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

[PATCH] D147836: [clang] Add test for CWG1822

[Vlad Serebrennikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG86946ebb796c: [clang] Add test for CWG1822 (authored by 
Endill).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147836

Files:
  clang/test/CXX/drs/dr18xx.cpp
  clang/www/cxx_dr_status.html


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -10739,7 +10739,7 @@
 https://wg21.link/cwg1822";>1822
 CD6
 Lookup of parameter names in lambda-expressions
-Unknown
+Yes
   
   
 https://wg21.link/cwg1823";>1823
Index: clang/test/CXX/drs/dr18xx.cpp
===
--- clang/test/CXX/drs/dr18xx.cpp
+++ clang/test/CXX/drs/dr18xx.cpp
@@ -50,6 +50,15 @@
 #endif
 }
 
+namespace dr1822 { // dr1822: yes
+#if __cplusplus >= 201103L
+  int a;
+  auto x = [] (int a) {
+#pragma clang __debug dump a // CHECK: ParmVarDecl
+  };
+#endif
+}
+
 namespace dr1872 { // dr1872: 9
 #if __cplusplus >= 201103L
   template struct A : T {


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -10739,7 +10739,7 @@
 https://wg21.link/cwg1822";>1822
 CD6
 Lookup of parameter names in lambda-expressions
-Unknown
+Yes
   
   
 https://wg21.link/cwg1823";>1823
Index: clang/test/CXX/drs/dr18xx.cpp
===
--- clang/test/CXX/drs/dr18xx.cpp
+++ clang/test/CXX/drs/dr18xx.cpp
@@ -50,6 +50,15 @@
 #endif
 }
 
+namespace dr1822 { // dr1822: yes
+#if __cplusplus >= 201103L
+  int a;
+  auto x = [] (int a) {
+#pragma clang __debug dump a // CHECK: ParmVarDecl
+  };
+#endif
+}
+
 namespace dr1872 { // dr1872: 9
 #if __cplusplus >= 201103L
   template struct A : T {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145868: [clang][ASTImporter] Fix import of typedef with unnamed structures

[Donát Nagy via Phabricator via cfe-commits]

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

This commit eliminates crashes caused by a rare corner case (typedefs to types 
derived from unnamed structs) of the AST import procedure; but introduces some 
incorrect behavior in a rare sub-case of of this corner case (which is explored 
by the testcases added by this commit). I think this is a good trade (one 
surprising behavior is less bad than 10 crashes) and it's worth to merge this 
simple change in its current state. However, it would be important to continue 
this project and create a separate followup commit that fixes the remaining 
little issues in this area, because if we don't handle them now, they might 
cause unpleasant surprises later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145868

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

[PATCH] D147848: [clang] Add test for CWG2370

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added inline comments.



Comment at: clang/test/CXX/drs/dr23xx.cpp:182
+  typedef N::type N_type;
+  // FIXME: `type` should be searched for in N
+  // friend void N::g(type);

shafik wrote:
> The implementation seems to all accept this example: 
> https://godbolt.org/z/vE6bEP6xa
> 
> but the examples from the `p1787` have a decidely mixed conformance: 
> https://godbolt.org/z/dhq7oEKaY
> 
> but the `A::F(F)` you point out in your example clang does get wrong and gcc 
> does not. So at minimum please file bug reports against the examples that 
> clang does not get right from `p1787` and we need to dig into why your 
> example above seems to not the same since that is what you intended. 
Are you sure behavior is different for my `N::g(type)` when compared to 
`A::f(F)`? Clang is the only one to reject both examples: 
https://godbolt.org/z/MKb6fE8K5


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147848

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

[PATCH] D147920: [clang] Add test for CWG399

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

I think I haven't stressed it enough, but this whole test is copied from dr244, 
which is written by Richard.




Comment at: clang/test/CXX/drs/dr3xx.cpp:1492
+// This is technically ill-formed; G is looked up in 'N::' and is not 
found.
+// Rejecting this seems correct, but most compilers accept, so we do also.
+f.N::F::~G(); // expected-error {{qualified destructor name only found in 
lexical scope; omit the qualifier to find this type name by unqualified lookup}}

shafik wrote:
> You say we accept the next line but it has an `expected-error` on it?
It's an error because of `-pedantic-errors`. It's a warning by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147920

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

[PATCH] D147839: [clang] Add test for CWG2007

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill updated this revision to Diff 512365.
Endill added a comment.

Replace unary `&` with `__builtin_addressof()`. It prevents unnecessary 
template instantiation (presumably to find overloaded unary `&`), and make 
Clang compliant since 3.4


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147839

Files:
  clang/test/CXX/drs/dr20xx.cpp
  clang/www/cxx_dr_status.html


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -11849,7 +11849,7 @@
 https://wg21.link/cwg2007";>2007
 CD6
 Argument-dependent lookup for operator=
-Partial
+Clang 3.4
   
   
 https://wg21.link/cwg2008";>2008
Index: clang/test/CXX/drs/dr20xx.cpp
===
--- clang/test/CXX/drs/dr20xx.cpp
+++ clang/test/CXX/drs/dr20xx.cpp
@@ -10,14 +10,13 @@
 #define static_assert(...) _Static_assert(__VA_ARGS__)
 #endif
 
-namespace dr2007 { // dr2007: partial
+namespace dr2007 { // dr2007: 3.4
 template struct A { typename T::error e; };
 template struct B { };
 B > b1;
 B > b2 = b1;
 int a = b2[0]; // expected-error {{does not provide a subscript operator}}
-// FIXME: the following code shouldn't instantiate A.
-// int b = (&b2)->foo;
+int b = __builtin_addressof(b2)->foo; // expected-error {{no member}}
 }
 
 namespace dr2026 { // dr2026: 11


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -11849,7 +11849,7 @@
 https://wg21.link/cwg2007";>2007
 CD6
 Argument-dependent lookup for operator=
-Partial
+Clang 3.4
   
   
 https://wg21.link/cwg2008";>2008
Index: clang/test/CXX/drs/dr20xx.cpp
===
--- clang/test/CXX/drs/dr20xx.cpp
+++ clang/test/CXX/drs/dr20xx.cpp
@@ -10,14 +10,13 @@
 #define static_assert(...) _Static_assert(__VA_ARGS__)
 #endif
 
-namespace dr2007 { // dr2007: partial
+namespace dr2007 { // dr2007: 3.4
 template struct A { typename T::error e; };
 template struct B { };
 B > b1;
 B > b2 = b1;
 int a = b2[0]; // expected-error {{does not provide a subscript operator}}
-// FIXME: the following code shouldn't instantiate A.
-// int b = (&b2)->foo;
+int b = __builtin_addressof(b2)->foo; // expected-error {{no member}}
 }
 
 namespace dr2026 { // dr2026: 11
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.

LGTM! Most of the diagnostic messages are short but precise. I like this very 
much. Well done!

Balázs actually tested the change on open source projects, but accidentally 
uploaded it to our internal server, so I have seen them, and they look good 
from the diagnostic message standpoint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144003

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

[PATCH] D147848: [clang] Add test for CWG2370

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

Bug report filed: https://github.com/llvm/llvm-project/issues/62061


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147848

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

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 512377.
sammccall marked an inline comment as done.
sammccall added a comment.

use llvm::printHTMLEscaped


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

Files:
  clang/include/clang/Analysis/FlowSensitive/Logger.h
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.css
  clang/lib/Analysis/FlowSensitive/HTMLLogger.html
  clang/lib/Analysis/FlowSensitive/HTMLLogger.js
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
  clang/utils/bundle_resources.py

Index: clang/utils/bundle_resources.py
===
--- /dev/null
+++ clang/utils/bundle_resources.py
@@ -0,0 +1,29 @@
+#!/usr/bin/env python3
+
+#===- bundle_resources.py - Generate string constants with file contents. ===
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===
+
+# Usage: bundle-resources.py foo.inc a.js path/b.css ...
+# Produces foo.inc containing:
+#   const char a_js[] = "...";
+#   const char b_css[] = "...";
+import os
+import sys
+
+outfile = sys.argv[1]
+infiles = sys.argv[2:]
+
+with open(outfile, 'w') as out:
+  for filename in infiles:
+varname = os.path.basename(filename).replace('.', '_')
+out.write("const char " + varname + "[] = \n");
+# MSVC limits each chunk of string to 2k, so split by lines.
+# The overall limit is 64k, which ought to be enough for anyone.
+for line in open(filename).read().split('\n'):
+  out.write('  R"x(' + line + ')x" "\\n"\n' )
+out.write('  ;\n');
Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -9,6 +9,7 @@
 
 namespace clang::dataflow::test {
 namespace {
+using testing::HasSubstr;
 
 struct TestLattice {
   int Elements = 0;
@@ -83,19 +84,24 @@
   void logText(llvm::StringRef Text) override { OS << Text << "\n"; }
 };
 
-TEST(LoggerTest, Sequence) {
+AnalysisInputs makeInputs() {
   const char *Code = R"cpp(
 int target(bool b, int p, int q) {
   return b ? p : q;
 }
 )cpp";
+  static const std::vector Args = {
+  "-fsyntax-only", "-fno-delayed-template-parsing", "-std=c++17"};
 
   auto Inputs = AnalysisInputs(
   Code, ast_matchers::hasName("target"),
   [](ASTContext &C, Environment &) { return TestAnalysis(C); });
-  std::vector Args = {
-  "-fsyntax-only", "-fno-delayed-template-parsing", "-std=c++17"};
   Inputs.ASTBuildArgs = Args;
+  return Inputs;
+}
+
+TEST(LoggerTest, Sequence) {
+  auto Inputs = makeInputs();
   std::string Log;
   TestLogger Logger(Log);
   Inputs.BuiltinOptions.Log = &Logger;
@@ -148,5 +154,29 @@
 )");
 }
 
+TEST(LoggerTest, HTML) {
+  auto Inputs = makeInputs();
+  std::vector Logs;
+  auto Logger = Logger::html([&]() {
+Logs.emplace_back();
+return std::make_unique(Logs.back());
+  });
+  Inputs.BuiltinOptions.Log = Logger.get();
+
+  ASSERT_THAT_ERROR(checkDataflow(std::move(Inputs),
+[](const AnalysisOutputs &) {}),
+llvm::Succeeded());
+
+  // Simple smoke tests: we can't meaningfully test the behavior.
+  ASSERT_THAT(Logs, testing::SizeIs(1));
+  EXPECT_THAT(Logs[0], HasSubstr("function updateSelection")) << "embeds JS";
+  EXPECT_THAT(Logs[0], HasSubstr("html {")) << "embeds CSS";
+  EXPECT_THAT(Logs[0], HasSubstr("b (ImplicitCastExpr")) << "has CFG elements";
+  EXPECT_THAT(Logs[0], HasSubstr("\"B3:1_B3.1\":"))
+  << "has analysis point state";
+  EXPECT_THAT(Logs[0], HasSubstr("transferBranch(0)")) << "has analysis logs";
+  EXPECT_THAT(Logs[0], HasSubstr("LocToVal")) << "has built-in lattice dump";
+}
+
 } // namespace
 } // namespace clang::dataflow::test
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.js
===
--- /dev/null
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.js
@@ -0,0 +1,213 @@
+//===-- HTMLLogger.js -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// Based on selected objects, hide/show sections & populate data from templates.
+//
+// For example, if the selection is {bb="BB4", elt=

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:77
+
+void escape(char C, llvm::raw_ostream &OS) {
+  switch (C) {

xazax.hun wrote:
> We already sort of have a way to escape HTML here: 
> https://github.com/llvm/llvm-project/blob/132f1d31fd66c30baf9773bf8f37b36a40fa7039/clang/include/clang/Rewrite/Core/HTMLRewrite.h#L60
> 
> I think we should reuse that (and potentially extend it if it does not fit 
> our needs).
Yeah, we have this in a bunch of places.

The canonical one looks like llvm::printHTMLEscaped in Support, switched to 
that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:58
 using namespace sema;
-
+bool isEnumClass = false;
 Sema::DeclGroupPtrTy Sema::ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType) {

This being global could lead to issues where one overwrites the other. I think 
a better approach would be to create another version of 
`GetDiagnosticTypeSpecifierID` that takes the DeclSpec, then this won't be 
needed.



Comment at: clang/lib/Sema/SemaDecl.cpp:5305-5310
+   if (TypeSpecType == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+isEnumClass = true;
+}
+  }

if you make a new `GetDiagnosticTypeSpecifierID` then you can check these 
conditions in that function instead of there and return 5 if they are all true, 
otherwise return `GetDiagnosticTypeSpecifierID(TypeSpecType)`.



Comment at: clang/lib/Sema/SemaDecl.cpp:5313-5316
 << AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
 << AL << GetDiagnosticTypeSpecifierID(TypeSpecType);

Then use the new `GetDiagnosticTypeSpecifierID` here, passing `DS` instead of 
`TypeSpecType`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added reviewers: mboehme, aaron.ballman.
samtebbs added a comment.

Looping in some people who have worked on and reviewed patches in this area for 
some experienced eyes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

[Sunil K via Phabricator via cfe-commits]

koops updated this revision to Diff 512381.
koops added a comment.

Moving the code to SemaOpenMP.cpp from CodeGen.


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

https://reviews.llvm.org/D144634

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/loop_bind_codegen.cpp

Index: clang/test/OpenMP/loop_bind_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/loop_bind_codegen.cpp
@@ -0,0 +1,132 @@
+// Copyright 2020 Hewlett Packard Enterprise Development LP
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ %s -emit-llvm -o - | FileCheck %s 
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+
+/*
+#include 
+#include 
+#include 
+#include 
+*/
+
+#define NNN 50
+int aaa[NNN];
+
+void parallel_loop() {
+  #pragma omp parallel
+  {
+ #pragma omp loop bind(parallel)
+ for (int j = 0 ; j < NNN ; j++) {
+   aaa[j] = j*NNN;
+ }
+   }
+}
+
+
+void teams_loop() {
+  #pragma omp teams
+  {
+ #pragma omp loop bind(teams)
+ for (int j = 0 ; j < NNN ; j++) {
+   aaa[j] = j*NNN;
+ }
+   }
+}
+
+void thread_loop() {
+  #pragma omp parallel
+  {
+ #pragma omp loop bind(thread)
+ for (int j = 0 ; j < NNN ; j++) {
+   aaa[j] = j*NNN;
+ }
+   }
+}
+
+void thread_loop2() {
+  #pragma omp loop bind(thread)
+  for (int j = 0 ; j < NNN ; j++) {
+aaa[j] = j*NNN;
+  }
+}
+
+int main() {
+  parallel_loop();
+  teams_loop();
+  thread_loop();
+  thread_loop2();
+
+  return 0;
+}
+#endif
+// CHECK-LABEL: define {{[^@]+}}@_Z13parallel_loopv
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @3, i32 0, ptr @.omp_outlined.)
+// CHECK-NEXT: ret void
+// CHECK-LABEL: define {{[^@]+}}@.omp_outlined
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[DOTGLOBAL_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTBOUND_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTOMP_IV:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[TMP:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_LB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_UB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_STRIDE:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_IS_LAST:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK:   call void @__kmpc_for_static_init_4(ptr @1, i32 %1, i32 34, ptr [[DOTOMP_IS_LAST]], ptr [[DOTOMP_LB]], ptr [[DOTOMP_UB]], ptr [[DOTOMP_STRIDE]], i32 1, i32 1)
+// CHECK-LABEL: cond.true:
+// CHECK-NEXT:br label [[COND_END:%.*]]
+// CHECK-LABEL: cond.false:
+// CHECK-NEXT:[[TMP11:%.*]] = load i32, ptr [[DOTOMP_UB]], align 4
+// CHECK-NEXT:br label [[COND_END]]
+// CHECK:   omp.inner.for.cond:
+// CHECK-NEXT:[[TMP13:%.*]] = load i32, ptr [[DOTOMP_IV]], align 4
+// CHECK:   omp.inner.for.body:
+// CHECK:   omp.loop.exit:
+// CHECK-NEXT:call void @__kmpc_for_static_fini(ptr @1, i32 %1)
+// CHECK-NEXT:call void @__kmpc_barrier(ptr @2, i32 %1)
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@_Z10teams_loopv
+// CHECK-NEXT:  entry:
+// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_teams(ptr @3, i32 0, ptr @.omp_outlined..1)
+// CHECK-NEXT: ret void
+//
+// CHECK-LABEL: define {{[^@]+}}@.omp_outlined
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[DOTGLOBAL_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTBOUND_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTOMP_IV:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[TMP:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_LB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_UB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_STRIDE:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_IS_LAST:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK:   call void @__kmpc_for_static_init_4(ptr @4, i32 %1, i32 92, ptr [[DOTOMP_IS_LAST]], ptr [[DOTOMP_LB]], ptr [[DOTOMP_UB]], ptr [[DOTOMP_STRIDE]], i32 1, i32 1)
+// CHECK-LABEL: cond.true:
+// CHECK-NEXT:br label [[COND_END:%.*]]
+// CHECK-LABEL: cond.false:
+// CHECK-NEXT:[[TMP11:%.*]] = load i32, ptr [[DOTOMP_UB]], align 4
+// CHECK-NEXT:br label [[COND_END]]
+// CHECK:   omp.inner.for.cond:
+// CHECK-NEXT:[[TMP13:%.*]] = load i32, ptr [[DOTOMP_IV]], align 4
+// CHECK:   omp.inner.for.body:
+// CHECK:   omp.loop.exit:
+// CHECK-NEXT:call void @__kmpc_for_static_fini(ptr @4, i32 %1)
+// CHECK-NEXT:ret void
+//
+// CHECK-LABEL: define {{[^@]+}}@_Z11thread_loopv()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__

[PATCH] D148004: [clang][dataflow][NFC] Remove unused parameter from `insertIfGlobal()`.

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148004

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -156,7 +156,6 @@
 
 /// Initializes a global storage value.
 static void insertIfGlobal(const Decl &D,
-   llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
   if (auto *V = dyn_cast(&D))
 if (V->hasGlobalStorage())
@@ -166,7 +165,7 @@
 static void getFieldsAndGlobalVars(const Decl &D,
llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
-  insertIfGlobal(D, Fields, Vars);
+  insertIfGlobal(D, Vars);
   if (const auto *Decomp = dyn_cast(&D))
 for (const auto *B : Decomp->bindings())
   if (auto *ME = dyn_cast_or_null(B->getBinding()))
@@ -191,11 +190,11 @@
   for (auto *D : DS->getDeclGroup())
   getFieldsAndGlobalVars(*D, Fields, Vars);
   } else if (auto *E = dyn_cast(&S)) {
-insertIfGlobal(*E->getDecl(), Fields, Vars);
+insertIfGlobal(*E->getDecl(), Vars);
   } else if (auto *E = dyn_cast(&S)) {
 // FIXME: should we be using `E->getFoundDecl()`?
 const ValueDecl *VD = E->getMemberDecl();
-insertIfGlobal(*VD, Fields, Vars);
+insertIfGlobal(*VD, Vars);
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   }


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -156,7 +156,6 @@
 
 /// Initializes a global storage value.
 static void insertIfGlobal(const Decl &D,
-   llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
   if (auto *V = dyn_cast(&D))
 if (V->hasGlobalStorage())
@@ -166,7 +165,7 @@
 static void getFieldsAndGlobalVars(const Decl &D,
llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
-  insertIfGlobal(D, Fields, Vars);
+  insertIfGlobal(D, Vars);
   if (const auto *Decomp = dyn_cast(&D))
 for (const auto *B : Decomp->bindings())
   if (auto *ME = dyn_cast_or_null(B->getBinding()))
@@ -191,11 +190,11 @@
   for (auto *D : DS->getDeclGroup())
   getFieldsAndGlobalVars(*D, Fields, Vars);
   } else if (auto *E = dyn_cast(&S)) {
-insertIfGlobal(*E->getDecl(), Fields, Vars);
+insertIfGlobal(*E->getDecl(), Vars);
   } else if (auto *E = dyn_cast(&S)) {
 // FIXME: should we be using `E->getFoundDecl()`?
 const ValueDecl *VD = E->getMemberDecl();
-insertIfGlobal(*VD, Fields, Vars);
+insertIfGlobal(*VD, Vars);
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147733: Set rounding_mode to tonearest in presence of a #pragma STDC FENV_ACCESS OFF.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: clang/test/CodeGen/pragma-fenv_access.c:4
 // RUN: %clang_cc1 -fexperimental-strict-floating-point 
-ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - 
-fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN %clang_cc1 -fexperimental-strict-floating-point -frounding-math 
-ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - 
-fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT %s
 // RUN: %clang_cc1 -fexperimental-strict-floating-point -triple 
%itanium_abi_triple -emit-llvm %s -o - | FileCheck 
--check-prefixes=CHECK,DEFAULT %s

andrew.w.kaylor wrote:
> Why doesn't this case use STRICT-RND? round.tonearest, but I don't understand 
> why.
It does. I just forgot to add the ":" after the RUN.


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

https://reviews.llvm.org/D147733

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

[PATCH] D147733: Set rounding_mode to tonearest in presence of a #pragma STDC FENV_ACCESS OFF.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 512388.

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

https://reviews.llvm.org/D147733

Files:
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/pragma-fenv_access.c


Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- clang/test/CodeGen/pragma-fenv_access.c
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -1,13 +1,17 @@
 // RUN: %clang_cc1 -fexperimental-strict-floating-point 
-ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | 
FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -frounding-math 
-ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | 
FileCheck --check-prefixes=CHECK,STRICT-RND %s
 // RUN: %clang_cc1 -fexperimental-strict-floating-point 
-ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - 
-fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -frounding-math 
-ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - 
-fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT-RND %s
 // RUN: %clang_cc1 -fexperimental-strict-floating-point -triple 
%itanium_abi_triple -emit-llvm %s -o - | FileCheck 
--check-prefixes=CHECK,DEFAULT %s
-
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -frounding-math 
-triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck 
--check-prefixes=CHECK,DEFAULT-RND %s
 
 float func_00(float x, float y) {
   return x + y;
 }
 // CHECK-LABEL: @func_00
 // STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// STRICT-RND: call float @llvm.experimental.constrained.fadd.f32(float 
{{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// DEFAULT-RND: call float @llvm.experimental.constrained.fadd.f32(float 
{{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
 // DEFAULT: fadd float
 
 
@@ -224,3 +228,17 @@
 // STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // DEFAULT: fadd float
 
+#pragma STDC FENV_ACCESS ON
+float func_19(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_19
+// STRICT:  call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+
+#pragma STDC FENV_ACCESS OFF
+float func_20(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_20
+// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// DEFAULT: fadd float
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1336,6 +1336,7 @@
   Diag(Loc, diag::err_pragma_fenv_requires_precise);
   }
   NewFPFeatures.setAllowFEnvAccessOverride(IsEnabled);
+  NewFPFeatures.setRoundingMathOverride(IsEnabled);
   FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
 }


Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- clang/test/CodeGen/pragma-fenv_access.c
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -1,13 +1,17 @@
 // RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -frounding-math -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,STRICT-RND %s
 // RUN: %clang_cc1 -fexperimental-strict-floating-point -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - -fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT %s
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -frounding-math -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - -fms-extensions -DMS | FileCheck --check-prefixes=CHECK,STRICT-RND %s
 // RUN: %clang_cc1 -fexperimental-strict-floating-point -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,DEFAULT %s
-
+// RUN: %clang_cc1 -fexperimental-strict-floating-point -frounding-math -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,DEFAULT-RND %s
 
 float func_00(float x, float y) {
   return x + y;
 }
 // CHECK-LABEL: @func_00
 // STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// STRICT-RND: call float @llvm

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added a comment.
Herald added a subscriber: bviyer.

ping for reviews. Thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

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

[PATCH] D148006: [clang][dataflow] Associate `FunctionToPointerDecay` nodes with a value.

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

To ensure that we have a pointee for the `PointerValue`, we also create
storage locations for `FunctionDecl`s referenced in the function under analysis.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148006

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5170,4 +5170,43 @@
   });
 }
 
+TEST(TransferTest, FunctionToPointerDecayHasValue) {
+  std::string Code = R"(
+struct A { static void static_member_func(); };
+void target() {
+  // To check that we're treating function-to-pointer decay correctly,
+  // create two pointers, then verify they refer to the same storage
+  // location.
+  // We need to do the test this way because even if an initializer (in this
+  // case, the function-to-pointer decay) does not create a value, we still
+  // create a value for the variable.
+  void (*non_member_p1)() = target;
+  void (*non_member_p2)() = target;
+
+  // Do the same thing but for a static member function.
+  void (*member_p1)() = A::static_member_func;
+  void (*member_p2)() = A::static_member_func;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+auto &NonMemberP1 =
+getValueForDecl(ASTCtx, Env, "non_member_p1");
+auto &NonMemberP2 =
+getValueForDecl(ASTCtx, Env, "non_member_p2");
+EXPECT_EQ(&NonMemberP1.getPointeeLoc(), &NonMemberP2.getPointeeLoc());
+
+auto &MemberP1 =
+getValueForDecl(ASTCtx, Env, "member_p1");
+auto &MemberP2 =
+getValueForDecl(ASTCtx, Env, "member_p2");
+EXPECT_EQ(&MemberP1.getPointeeLoc(), &MemberP2.getPointeeLoc());
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -403,6 +403,18 @@
   Env.setValue(Loc, NullPointerVal);
   break;
 }
+case CK_FunctionToPointerDecay: {
+  StorageLocation *PointeeLoc =
+  Env.getStorageLocation(*SubExpr, SkipPast::Reference);
+  if (PointeeLoc == nullptr)
+break;
+
+  auto &PointerLoc = Env.createStorageLocation(*S);
+  auto &PointerVal = Env.create(*PointeeLoc);
+  Env.setStorageLocation(*S, PointerLoc);
+  Env.setValue(PointerLoc, PointerVal);
+  break;
+}
 default:
   break;
 }
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -163,10 +163,19 @@
   Vars.insert(V);
 }
 
-static void getFieldsAndGlobalVars(const Decl &D,
-   llvm::DenseSet &Fields,
-   llvm::DenseSet &Vars) {
+static void insertIfFunction(const Decl &D,
+ llvm::DenseSet &Funcs) {
+  if (auto *FD = dyn_cast(&D))
+Funcs.insert(FD);
+}
+
+static void
+getFieldsGlobalsAndFuncs(const Decl &D,
+ llvm::DenseSet &Fields,
+ llvm::DenseSet &Vars,
+ llvm::DenseSet &Funcs) {
   insertIfGlobal(D, Fields, Vars);
+  insertIfFunction(D, Funcs);
   if (const auto *Decomp = dyn_cast(&D))
 for (const auto *B : Decomp->bindings())
   if (auto *ME = dyn_cast_or_null(B->getBinding()))
@@ -175,27 +184,32 @@
   Fields.insert(FD);
 }
 
-/// Traverses `S` and inserts into `Vars` any global storage values that are
-/// declared in or referenced from sub-statements.
-static void getFieldsAndGlobalVars(const Stmt &S,
-   llvm::DenseSet &Fields,
-   llvm::DenseSet &Vars) {
+/// Traverses `S` and inserts into `Fields`, `Vars` and `Funcs` any fields,
+/// global variables and functions that are declared in or referenced from
+/// sub-statements.
+static void
+getFieldsGlobalsAndFuncs(const Stmt &S,
+ llvm::DenseSet &Fields,
+ l

[PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Michael Buch via Phabricator via cfe-commits]

Michael137 added subscribers: aprantl, dblaikie.
Michael137 added inline comments.



Comment at: lldb/test/API/lang/cpp/no_unique_address/main.cpp:38
+ long v = 42;
+} _f3;
+

I haven't checked the reworked logic yet, but it still crashes on this:

```
self.expect_expr("_f3")
```

I'm leaning towards not trying to support older compilers if it gets too 
complicated. Proposing a `DW_AT_no_unique_address` seems like the best option 
to me since that's pretty trivial to implement and requires almost no support 
on the LLDB side.

CC: @dblaikie @aprantl (since both were part of the [[ 
https://www.mail-archive.com/dwarf-discuss@lists.dwarfstd.org/msg00876.html | 
original discussion ]])


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[PATCH] D147698: [clang][dataflow] Add support for new and delete expressions.

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

@gribozavr2 Just to make sure I understand you correctly here (before I make 
any changes to the code): IIUC you recommend simply doing nothing on a delete 
expression? (Your arguments for this make sense to me, just want to 
double-check I understood correctly.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147698

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

[PATCH] D147986: [RISCV] Print a better error message when a rv32 CPU is used on rv64 and vice versa.

[Alex Bradbury via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147986

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

[clang] 991c7e1 - [clang][dataflow][NFC] Remove unused parameter from `insertIfGlobal()`.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-04-11T11:31:56Z
New Revision: 991c7e11728f6f6372e5dc865e3a4c0636a575ea

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

LOG: [clang][dataflow][NFC] Remove unused parameter from `insertIfGlobal()`.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 6a6343b5f169b..680036b6a5b39 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -156,7 +156,6 @@ static Value &widenDistinctValues(QualType Type, Value 
&Prev,
 
 /// Initializes a global storage value.
 static void insertIfGlobal(const Decl &D,
-   llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
   if (auto *V = dyn_cast(&D))
 if (V->hasGlobalStorage())
@@ -166,7 +165,7 @@ static void insertIfGlobal(const Decl &D,
 static void getFieldsAndGlobalVars(const Decl &D,
llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
-  insertIfGlobal(D, Fields, Vars);
+  insertIfGlobal(D, Vars);
   if (const auto *Decomp = dyn_cast(&D))
 for (const auto *B : Decomp->bindings())
   if (auto *ME = dyn_cast_or_null(B->getBinding()))
@@ -191,11 +190,11 @@ static void getFieldsAndGlobalVars(const Stmt &S,
   for (auto *D : DS->getDeclGroup())
   getFieldsAndGlobalVars(*D, Fields, Vars);
   } else if (auto *E = dyn_cast(&S)) {
-insertIfGlobal(*E->getDecl(), Fields, Vars);
+insertIfGlobal(*E->getDecl(), Vars);
   } else if (auto *E = dyn_cast(&S)) {
 // FIXME: should we be using `E->getFoundDecl()`?
 const ValueDecl *VD = E->getMemberDecl();
-insertIfGlobal(*VD, Fields, Vars);
+insertIfGlobal(*VD, Vars);
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   }



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

[PATCH] D148004: [clang][dataflow][NFC] Remove unused parameter from `insertIfGlobal()`.

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG991c7e11728f: [clang][dataflow][NFC] Remove unused parameter 
from `insertIfGlobal()`. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148004

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -156,7 +156,6 @@
 
 /// Initializes a global storage value.
 static void insertIfGlobal(const Decl &D,
-   llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
   if (auto *V = dyn_cast(&D))
 if (V->hasGlobalStorage())
@@ -166,7 +165,7 @@
 static void getFieldsAndGlobalVars(const Decl &D,
llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
-  insertIfGlobal(D, Fields, Vars);
+  insertIfGlobal(D, Vars);
   if (const auto *Decomp = dyn_cast(&D))
 for (const auto *B : Decomp->bindings())
   if (auto *ME = dyn_cast_or_null(B->getBinding()))
@@ -191,11 +190,11 @@
   for (auto *D : DS->getDeclGroup())
   getFieldsAndGlobalVars(*D, Fields, Vars);
   } else if (auto *E = dyn_cast(&S)) {
-insertIfGlobal(*E->getDecl(), Fields, Vars);
+insertIfGlobal(*E->getDecl(), Vars);
   } else if (auto *E = dyn_cast(&S)) {
 // FIXME: should we be using `E->getFoundDecl()`?
 const ValueDecl *VD = E->getMemberDecl();
-insertIfGlobal(*VD, Fields, Vars);
+insertIfGlobal(*VD, Vars);
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   }


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -156,7 +156,6 @@
 
 /// Initializes a global storage value.
 static void insertIfGlobal(const Decl &D,
-   llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
   if (auto *V = dyn_cast(&D))
 if (V->hasGlobalStorage())
@@ -166,7 +165,7 @@
 static void getFieldsAndGlobalVars(const Decl &D,
llvm::DenseSet &Fields,
llvm::DenseSet &Vars) {
-  insertIfGlobal(D, Fields, Vars);
+  insertIfGlobal(D, Vars);
   if (const auto *Decomp = dyn_cast(&D))
 for (const auto *B : Decomp->bindings())
   if (auto *ME = dyn_cast_or_null(B->getBinding()))
@@ -191,11 +190,11 @@
   for (auto *D : DS->getDeclGroup())
   getFieldsAndGlobalVars(*D, Fields, Vars);
   } else if (auto *E = dyn_cast(&S)) {
-insertIfGlobal(*E->getDecl(), Fields, Vars);
+insertIfGlobal(*E->getDecl(), Vars);
   } else if (auto *E = dyn_cast(&S)) {
 // FIXME: should we be using `E->getFoundDecl()`?
 const ValueDecl *VD = E->getMemberDecl();
-insertIfGlobal(*VD, Fields, Vars);
+insertIfGlobal(*VD, Vars);
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147989: [clang] Fix Attribute Placement

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this!

The changes are missing test coverage; please be sure to add that, along with a 
release note about the fix. I think there's likely more work to be done here as 
well, considering this behavior: https://godbolt.org/z/sdK3Gef75 (notice the 
suggested location for the fix-it -- that also needs to be fixed for this)




Comment at: clang/include/clang/Sema/DeclSpec.h:274
   };
-
   // Import type specifier type enumeration and constants.

Spurious whitespace change can be removed.



Comment at: clang/lib/Sema/SemaDecl.cpp:58
 using namespace sema;
-
+bool isEnumClass = false;
 Sema::DeclGroupPtrTy Sema::ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType) {

samtebbs wrote:
> This being global could lead to issues where one overwrites the other. I 
> think a better approach would be to create another version of 
> `GetDiagnosticTypeSpecifierID` that takes the DeclSpec, then this won't be 
> needed.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147978: [RISCV] Remove getCPUFeaturesExceptStdExt.

[Alex Bradbury via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147978

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

[PATCH] D147901: [NFC][CLANG][API] Fix coverity remarks about large copies by values

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:138
   APIRecord(RecordKind Kind, StringRef USR, StringRef Name,
-PresumedLoc Location, AvailabilitySet Availabilities,
+PresumedLoc Location, const AvailabilitySet &Availabilities,
 LinkageInfo Linkage, const DocComment &Comment,

RKSimon wrote:
> aaron.ballman wrote:
> > A lot of these changes look to be regressions, so I think Coverity is 
> > incorrect to flag these. The old code is passing an `AvailabilitySet` by 
> > value because it's doing a move operation on initialization: 
> > `Availabilities(std::move(Availabilities))`. Making this into a const 
> > reference defeats that optimization because you can't steal resources from 
> > a const object (so this turns a move into a copy).
> > 
> > You should look through the rest of the patch for similar problematic 
> > changes.
> I've never been sure whether coverity is being particularly poor at 
> recognising the std::move pattern or particularly smart at realising it can't 
> occur for some reason.
Yeah this certainly doesn't look right, since the new version tries to move 
from the const reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147901

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

[PATCH] D147935: [RISCV] Add SiFive extension support

[Alex Bradbury via Phabricator via cfe-commits]

asb added inline comments.



Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:465
   RVV_REQ_FullMultiply = 1 << 1,
+  RVV_REQ_xsfvcp = 1 << 2,
 

Nit: It would better match the surrounding capitalisation to call this 
RVV_REQ_Xsfvcp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147935

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

[PATCH] D147941: [Flang][Driver][OpenMP] Enable flags for filtering of offloading passes in flang

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 512399.
skatrak added a comment.

Address reviewer's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147941

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/omp-driver-offload.f90
  flang/test/Driver/omp-frontend-forwarding.f90


Index: flang/test/Driver/omp-driver-offload.f90
===
--- flang/test/Driver/omp-driver-offload.f90
+++ flang/test/Driver/omp-driver-offload.f90
@@ -7,6 +7,42 @@
 ! CHECK-OPENMP: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}}.f90"
 ! CHECK-OPENMP-NOT: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} 
"-fopenmp-is-device" {{.*}}.f90"
 
+! Test regular -fopenmp with offload, and invocation filtering options
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-HOST-AND-DEVICE
+
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 
--offload-host-device \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-HOST-AND-DEVICE
+
+! OFFLOAD-HOST-AND-DEVICE: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"aarch64-unknown-linux-gnu"
+! OFFLOAD-HOST-AND-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"amdgcn-amd-amdhsa"
+! OFFLOAD-HOST-AND-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"nvptx64-nvidia-cuda"
+! OFFLOAD-HOST-AND-DEVICE: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"aarch64-unknown-linux-gnu"
+
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 --offload-host-only 
\
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-HOST
+
+! OFFLOAD-HOST: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"aarch64-unknown-linux-gnu"
+! OFFLOAD-HOST-NOT: "-triple" "amdgcn-amd-amdhsa"
+! OFFLOAD-HOST-NOT: "-triple" "nvptx64-nvidia-cuda"
+! OFFLOAD-HOST-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"aarch64-unknown-linux-gnu"
+
+! RUN: %flang -S -### %s -o %t 2>&1 \
+! RUN: -fopenmp --offload-arch=gfx90a --offload-arch=sm_70 
--offload-device-only \
+! RUN: --target=aarch64-unknown-linux-gnu \
+! RUN:   | FileCheck %s --check-prefix=OFFLOAD-DEVICE
+
+! OFFLOAD-DEVICE: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"aarch64-unknown-linux-gnu"
+! OFFLOAD-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"amdgcn-amd-amdhsa"
+! OFFLOAD-DEVICE-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"nvptx64-nvidia-cuda"
+! OFFLOAD-DEVICE-NOT: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"aarch64-unknown-linux-gnu"
+
 ! Test regular -fopenmp with offload for basic fopenmp-is-device flag addition 
and correct fopenmp 
 ! RUN: %flang -### -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa %s 2>&1 | 
FileCheck --check-prefixes=CHECK-OPENMP-IS-DEVICE %s
 ! CHECK-OPENMP-IS-DEVICE: "{{[^"]*}}flang-new" "-fc1" {{.*}} "-fopenmp" {{.*}} 
"-fopenmp-is-device" {{.*}}.f90"
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -65,6 +65,9 @@
 ! HELP-NEXT: -mmlir  Additional arguments to forward to MLIR's 
option processing
 ! HELP-NEXT: -module-dir   Put MODULE files in 
 ! HELP-NEXT: -nocpp Disable predefined and command line 
preprocessor macros
+! HELP-NEXT: --offload-device-only   Only compile for the offloading device.
+! HELP-NEXT: --offload-host-device   Only compile for the offloading host.
+! HELP-NEXT: --offload-host-only Only compile for the offloading host.
 ! HELP-NEXT: -o   Write output to 
 ! HELP-NEXT: -pedantic  Warn on language extensions
 ! HELP-NEXT: -print-effective-triple Print the effective target triple
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -69,6 +69,9 @@
 ! CHECK-NEXT: -mmlir  Additional arguments to forward to MLIR's 
option processing
 ! CHECK-NEXT: -module-dir   Put MODULE files in 
 ! CHECK-NEXT: -nocpp Disable predefined and command line 
preprocessor macros
+! CHECK-NEXT: --offload-device-only   Only compile for the offloading device.
+! CHECK-NEXT: --offload-host-device   Only compile for the offloading host.
+! CHECK-NEXT: --offload-host-only Only compile for the offloading host.
 ! CHECK-NEXT: -o  Write output to 
 ! CHECK-NEXT: -pedantic  Warn on language extensions
 ! CHECK-NEXT: -print-effective-triple Print the effective target triple
Index: clang/include/clang/Driver/Options.td
===

[PATCH] D147989: [clang] Fix Attribute Placement

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

In D147989#4257539 , @samtebbs wrote:

> Looping in some people who have worked on and reviewed patches in this area 
> for some experienced eyes.

Nothing else to add beyond what @aaron.ballman has noted.

Thanks for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147941: [Flang][Driver][OpenMP] Enable options for selecting offloading phases in flang

[Sergio Afonso via Phabricator via cfe-commits]

skatrak marked 4 inline comments as done.
skatrak added a comment.

In D147941#4255622 , @awarzynski 
wrote:

> In the context of LLVM, I would normally associate "pass" with something 
> else. I'm not that familiar with offloading, so perhaps that's the language 
> that people use? Personally, in the context of the driver I'd normally use 
> the term "phase" rather than "pass".
>
> This patch makes sense, though I'd like the following to be addressed before 
> landing this:
>
> 1. Where's the logic that implements what `--offload-host-device`? It looks 
> like this is already implemented elsewhere and this patch simply "unlocks" 
> (rather than "implements") this option for Flang.
> 2. The name of "omp-frontend-forwarding.f90" is very misleading. "Forwarding" 
> would mean `flang-new --offload-host-only %s` --> `flang-new -fc1 
> --offload-host-only %s`, but that's not what's happening here, is it?
>
> For 1. you could just update the summary, and for 2. I suggest renaming 
> "omp-frontend-forwarding.f90" as e.g. "omp-compiler-flag-expansion.f90". WDYT?

I updated the patch title and summary, and renamed the test file to something 
that I think represents better the contents of the test. Thank you again for 
the suggestions, let me know if there are any other improvements to make before 
landing this patch.




Comment at: clang/include/clang/Driver/Options.td:2738
   HelpText<"Don't Use the new driver for offloading compilation.">;
-def offload_device_only : Flag<["--"], "offload-device-only">,
+def offload_device_only : Flag<["--"], "offload-device-only">, 
Flags<[CoreOption, FlangOption]>,
   HelpText<"Only compile for the offloading device.">;

awarzynski wrote:
> Why is `CoreOption` needed here? Wouldn't `FlangOption` be sufficient?
You're right, thanks for the comment.



Comment at: flang/test/Driver/omp-frontend-forwarding.f90:30
+
+! CHECK-OFFLOAD-HOST: "{{[^"]*}}flang-new" "-fc1" "-triple" 
"aarch64-unknown-linux-gnu"
+! CHECK-OFFLOAD-HOST-NOT: "-triple" "amdgcn-amd-amdhsa"

awarzynski wrote:
> Shouldn't there be 2 driver invocation for the host, as in the the 
> `CHECK-OFFLOAD-HOST-DEVICE` case?
In this case only the first invocation would remain, but I added an additional 
`OFFLOAD-HOST-NOT` to make this behavior explicit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147941

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

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D147256#4249527 , @zequanwu wrote:

> - Add a `-use-target-path-separator` flag for llc.
> - Add test for llc with that flag.

But where does `TM.Options.ObjectFilenameForDebug` come from? Presumably it 
comes from Clang at some point, so is there any chance we can fix it "at the 
source" instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[PATCH] D147875: [clang][Diagnostics] WIP: Show line numbers when printing code snippets

[Timm Bäder via Phabricator via cfe-commits]

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

Wow, in `FixIt/fixits-function-call`, this output:

  ./array.cpp:123:3: error: no matching function for call to 'f1'
f1(a + 1);
^~
  ./array.cpp:114:6: note: candidate function not viable: no known conversion 
from 'intTy2 *' (aka 'int *') to 'intTy &' (aka 'int &') for 1st argument; 
dereference the argument with *
  void f1(intTy &a);
   ^
  fix-it:"./array.cpp":{123:6-123:6}:"*("
  fix-it:"./array.cpp":{123:11-123:11}:")"

now (with increased snippet line limit) prints:

  ./array.cpp:123:3: error: no matching function for call to 'f1'
123 |   f1(a + 1);
|   ^~
  ./array.cpp:114:6: note: candidate function not viable: no known conversion 
from 'intTy2 *' (aka 'int *') to 'intTy &' (aka 'int &') for 1st argument; 
dereference the argument with *
114 | void f1(intTy &a);
|  ^
116 | void f2(intTy2 *a) {
117 | // CHECK: error: no matching function for call to 'f1
118 | // CHECK: dereference the argument with *
119 | // CHECK: void f1(intTy &a);
120 | // CHECK: fix-it{{.*}}*(
121 | // CHECK-NEXT: fix-it{{.*}})
122 | // CHECK: void f1(double *a);
123 |   f1(a + 1);
|  ~
|  *(   )
  fix-it:"./array.cpp":{123:6-123:6}:"*("
  fix-it:"./array.cpp":{123:11-123:11}:")"

Is this actually what we want? It somehow makes sense but also doesn't. It's 
weird that the test ensures the parseable fixits but the output doesn't show 
the fixit at all (in the first case).


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

https://reviews.llvm.org/D147875

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

[PATCH] D143364: [RISCV] Support scalar/fix-length vector NTLH intrinsic with different domain

[Piyou Chen via Phabricator via cfe-commits]

BeMg added a comment.

  test_nontemporal_load_v16i8
  test_nontemporal_load_v8i16

These two test cases in nontemporal.ll are affected by 
`areTwoSDNodeTargetMMOFlagsMergeable`.
Does we need extra testcases for `areTwoSDNodeTargetMMOFlagsMergeable`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143364

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

[PATCH] D147962: [RFC][clang] Pull experimental targets' info out of TargetInfo.cpp (NFC)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rjmccall, efriedma.
aaron.ballman added a comment.

Adding the codegen code owners for their approval, but in general I think this 
is a good idea.




Comment at: clang/lib/CodeGen/ABIInfoImpl.h:33-52
+bool isAggregateTypeForABI(QualType T);
+
+llvm::Type *getVAListElementType(CodeGenFunction &CGF);
+
+CGCXXABI::RecordArgABI getRecordArgABI(const RecordType *RT, CGCXXABI &CXXABI);
+
+CGCXXABI::RecordArgABI getRecordArgABI(QualType T, CGCXXABI &CXXABI);

Because we're sort of elevating these from implementation details hidden in the 
.cpp to interfaces exposed via a header file, we should probably document these 
functions better (not needed as part of this change though).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147962

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

[PATCH] D147698: [clang][dataflow] Add support for new and delete expressions.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

In D147698#4257698 , @mboehme wrote:

> @gribozavr2 Just to make sure I understand you correctly here (before I make 
> any changes to the code): IIUC you recommend simply doing nothing on a delete 
> expression? (Your arguments for this make sense to me, just want to 
> double-check I understood correctly.)

Yes, that's my suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147698

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Priyanshi Agarwal via Phabricator via cfe-commits]

ipriyanshi1708 updated this revision to Diff 512419.
ipriyanshi1708 added a comment.

Implemented the required changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -55,7 +55,6 @@
 
 using namespace clang;
 using namespace sema;
-
 Sema::DeclGroupPtrTy Sema::ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType) {
   if (OwnedType) {
 Decl *Group[2] = { OwnedType, Ptr };
@@ -5043,7 +5042,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const 
DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(T);
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5310,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -271,7 +271,6 @@
 TSC_imaginary,
 TSC_complex
   };
-
   // Import type specifier type enumeration and constants.
   typedef TypeSpecifierType TST;
   static const TST TST_unspecified = clang::TST_unspecified;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3421,7 +3421,7 @@
   InGroup;
 def warn_declspec_attribute_ignored : Warning<
   "attribute %0 is ignored, place it after "
-  "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
+  "\"%select{class|struct|interface|union|enum|enum class}1\" to apply 
attribute to "
   "type declaration">, InGroup;
 def warn_attribute_precede_definition : Warning<
   "attribute declaration must precede definition">,


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -55,7 +55,6 @@
 
 using namespace clang;
 using namespace sema;
-
 Sema::DeclGroupPtrTy Sema::ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType) {
   if (OwnedType) {
 Decl *Group[2] = { OwnedType, Ptr };
@@ -5043,7 +5042,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(T);
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5310,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -271,7 +271,6 @@
 TSC_imaginary,
 TSC_compl

[PATCH] D147626: [clang] Do not crash when initializing union with flexible array member

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:808
   unsigned NumElems = numStructUnionElements(ILE->getType());
-  if (RDecl->hasFlexibleArrayMember())
+  if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
 ++NumElems;

aaron.ballman wrote:
> Fznamznon wrote:
> > shafik wrote:
> > > Fznamznon wrote:
> > > > Just for some context, numStructUnionElements checks that there is a 
> > > > flexible array member and returns number_of_initializable_fields-1 for 
> > > > structs. For unions it just returns 1 or 0, so flexible array member 
> > > > caused adding one more element to initlistexpr that was never properly 
> > > > handled.
> > > > 
> > > > Instead of doing this change, we could probably never enter 
> > > > initialization since the record (union) declaration is not valid, but 
> > > > that is not the case even for other types of errors in code, for 
> > > > example, I've tried declaring field of struct with a typo:
> > > > 
> > > > ```
> > > > struct { cha x[]; } r = {1}; 
> > > > ```
> > > > Initialization is still performed by clang.
> > > > Also, it seems MSVC considers flexible array member inside union as 
> > > > valid, so the test code is probably not always invalid.
> > > I am not sure what to think here, looking at gcc documentation for this 
> > > extension: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html 
> > > 
> > > and using the following code:
> > > 
> > > ```
> > > struct f1 {
> > >   int x; int y[];
> > > } f1 = { 1, { 2, 3, 4 } }; // #1
> > > 
> > > struct f2 {
> > >   struct f1 f1; int data[3];
> > > } f2 = { { 1 }, { 2, 3, 4 } }; // #2
> > > 
> > > struct { char x[]; } r = {1};  // #3
> > > ```
> > > 
> > > gcc rejects 2 and 3 even though 2 comes from their documentation. Clang 
> > > warns on 2 and MSVC rejects 2
> > > 
> > > CC @aaron.ballman @rsmith 
> > Yes, I also had a feeling that we probably need to refine how these 
> > extensions are supported by clang, that is probably a bit out of scope of 
> > the fix though.
> The GCC extension differs based on C vs C++: https://godbolt.org/z/E14Yz37To
> As does the extension in Clang, but differently than GCC: 
> https://godbolt.org/z/zYznaYPf5
> 
> So I agree that there's work to be done on this extension, but it's outside 
> of the scope of the crash fix for this patch. For this patch, GCC rejects 
> allowing a flexible array member in a union in both C and C++, but it looks 
> like Clang rejects in C and perhaps accepts in C++: 
> https://godbolt.org/z/bTsPn3G7b So how about we add a C++ test case for this 
> as well -- if that still crashes, that should be fixed, but if the code is 
> accepted, we should either decide we want to start rejecting it or we should 
> ensure the codegen for it is correct.
While experimenting with C++ test, I've noticed the following thing:
```
union { char x[]; } r = {0}; // when in global scope generates no IR (without 
my patch crashes)

void foo() {
  union { char x[]; } r = {0}; // fails with error "initialization of flexible 
array member is not allowed" in both C/C++, no crash even without my patch
}

union A {char x[]; };
A a = {0}; // crashes even with my patch but with different assertion when 
trying -emit-llvm
void foo() {
  A a = {0}; // fails with error "initialization of flexible array member is 
not allowed" in both C and C++, no crash even without my patch
}
```

It is not entirely clear to me why the behavior different for function and TU 
scope. gcc always gives an error about flexible array in union, no matter how 
it is used. Also, it is strange that I'm not seeing the same "initialization of 
flexible array member is not allowed" error message for structs, just for 
unions. I'm thinking that we don't really have proper codegen for the code that 
I'm trying to fix and we should reject the code like gcc does. MSVC is fine 
with all though - https://godbolt.org/z/aoarEzd56 .

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Priyanshi Agarwal via Phabricator via cfe-commits]

ipriyanshi1708 updated this revision to Diff 512421.
ipriyanshi1708 added a comment.

Removed the spurious whitespace changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5043,7 +5043,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const 
DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(T);
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5311,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -271,7 +271,7 @@
 TSC_imaginary,
 TSC_complex
   };
-
+  
   // Import type specifier type enumeration and constants.
   typedef TypeSpecifierType TST;
   static const TST TST_unspecified = clang::TST_unspecified;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3421,7 +3421,7 @@
   InGroup;
 def warn_declspec_attribute_ignored : Warning<
   "attribute %0 is ignored, place it after "
-  "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
+  "\"%select{class|struct|interface|union|enum|enum class}1\" to apply 
attribute to "
   "type declaration">, InGroup;
 def warn_attribute_precede_definition : Warning<
   "attribute declaration must precede definition">,


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5043,7 +5043,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(T);
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5311,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -271,7 +271,7 @@
 TSC_imaginary,
 TSC_complex
   };
-
+  
   // Import type specifier type enumeration and constants.
   typedef TypeSpecifierType TST;
   static const TST TST_unspecified = clang::TST_unspecified;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3421,7 +

[PATCH] D147989: [clang] Fix Attribute Placement

[Priyanshi Agarwal via Phabricator via cfe-commits]

ipriyanshi1708 added a comment.

In D147989#4257721 , @aaron.ballman 
wrote:

> Thank you for working on this!
>
> The changes are missing test coverage; please be sure to add that, along with 
> a release note about the fix. I think there's likely more work to be done 
> here as well, considering this behavior: https://godbolt.org/z/sdK3Gef75 
> (notice the suggested location for the fix-it -- that also needs to be fixed 
> for this)

Thanks for reviewing. I am working on tests now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D138777: [clang-tidy] Add check bugprone-multiple-new-in-one-expression.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 512425.
balazske added a comment.

Fixed documentation issues.
Check is added to list.rst.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138777

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/multiple-new-in-one-expression.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/multiple-new-in-one-expression.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/multiple-new-in-one-expression.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/multiple-new-in-one-expression.cpp
@@ -0,0 +1,183 @@
+// RUN: %check_clang_tidy -std=c++11 -check-suffixes=ALL,CPP11 %s bugprone-multiple-new-in-one-expression %t
+// RUN: %check_clang_tidy -std=c++17 -check-suffixes=ALL,CPP17 %s bugprone-multiple-new-in-one-expression %t
+
+namespace std {
+typedef __typeof__(sizeof(0)) size_t;
+enum class align_val_t : std::size_t {};
+class exception {};
+class bad_alloc : public exception {};
+struct nothrow_t {};
+extern const nothrow_t nothrow;
+} // namespace std
+
+void *operator new(std::size_t, const std::nothrow_t &) noexcept;
+void *operator new(std::size_t, std::align_val_t, const std::nothrow_t &) noexcept;
+void *operator new(std::size_t, void *) noexcept;
+void *operator new(std::size_t, char);
+
+struct B;
+
+struct A { int VarI; int *PtrI; B *PtrB; };
+
+struct B { int VarI; };
+
+struct G {
+  G(A*, B*) {}
+  int operator+=(A *) { return 3; };
+};
+
+int f(int);
+int f(A*);
+int f(A*, B*);
+int f(int, B*);
+int f(G, G);
+int f(B*);
+void f1(void *, void *);
+A *g(A *);
+
+G operator+(const G&, const G&);
+
+void test_function_parameter(A *XA, B *XB) {
+  (void)f(new A, new B);
+  try {
+(void)f(new A, new B);
+  }
+  catch (A) {};
+  try {
+(void)f(new A, new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:13: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception; order of these allocations is undefined [
+(void)f(f(new A, new B));
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:15: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+int X = f(new A, new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:15: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+X = f(new A, new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:11: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+X = 1 + f(new A, new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:15: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+
+(void)f(g(new A), new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:15: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+
+(void)f(1 + f(new A), new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:19: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+(void)f(XA = new A, new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:18: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+(void)f(1 + f(new A), XB = new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:19: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+  }
+  catch (std::exception) {}
+}
+
+void test_operator(G *G1) {
+  (void)(f(new A) + f(new B));
+  try {
+(void)(f(new A) + f(new B));
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:14: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+(void)f(f(new A) + f(new B));
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:15: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+int X = f(new A) + f(new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:15: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+X = f(new A) + f(new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:11: warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception;
+X = 1 + f(new A) + 1 + f(new B);
+ // CHECK-MESSAGES-ALL: :[[@LINE-1]]:15: warning: memory allocation may leak if an other allocation is sequenced after it and throws an excep

[PATCH] D143813: [ClangFE] Check that __sync builtins are naturally aligned.

[Jonas Paulsson via Phabricator via cfe-commits]

jonpa added a comment.

In D143813#4256969 , @ahatanak wrote:

> I think this patch is causing the assertion in 
> `CodeGenFunction::setAddrOfLocalVar` to fail when the following code is 
> compiled:
>
>   void foo(unsigned long long t) {
> __sync_bool_compare_and_swap(({int x = 1; &t;}), t, t);
>   }
>
> Could you take a look?

I don't understand the first argument - I thought it was supposed to be just an 
address...

This patch seems to (in CheckAtomicAlignment()) call 
CGF.EmitPointerWithAlignment() on the CompoundStmt, which is unexpected. If 
this is a legal parameter for the address, I guess there should be some kind of 
special handling here. Perhaps it would be ok to only emit the warning for 
plain address paramters, I might think...?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143813

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added a comment.

Thanks for the input Aaron and Martin. This is looking good. With the suggested 
changes (don't forget the test, you could try adding one to 
clang/test/Sema/attr-declspec-ignored.c and 
clang/test/SemaCXX/attr-declspec-ignored.cpp) and some formatting 
 I think it will 
be ready.




Comment at: clang/include/clang/Sema/DeclSpec.h:274
   };
-
+  
   // Import type specifier type enumeration and constants.

There is still a whitespace change here.



Comment at: clang/lib/Sema/SemaDecl.cpp:5045
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const 
DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {

Thanks to C++ overloading, this can be called `GetDiagnosticTypeSpecifierID`, 
and I don't think it will need the `T` parameter since that can be fetched with 
`DS.getTypeSpecType()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147941: [Flang][Driver][OpenMP] Enable options for selecting offloading phases in flang

[Andrzej Warzynski via Phabricator via cfe-commits]

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

LGTM, thanks for addressing my comments. I'd wait a day before landing this - 
just in case other reviewers have comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147941

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

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D147779#4257212 , @PiotrZSL wrote:

> arc land should work, I usually use arc patch --nobranch to check things 
> before committing and then git push.

Hi @PiotrZSL, thank you for the help :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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

[PATCH] D147698: [clang][dataflow] Add support for new and delete expressions.

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 512434.
mboehme added a comment.

Eliminate specific handling of delete expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147698

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5170,4 +5170,66 @@
   });
 }
 
+TEST(TransferTest, NewExpressions) {
+  std::string Code = R"(
+void target() {
+  int *p = new int(42);
+  // [[after_new]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+getEnvironmentAtAnnotation(Results, "after_new");
+
+auto &P = getValueForDecl(ASTCtx, Env, "p");
+
+EXPECT_THAT(Env.getValue(P.getPointeeLoc()), NotNull());
+  });
+}
+
+TEST(TransferTest, NewExpressions_Structs) {
+  std::string Code = R"(
+struct Inner {
+  int InnerField;
+};
+
+struct Outer {
+  Inner OuterField;
+};
+
+void target() {
+  Outer *p = new Outer;
+  // Access the fields to make sure the analysis actually generates 
children
+  // for them in the `AggregateStorageLoc` and `StructValue`.
+  p->OuterField.InnerField;
+  // [[after_new]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+getEnvironmentAtAnnotation(Results, "after_new");
+
+const ValueDecl *OuterField = findValueDecl(ASTCtx, "OuterField");
+const ValueDecl *InnerField = findValueDecl(ASTCtx, "InnerField");
+
+auto &P = getValueForDecl(ASTCtx, Env, "p");
+
+auto &OuterLoc = cast(P.getPointeeLoc());
+auto &OuterFieldLoc =
+cast(OuterLoc.getChild(*OuterField));
+auto &InnerFieldLoc = OuterFieldLoc.getChild(*InnerField);
+
+// Values for the struct and all fields exist after the new.
+EXPECT_THAT(Env.getValue(OuterLoc), NotNull());
+EXPECT_THAT(Env.getValue(OuterFieldLoc), NotNull());
+EXPECT_THAT(Env.getValue(InnerFieldLoc), NotNull());
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -469,6 +469,20 @@
 Env.setValue(Loc, Env.create(*ThisPointeeLoc));
   }
 
+  void VisitCXXNewExpr(const CXXNewExpr *S) {
+auto &Loc = Env.createStorageLocation(*S);
+Env.setStorageLocation(*S, Loc);
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
+  }
+
+  void VisitCXXDeleteExpr(const CXXDeleteExpr *S) {
+// Empty method.
+// We consciously don't do anything on deletes.  Diagnosing double deletes
+// (for example) should be done by a specific analysis, not by the
+// framework.
+  }
+
   void VisitReturnStmt(const ReturnStmt *S) {
 if (!Env.getAnalysisOptions().ContextSensitiveOpts)
   return;


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5170,4 +5170,66 @@
   });
 }
 
+TEST(TransferTest, NewExpressions) {
+  std::string Code = R"(
+void target() {
+  int *p = new int(42);
+  // [[after_new]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+getEnvironmentAtAnnotation(Results, "after_new");
+
+auto &P = getValueForDecl(ASTCtx, Env, "p");
+
+EXPECT_THAT(Env.getValue(P.getPointeeLoc()), NotNull());
+  });
+}
+
+TEST(TransferTest, NewExpressions_Structs) {
+  std::string Code = R"(
+struct Inner {
+  int InnerField;
+};
+
+struct Outer {
+  Inner OuterField;
+};
+
+void target() {
+  Outer *p = new Outer;
+  // Access the fields to make sure the analysis actually generates children
+  // for them in the `AggregateStorageLoc` and `StructValue`.
+  p->OuterField.InnerField;
+  // [[after_new]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+getEnvironmentAtAnnotation(Results, "after_new");
+
+const ValueDecl *OuterField = findValueDecl(ASTCtx, "OuterField");
+const ValueDecl *InnerField = findValueDecl(ASTCtx, "InnerField"

[PATCH] D147698: [clang][dataflow] Add support for new expressions.

[Martin Böhme via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:315
+  ///  `D` must currently be associated with a value.
+  void unsetChild(const ValueDecl &D) {
+auto It = Children.find(&D);

gribozavr2 wrote:
> Modifying already-created values is wrong since values are supposed to be 
> immutable (I know we already do it in setChild above, but let's not add more?)
> 
> A value can be stored in multiple locations, and if one of them is deleted, 
> the other users of the same value shouldn't observe the change.
> Modifying already-created values is wrong since values are supposed to be 
> immutable (I know we already do it in setChild above, but let's not add more?)

Good point -- removed.

> A value can be stored in multiple locations, and if one of them is deleted, 
> the other users of the same value shouldn't observe the change.

No longer relevant, but just to complete the discussion: I don't believe this 
is true for `StructValue`s because of the way that prvalues of class type get 
materialized into result objects. I believe this causes every `StructValue` to 
be materialized into exactly one `AggregateStorageLocation`.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:485
+
+Env.unsetValue(PointerVal->getPointeeLoc());
+

gribozavr2 wrote:
> I'm not convinced that we need to break the loc/value association for deleted 
> heap allocations. After all, we don't do that for stack allocations that go 
> out of scope. So why bother for the heap? Especially since "delete" is very 
> rare in modern idiomatic C++.
> 
> 
Good point. This makes everything much simpler. Done.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:490
+// For example, an analysis that diagnoses double deletes may want to 
attach
+// properties to the `PointerValue` and access those after the first 
delete.
+  }

gribozavr2 wrote:
> It is correct that we shouldn't deallocate PointerValue, but the reasons for 
> that are different:
> 
> (1) The code being analyzed might have multiple copies of the pointer, and it 
> might have a double-free or a use-after-free. The analyzer shouldn't execute 
> UB when the program being analyzed has UB.
> 
> (2) The PointerValue is needed to analyze other basic blocks.
> 
> (3) The downstream code that is going to inspect the environments at various 
> program points will need the PointerValues (like the test in this patch).
> 
> 
> (1) The code being analyzed might have multiple copies of the pointer, and it 
> might have a double-free or a use-after-free. The analyzer shouldn't execute 
> UB when the program being analyzed has UB.

That's what I was trying to say with my comment here -- but this is now moot 
anyway, as we no longer do anything for deletes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147698

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Priyanshi Agarwal via Phabricator via cfe-commits]

ipriyanshi1708 updated this revision to Diff 512437.
ipriyanshi1708 added a comment.

Added the Release note for the fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5043,7 +5043,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const 
DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(T);
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5311,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3421,7 +3421,7 @@
   InGroup;
 def warn_declspec_attribute_ignored : Warning<
   "attribute %0 is ignored, place it after "
-  "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
+  "\"%select{class|struct|interface|union|enum|enum class}1\" to apply 
attribute to "
   "type declaration">, InGroup;
 def warn_attribute_precede_definition : Warning<
   "attribute declaration must precede definition">,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -193,6 +193,8 @@
 
 Bug Fixes to Attribute Support
 ^^
+- Fixed a bug where attribute annotations on type specifiers (enums, classes, 
structs, unions, and scoped enums) were not properly ignored, resulting in 
misleading warning messages. Now, such attribute annotations are correctly 
ignored.
+(`#61660 `_)
 
 Bug Fixes to C++ Support
 


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5043,7 +5043,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(T);
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5311,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierIDWithAttrs(TypeSpecType,DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3421,7 +3421,7 @@
   InGroup;
 def warn_declspec_attribute_ignored : Warning<
   "attribute %0 is ignored, place it after "
-  "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
+  "\"%select{class|struct|interface|uni

[PATCH] D147989: [clang] Fix Attribute Placement

[Priyanshi Agarwal via Phabricator via cfe-commits]

ipriyanshi1708 added a comment.

In D147989#4257947 , @samtebbs wrote:

> Thanks for the input Aaron and Martin. This is looking good. With the 
> suggested changes and some formatting 
>  I think it 
> will be ready.

Okay! I will update it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D138777: [clang-tidy] Add check bugprone-multiple-new-in-one-expression.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:91
+  ExceptionType, ExceptionReferenceType)));
+  auto BadAllocCatchingTryBlock = cxxTryStmt(hasHandlerFor(CatchBadAllocType));
+

what about: ```catch(...)```



Comment at: 
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:102
+  hasAnyArgument(
+  expr(HasNewExpr1, unless(equalsBoundNode("arg2"))).bind("arg1")),
+  hasAnyArgument(

this doesnt look valid, arg2 isn't known at this point yet, so this could be 
removed.
and this may not work for case like this:

```callSomething({new A,  new B});```  
Most probably this equalsBoundNode should be on HasNewExpr2 level, to avoid 
duplicated newExpr, not arguments of call

and image situation like this:

```
try {
  something(std::shared_ptr(new int), std::shared_ptr(new int));
} catch(const std::bad_alloc&) {}
```
this in theory could also produce false-positive.

other issue is that first call could be anything, maloc, new, calloc, wzalloc, 
operator new(), it doesn't need to be just new.
You could try to use some simple way of checking this like, 
isBeforeInTransationUnit

And this will also produce false-positive if we use `new(std::nothrow)` on 
second.
There are some "utils" to check sequence order, maybe would be good to 
investigate them.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:105
+  expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")),
+  hasAncestor(BadAllocCatchingTryBlock)),
+  this);

To be honest, I don't see any reason, how this try-catch would change anything, 
there can be one in parent function, and we going to heave a leak if we have 
try-catch or not.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:125
+  this);
+}
+

other issue I see is that same new could be matched multiple times by those 
Matchers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138777

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is right on the line of what I think is reasonable in terms of diagnostics 
from the backend. It provides useful information to users about inlining 
decisions, so that's awesome when that information is helpful. But those 
inlining decisions can be arbitrarily long and complex, so this can emit a 
*lot* of note diagnostics on real world code which can make the diagnostic 
harder to reason about. In general, I think inlining decisions should be left 
to an optimization report, but this information could make interpreting the 
`error` or `warning` attributes easier.

Have you checked to see how the diagnostics work in practice on deep inlining 
stacks? If it's usually only 1-2 extra notes, that's probably not going to 
overwhelm anyone. But if it's significantly more, that could be distracting. Do 
we want any options to control when to emit the notes? e.g., do we want to give 
users a way to say "only emit the last N inlining decision notes"?

Also, how does this work with LTO when it makes different inlining decisions?




Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:12
   foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh 
no foo}}
+ // expected-note@* {{In function 'baz'}}
   if (x())

Instead of allowing this note to appear anywhere in the file, I think it's 
better to use "bookmark" comments. e.g.,
```
void baz() { // #baz_defn
}

void foo() {
  baz(); // expected-note@#baz_defn {{whatever note text}}
}
```
so that we're testing where the diagnostic is emitted. (This is especially 
important given that the changes are about location tracking.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Just comments from the CFE.




Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:94
+def note_fe_backend_in : Note<"In function '%0'">;
+def note_fe_backend_inlined : Note<"\twhich inlined function '%0'">;
 

This tab in the diagnostic is odd, we never do this, we just count on the 
cascading notes to be clear.  Also, this probably needs bikeshedding for 
diagnostic messages.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:860
+
+  Diags.Report(diag::note_fe_backend_in) << 
llvm::demangle(D.getCaller().str());
+

Could we instead just make `demangle` take a `string_view` here?  It takes it 
by const-ref, which shows that it doesn't really seem to need it to be a 
string, so I would imagine this would be a minor refactor (to add such an 
overload).



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:29
+   // expected-note@* {{In function 'd'}}
+   // expected-note@* {{which inlined function 'b'}}
+   // expected-note@* {{which inlined function 'a'}}

Same comment re-bookmarks, but this diagnostic seems awkward.  

Should we instead say :

`call to 'foo' declared with 'error' attribute: oh no foo`
`called by function 'a'`
`inlined by function 'b'`
`inlined by function 'd'`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D147895: [clang-format] Handle Verilog assertions and loops

[sstwcw via Phabricator via cfe-commits]

sstwcw added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:844
 "};");
-  ASSERT_EQ(Tokens.size(), 44u);
+  ASSERT_EQ(Tokens.size(), 44u) << Tokens;
   EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);

HazardyKnusperkeks wrote:
> Unrelated, I'd prefer it as a single commit.
How should I name the separate commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147895

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Priyanshi Agarwal via Phabricator via cfe-commits]

ipriyanshi1708 updated this revision to Diff 512445.
ipriyanshi1708 added a comment.

Applied C++ Overloading


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5043,7 +5043,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierID(const DeclSpec &DS){
+  if (DS.getTypeSpecType() == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(DS.getTypeSpecType());
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5311,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierID(DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierID(DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3421,7 +3421,7 @@
   InGroup;
 def warn_declspec_attribute_ignored : Warning<
   "attribute %0 is ignored, place it after "
-  "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
+  "\"%select{class|struct|interface|union|enum|enum class}1\" to apply 
attribute to "
   "type declaration">, InGroup;
 def warn_attribute_precede_definition : Warning<
   "attribute declaration must precede definition">,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -193,6 +193,8 @@
 
 Bug Fixes to Attribute Support
 ^^
+- Fixed a bug where attribute annotations on type specifiers (enums, classes, 
structs, unions, and scoped enums) were not properly ignored, resulting in 
misleading warning messages. Now, such attribute annotations are correctly 
ignored.
+(`#61660 `_)
 
 Bug Fixes to C++ Support
 


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5043,7 +5043,18 @@
 llvm_unreachable("unexpected type specifier");
   }
 }
-
+static unsigned GetDiagnosticTypeSpecifierID(const DeclSpec &DS){
+  if (DS.getTypeSpecType() == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+  return 5;
+}
+  }
+  else{
+return GetDiagnosticTypeSpecifierID(DS.getTypeSpecType());
+  }
+  return 0;
+}
 /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
 /// no declarator (e.g. "struct foo;") is parsed. It also accepts template
 /// parameters to cope with template friend declarations.
@@ -5300,11 +5311,11 @@
 TypeSpecType == DeclSpec::TST_enum) {
   for (const ParsedAttr &AL : DS.getAttributes())
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+<< AL << GetDiagnosticTypeSpecifierID(DS);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-}
+<< AL << GetDiagnosticTypeSpecifierID(DS);
+}
   }
 
   return TagD;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3421,7 +3421,7 @@
   InGroup;
 def warn_declspec_attribute_ignored : Warning<
   "attribute %0 is ignored, place it after "
-  "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
+  "\"%select{class|struct|interface|union|enum|enum class}1\" to apply attribute to "
   "type declaration">, InGro

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

[Sam McCall via Phabricator via cfe-commits]

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

Time to wake up an old review!

Going to put high-level thoughts on 
https://github.com/clangd/clangd/discussions/1206, but since one of them is 
memory usage I wanted to have a look at the concrete data structure here.

TL;DR: I think we can make this cheap enough we don't care about the extra 
memory. The simple lookup optimization below might be enough (probably not). 
Dropping non-callee refs from the data structure is likely enough, the deep 
lookup optimization is likely enough, doing both probably **reduces** memory 
usage vs today!

---

Lookup optimization (simple):

`DenseMap>` is something like `56*containers + 
16*refs` bytes:

- a `pair, which is 4 pointers per symbol, plus extra 
buckets so >7 pointers if I'm reading DenseMap right
- a `RevRef` per entry which is two pointers
- maybe some allocator overhead for all those little vectors? (IDK how 
allocators work in practice)

Dropping the hashtable and just storing a flat `vector` sorted by 
container eliminates the per-symbol costs for `16*refs` bytes. If there were 
only 3-4 refs per function this would save 50%, but there's probably more. The 
tradeoff is binary search (with double-indirection for compare) instead of hash 
lookup, but still seems fast enough for what we want it for.

The elephant in the room is that most of these refs we're storing are not 
callees. If we were to drop those from the lookup structure as discussed inline 
above, then the ratio of refs:symbols goes down and this optimization becomes 
relatively more effective.

---

Lookup optimization (deeper): h/t @kadircet

I think it's easiest to explain as a pair of modifications to the current refs 
data structures (as checked-in, not the ones in this patch).

**First**: imagine we change RefSlab to produce:

- a big flat `vector` grouped by target.
- a lookup structure consisting of a sorted `vector>`

This is smaller than our current representation: the lookup structure is `20 * 
targets` bytes instead of the current `~40 * targets` bytes and making the ref 
storage contiguous is ~free.

Clearly this supports lookup of refs for a target by binary searching over the 
lookup structure by target.

But observe that also given a `Ref*` we can find its target: binary search over 
the lookup structure searching for the right **offset**, rather than target.

**Second**: have the index compute a permutation of refs grouped by Container. 
Naively this is a `vector`, but since we made the array flat it can be 
`vector`

Now we can look up the refs for a given container by binary searching within 
this permutation, as the container is stored in the Refs themselves.

For each result we have to do a second binary search in the lookup structure to 
get the target symbol, but we expect the #symbols per container to be small so 
this nesting isn't a great concern.

So the net cost vs HEAD is `4 * refs - 20 * targets` bytes, which is less than 
a quarter of the version above. This takes us to ~2% increase which we can eat 
even if we store this info for all references.

The costs here are expensive reverse lookups (I don't think we care), slightly 
more expensive forward lookups (also OK I think), code complexity (*mostly* 
localized). Obviously this is all somewhat invasive...

(We discussed an even further squeeze: we could use the same trick to avoid our 
current 8 bytes inside the ref for the container. I think this leads to too 
many levels of nested binary searches)




Comment at: clang-tools-extra/clangd/XRefs.cpp:1843
+auto Kind = Callee.SymInfo.Kind;
+if (Kind != SK::Function && Kind != SK::InstanceMethod &&
+Kind != SK::ClassMethod && Kind != SK::StaticMethod &&

qchateau wrote:
> nridge wrote:
> > If memory usage of `Dex::RevRefs` becomes a concern, we could consider only 
> > storing the references that would pass this filter in the first place. That 
> > would trade off time spent building the reverse lookup (we'd have to do 
> > symbol lookups or keep around extra state to track the symbol kinds) for 
> > space savings.
> I've used tried this patch with the whole llvm project indexed. If I remember 
> correctly it's something in the like of 100MB, around 5-10% même usage of 
> clangd. As such it's not very high but it's definitely not negligible, 
> especially for a feature that's not used very often.
> That would trade off time spent building the reverse lookup (we'd have to do 
> symbol lookups or keep around extra state to track the symbol kinds) for 
> space savings.

RefKind is a bitfield, so I think we could just tack on an extra `Call` bit at 
indexing time. Technically it's extra state, but it's cheap to compute and free 
to plumb around.

Then the tradeoff becomes: the index needs to know which refkinds to include in 
its reverse-lookup structure. In practical terms we could hard-code this, it's 
an ugly layering violation though. The prettiest version I can 

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:47-50
+for (tmpl of root.getElementsByTagName('template')) {
+  // Clear previously rendered template contents.
+  while (tmpl.nextSibling && tmpl.nextSibling.inflated)
+tmpl.parentNode.removeChild(tmpl.nextSibling);

sammccall wrote:
> gribozavr2 wrote:
> > Would it make sense to move this removal logic into `inflate` itself?
> > 
> > (Of course the recursive part of `inflate` would stay as a separate 
> > function, `inflateImpl` or something.)
> I think we end up with a 60 line function wrapped in a +4 line function, and 
> the nesting causes more confusion than it solves. I don't think clearing the 
> old content matches the name `inflate` either.
> 
> Happy to pull out `reinflate()` as a sibling if you think it helps - with one 
> callsite I'm not sure it does.
> Happy to pull out reinflate() as a sibling if you think it helps - with one 
> callsite I'm not sure it does.

I'd prefer that. The reason is that you can't correctly call inflate without 
first deleting the previously expanded nodes (even though there's only one call 
site.) And that suggests to me that the callee should be responsible for that 
part of the work as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin updated this revision to Diff 512447.
jrmolin marked 3 inline comments as done.
jrmolin added a comment.

- updated the tests to use long-form
- added new tests to verify the no-parameter cases
- updated the documentation to minimize formatting variables
- ran the style guide generator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25429,6 +25429,50 @@
   verifyFormat("auto x = 5s .count() == 5;");
 }
 
+TEST_F(FormatTest, BreakBeforeParameterList) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.AlwaysBreakBeforeFunctionParameters, false);
+
+  // verify that there is no break by default
+  verifyFormat("int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   "int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   Style);
+
+  verifyFormat("void function1(int param1, int param2, int param3) {}\n",
+   "void function1(int param1, int param2, int param3) {}\n",
+   Style);
+
+  // verify that there is a break when told to break
+  Style.AlwaysBreakBeforeFunctionParameters = true;
+  verifyFormat("int function1(\n" // the formatted part
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function2();\n",
+   // the original
+   "int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   Style);
+
+  verifyFormat("void function1(\n" // the formatted part
+   "int param1,\n"
+   "int param2,\n"
+   "int param3) {}\n",
+   // the original
+   "void function1(int param1, int param2, int param3) {}\n",
+   Style);
+
+  // verify that having no parameters doesn't affect the parentheses
+  verifyFormat("void function1() {}\n", // the formatted part
+   "void function1() {}\n", // the original
+   Style);
+
+  verifyFormat("void function1();\n", // the formatted part
+   "void function1();\n", // the original
+   Style);
+}
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -149,6 +149,7 @@
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeFunctionParameters);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4847,6 +4847,14 @@
 return true;
   }
 
+  // If AlwaysBreakBeforeFunctionParameters is true, we want to break before
+  // the next parameter, if there is one.
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+  !Right.is(tok::r_paren) && Left.Previous &&
+  Left.Previous->is(TT_FunctionDeclarationName)) {
+return true;
+  }
+
   // If the last token before a '}', ']', or ')' is a comma or a trailing
   // comment, the intention is to insert a line break after it in order to make
   // shuffling around entries easier. Import statements, especially in
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -873,6 +873,8 @@
Style.AlwaysBreakAfterReturnType);
 IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
Style.AlwaysBreakBeforeMultilineStrings);
+IO.mapOptional("AlwaysBreakBeforeFunctionParameters",
+   Style.AlwaysBreakBeforeFunctionParameters);
 IO.mapOptional("AlwaysBreakTemplateDeclarations",
Style.AlwaysBreakTemplateDeclarations);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
@@ -1331,6 +1333,7 @@
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLV

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

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



Comment at: clang/docs/ClangFormatStyleOptions.rst:1380
+  Example uses
+  ``AlwaysBreakAfterReturnType`` set to ``All``.
+

MyDeveloperDay wrote:
> This isn't relevant is it? `AlwaysBreakAfterReturnType`
No, it's not relevant, but I saw it in other examples, and I like the way it 
looks. I can change the example to be more minimal.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4789
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+  !Right.is(tok::r_paren) && Left.Previous &&
+  Left.Previous->is(TT_FunctionDeclarationName)) {

MyDeveloperDay wrote:
> can you test the `()` case in your tests please with 
> `AlwaysBreakBeforeFunctionParameters` set to true
updated the tests.



Comment at: clang/unittests/Format/FormatTest.cpp:25410-25415
+  verifyFormat("int function1(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function2();\n",
+   Code, Style);

MyDeveloperDay wrote:
> 
Oh, I see. Sure!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin updated this revision to Diff 512449.
jrmolin marked an inline comment as done.
jrmolin added a comment.

- updated the documentation to show the option works for function declaration 
and definition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25429,6 +25429,50 @@
   verifyFormat("auto x = 5s .count() == 5;");
 }
 
+TEST_F(FormatTest, BreakBeforeParameterList) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.AlwaysBreakBeforeFunctionParameters, false);
+
+  // verify that there is no break by default
+  verifyFormat("int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   "int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   Style);
+
+  verifyFormat("void function1(int param1, int param2, int param3) {}\n",
+   "void function1(int param1, int param2, int param3) {}\n",
+   Style);
+
+  // verify that there is a break when told to break
+  Style.AlwaysBreakBeforeFunctionParameters = true;
+  verifyFormat("int function1(\n" // the formatted part
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function2();\n",
+   // the original
+   "int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   Style);
+
+  verifyFormat("void function1(\n" // the formatted part
+   "int param1,\n"
+   "int param2,\n"
+   "int param3) {}\n",
+   // the original
+   "void function1(int param1, int param2, int param3) {}\n",
+   Style);
+
+  // verify that having no parameters doesn't affect the parentheses
+  verifyFormat("void function1() {}\n", // the formatted part
+   "void function1() {}\n", // the original
+   Style);
+
+  verifyFormat("void function1();\n", // the formatted part
+   "void function1();\n", // the original
+   Style);
+}
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -149,6 +149,7 @@
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeFunctionParameters);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4847,6 +4847,14 @@
 return true;
   }
 
+  // If AlwaysBreakBeforeFunctionParameters is true, we want to break before
+  // the next parameter, if there is one.
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+  !Right.is(tok::r_paren) && Left.Previous &&
+  Left.Previous->is(TT_FunctionDeclarationName)) {
+return true;
+  }
+
   // If the last token before a '}', ']', or ')' is a comma or a trailing
   // comment, the intention is to insert a line break after it in order to make
   // shuffling around entries easier. Import statements, especially in
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -873,6 +873,8 @@
Style.AlwaysBreakAfterReturnType);
 IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
Style.AlwaysBreakBeforeMultilineStrings);
+IO.mapOptional("AlwaysBreakBeforeFunctionParameters",
+   Style.AlwaysBreakBeforeFunctionParameters);
 IO.mapOptional("AlwaysBreakTemplateDeclarations",
Style.AlwaysBreakTemplateDeclarations);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
@@ -1331,6 +1333,7 @@
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  LLVMStyle.Alw

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

[Sunil K via Phabricator via cfe-commits]

koops updated this revision to Diff 512448.
koops added a comment.

Removing changes from :

1. clang/include/clang/AST/StmtOpenMP.h
2. clang/lib/AST/StmtOpenMP.cpp
3. clang/include/clang/Parse/Parser.h

These were useful when the code was in CodeGen to handle the bind clause.


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

https://reviews.llvm.org/D144634

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/loop_bind_codegen.cpp

Index: clang/test/OpenMP/loop_bind_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/loop_bind_codegen.cpp
@@ -0,0 +1,132 @@
+// Copyright 2020 Hewlett Packard Enterprise Development LP
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ %s -emit-llvm -o - | FileCheck %s 
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+
+/*
+#include 
+#include 
+#include 
+#include 
+*/
+
+#define NNN 50
+int aaa[NNN];
+
+void parallel_loop() {
+  #pragma omp parallel
+  {
+ #pragma omp loop bind(parallel)
+ for (int j = 0 ; j < NNN ; j++) {
+   aaa[j] = j*NNN;
+ }
+   }
+}
+
+
+void teams_loop() {
+  #pragma omp teams
+  {
+ #pragma omp loop bind(teams)
+ for (int j = 0 ; j < NNN ; j++) {
+   aaa[j] = j*NNN;
+ }
+   }
+}
+
+void thread_loop() {
+  #pragma omp parallel
+  {
+ #pragma omp loop bind(thread)
+ for (int j = 0 ; j < NNN ; j++) {
+   aaa[j] = j*NNN;
+ }
+   }
+}
+
+void thread_loop2() {
+  #pragma omp loop bind(thread)
+  for (int j = 0 ; j < NNN ; j++) {
+aaa[j] = j*NNN;
+  }
+}
+
+int main() {
+  parallel_loop();
+  teams_loop();
+  thread_loop();
+  thread_loop2();
+
+  return 0;
+}
+#endif
+// CHECK-LABEL: define {{[^@]+}}@_Z13parallel_loopv
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @3, i32 0, ptr @.omp_outlined.)
+// CHECK-NEXT: ret void
+// CHECK-LABEL: define {{[^@]+}}@.omp_outlined
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[DOTGLOBAL_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTBOUND_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTOMP_IV:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[TMP:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_LB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_UB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_STRIDE:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_IS_LAST:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK:   call void @__kmpc_for_static_init_4(ptr @1, i32 %1, i32 34, ptr [[DOTOMP_IS_LAST]], ptr [[DOTOMP_LB]], ptr [[DOTOMP_UB]], ptr [[DOTOMP_STRIDE]], i32 1, i32 1)
+// CHECK-LABEL: cond.true:
+// CHECK-NEXT:br label [[COND_END:%.*]]
+// CHECK-LABEL: cond.false:
+// CHECK-NEXT:[[TMP11:%.*]] = load i32, ptr [[DOTOMP_UB]], align 4
+// CHECK-NEXT:br label [[COND_END]]
+// CHECK:   omp.inner.for.cond:
+// CHECK-NEXT:[[TMP13:%.*]] = load i32, ptr [[DOTOMP_IV]], align 4
+// CHECK:   omp.inner.for.body:
+// CHECK:   omp.loop.exit:
+// CHECK-NEXT:call void @__kmpc_for_static_fini(ptr @1, i32 %1)
+// CHECK-NEXT:call void @__kmpc_barrier(ptr @2, i32 %1)
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@_Z10teams_loopv
+// CHECK-NEXT:  entry:
+// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_teams(ptr @3, i32 0, ptr @.omp_outlined..1)
+// CHECK-NEXT: ret void
+//
+// CHECK-LABEL: define {{[^@]+}}@.omp_outlined
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[DOTGLOBAL_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTBOUND_TID__ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTOMP_IV:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[TMP:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_LB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_UB:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_STRIDE:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_IS_LAST:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK:   call void @__kmpc_for_static_init_4(ptr @4, i32 %1, i32 92, ptr [[DOTOMP_IS_LAST]], ptr [[DOTOMP_LB]], ptr [[DOTOMP_UB]], ptr [[DOTOMP_STRIDE]], i32 1, i32 1)
+// CHECK-LABEL: cond.true:
+// CHECK-NEXT:br label [[COND_END:%.*]]
+// CHECK-LABEL: cond.false:
+// CHECK-NEXT:[[TMP11:%.*]] = load i32, ptr [[DOTOMP_UB]], align 4
+// CHECK-NEXT:br label [[COND_END]]
+// CHECK:   omp.inner.for.cond:
+// CHECK-NEXT:[[TMP13:%.*]] = load i32, ptr [[DOTOMP_IV]], align 4
+// CHECK:   omp.inner.for.body:
+// CHECK:   omp.loop.exit:
+// CHECK-NEXT:call void @__kmpc_for_static_fini(ptr @4, i32 %1)
+// CHECK-NEXT:ret void
+//
+// CHECK-LABEL: define {{[^@]+}}@_Z11thread_loopv()
+// CHECK-NEXT: entry:

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:6124
 BindKind = BC->getBindKind();
+
   // First check CancelRegion which is then used in checkNestingOfRegions.

Remove this new line



Comment at: clang/lib/Sema/SemaOpenMP.cpp:6181-6189
+  DSAStack->setCurrentDirective(OMPD_for);
+  break;
+case OMPC_BIND_teams:
+  Kind = OMPD_distribute;
+  DSAStack->setCurrentDirective(OMPD_distribute);
+  break;
+case OMPC_BIND_thread:

1. You're overriding the directive kind here and do not restore it then. It may 
cause the compiler crash.
2. I think you need to here to create a new OpenMP region rather than 
overriding the existing one.



Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:1
+// Copyright 2020 Hewlett Packard Enterprise Development LP
+

Remove this



Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:51-56
+void thread_loop2() {
+  #pragma omp loop bind(thread)
+  for (int j = 0 ; j < NNN ; j++) {
+aaa[j] = j*NNN;
+  }
+}

I think it should trigger the assert in setCurrentDirective


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

https://reviews.llvm.org/D144634

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

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Chris Cotter via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases

PiotrZSL wrote:
> ccotter wrote:
> > ccotter wrote:
> > > PiotrZSL wrote:
> > > > what about when someone uses std::move instead of std::format ?
> > > > maybe some "note" for such issue ?
> > > Are you suggesting to have the tool add a special note in something like
> > > 
> > > ```
> > > template 
> > > void foo(T&& t) { T other = std::move(t); }
> > > ```
> > > 
> > > I'm not sure I completely followed what you were saying. Or perhaps a 
> > > fixit for this specific case of using move on a forwarding reference 
> > > (fixit to replace `move` with `forward`).
> > @PiotrZSL - I wasn't sure what you meant here.
> Yes, I meant that situation. But only because "forwarding reference parameter 
> 't' is never forwarded inside the function body" could be confused in such 
> case, where there is std::move instead of std::forward.
> But that could be ignored, or covered by other check that would simply detect 
> that you doing move when you should do forward.
Ya, 
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/move-forwarding-reference.html
 should cover this. This new check missing-std-forward only knows how to find 
code that is missing `forward`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

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

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 512455.
ccotter marked an inline comment as done.
ccotter added a comment.

Fix tests


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

https://reviews.llvm.org/D146921

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference { using type = T; };
+
+template  using remove_reference_t = typename remove_reference::type;
+
+template  constexpr T &&forward(remove_reference_t &t) noexcept;
+template  constexpr T &&forward(remove_reference_t &&t) noexcept;
+template  constexpr remove_reference_t &&move(T &&x);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+  S();
+  S(const S&);
+  S(S&&) noexcept;
+  S& operator=(const S&);
+  S& operator=(S&&) noexcept;
+};
+
+template 
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template 
+void does_not_forward(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+  T other = t;
+}
+
+template 
+void does_not_forward_invoked(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+  T other = t();
+}
+
+template 
+void forwards_pairwise(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+  auto first = std::forward(t.first);
+  auto second = std::forward(t.second);
+}
+
+template 
+void does_not_forward_pack(Ts&&... ts) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+  consumes_all(ts...);
+}
+
+template 
+class AClass {
+
+  template 
+  AClass(U&& u) : data(u) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+  template 
+  AClass& operator=(U&& u) { }
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+  template 
+  void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+  }
+
+  T data;
+};
+
+template 
+void does_not_forward_in_evaluated_code(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+  using result_t = decltype(std::forward(t));
+  unsigned len = sizeof(std::forward(t));
+  T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template 
+void just_a_decl(T&&t);
+
+template 
+void does_forward(T&& t) {
+  T other = std::forward(t);
+}
+
+template 
+void does_forward_pack(Ts&&... ts) {
+  consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+  S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+  S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+  S other = std::move(s);
+}
+
+template 
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+  T other = std::move(t);
+}
+
+template 
+class AClass {
+
+  template 
+  AClass(U&& u) : data(std::forward(u)) {}
+
+  template 
+  AClass& operator=(U&& u) {
+data = std::forward(u);
+  }
+
+  void rvalue_ref(T&& t) {
+T other = std::move(t);
+  }
+
+  T data;
+};
+
+} // namespace negative_cases
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/lis

[PATCH] D147989: [clang] Fix Attribute Placement

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3424
   "attribute %0 is ignored, place it after "
-  "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
+  "\"%select{class|struct|interface|union|enum|enum class}1\" to apply 
attribute to "
   "type declaration">, InGroup;

"enum struct" is also possible in C++, so I think this needs to be covered as 
well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added reviewers: RKSimon, pengfei, goldstein.w.n.
Herald added a project: All.
probinson requested review of this revision.

https://reviews.llvm.org/D148021

Files:
  clang/lib/Headers/fmaintrin.h

Index: clang/lib/Headers/fmaintrin.h
===
--- clang/lib/Headers/fmaintrin.h
+++ clang/lib/Headers/fmaintrin.h
@@ -18,192 +18,724 @@
 #define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__, __target__("fma"), __min_vector_width__(128)))
 #define __DEFAULT_FN_ATTRS256 __attribute__((__always_inline__, __nodebug__, __target__("fma"), __min_vector_width__(256)))
 
+/// Computes a multiply-add of 128-bit vectors of [4 x float].
+///For each element, computes  (__A * __B) + __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the  VFMADD213PS  instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the addend.
+/// \returns A 128-bit vector of [4 x float] containing the result.
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_fmadd_ps(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddps((__v4sf)__A, (__v4sf)__B, (__v4sf)__C);
 }
 
+/// Computes a multiply-add of 128-bit vectors of [2 x double].
+///For each element, computes  (__A * __B) + __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the  VFMADD213PD  instruction.
+///
+/// \param __A
+///A 128-bit vector of [2 x double] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [2 x double] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [2 x double] containing the addend.
+/// \returns A 128-bit [2 x double] vector containing the result.
 static __inline__ __m128d __DEFAULT_FN_ATTRS128
 _mm_fmadd_pd(__m128d __A, __m128d __B, __m128d __C)
 {
   return (__m128d)__builtin_ia32_vfmaddpd((__v2df)__A, (__v2df)__B, (__v2df)__C);
 }
 
+/// Computes a scalar multiply-add of the single-precision values in the
+///low 32 bits of 128-bit vectors of [4 x float]. \n
+/// result[31:0] = (__A[31:0] * __B[31:0]) + __C[31:0]  \n
+/// result[127:32] = __A[127:32] 
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the  VFMADD213SS  instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand in the low
+///32 bits.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier in the low
+///32 bits.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the addend in the low
+///32 bits.
+/// \returns A 128-bit vector of [4 x float] containing the result in the low
+///32 bits and a copy of \a __A[127:32] in the upper 96 bits.
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_fmadd_ss(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddss3((__v4sf)__A, (__v4sf)__B, (__v4sf)__C);
 }
 
+/// Computes a scalar multiply-add of the double-precision values in the
+///low 64 bits of 128-bit vectors of [2 x double]. \n
+/// result[63:0] = (__A[63:0] * __B[63:0]) + __C[63:0]  \n
+/// result[127:64] = __A[127:64] 
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the  VFMADD213SD  instruction.
+///
+/// \param __A
+///A 128-bit vector of [2 x double] containing the multiplicand in the low
+///64 bits.
+/// \param __B
+///A 128-bit vector of [2 x double] containing the multiplier in the low
+///64 bits.
+/// \param __C
+///A 128-bit vector of [2 x double] containing the addend in the low
+///64 bits.
+/// \returns A 128-bit vector of [2 x double] containing the result in the low
+///64 bits and a copy of \a __A[127:64] in the upper 64 bits.
 static __inline__ __m128d __DEFAULT_FN_ATTRS128
 _mm_fmadd_sd(__m128d __A, __m128d __B, __m128d __C)
 {
   return (__m128d)__builtin_ia32_vfmaddsd3((__v2df)__A, (__v2df)__B, (__v2df)__C);
 }
 
+/// Computes a multiply-subtract of 128-bit vectors of [4 x float].
+///For each element, computes  (__A * __B) - __C .
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the  VFMSUB213PS  instruction.
+///
+/// \param __A
+///A 128-bit vector of [4 x float] containing the multiplicand.
+/// \param __B
+///A 128-bit vector of [4 x float] containing the multiplier.
+/// \param __C
+///A 128-bit vector of [4 x float] containing the subtrahend.
+/// \returns A 128-bit vector of [4 x float] containing the result.
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_fmsub_ps(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddps((__v4sf)__A, (__v4sf)__B, -(__v4sf)__C);
 }
 
+/// Computes a multiply-subtract of 128-bit vectors of [2 x double].
+///For each element, computes  (__A * __B) - 

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Headers/fmaintrin.h:22
+/// Computes a multiply-add of 128-bit vectors of [4 x float].
+///For each element, computes  (__A * __B) + __C .
+///

We are using a special format to describute the function in a pseudo code to 
share it with the intrinsic guide, e.g.,
https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/avx512fintrin.h#L9604-L9610
https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm512_i32logather_pd&ig_expand=4077

There's no strong requirement to follow it, but it would be better to adopt a 
uniform format.



Comment at: clang/lib/Headers/fmaintrin.h:26
+///
+/// This intrinsic corresponds to the  VFMADD213PS  instruction.
+///

It would be odd to user given this is just 1/3 instructions the intrinsic may 
generate, but I don't have a good idea here.


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

https://reviews.llvm.org/D148021

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

[PATCH] D147909: [clang] Implement CWG 2397

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: clang-language-wg.
aaron.ballman added a comment.

The changes need a release note, but also this should have changes to 
`clang/test/CXX/drs/dr23xx.cpp` with the proper dr markings and update 
`clang/www/cxx_dr_status.html`.




Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/cwg2397.cpp:11
+}
+
+void g() {

I think it'd be good to also show a constexpr test, like:
```
constexpr int foo() {
  int a[] = { 1, 2, 3 };
  auto (&c)[3] = a;

  return c[2];
}

static_assert(foo() == 3, "");
```
to prove that we actually perform the assignment properly, not just figure out 
the deduced type correctly.



Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p1-cxx0x.cpp:5
   int b[5];
-  auto a[5] = b; // expected-error{{'a' declared as array of 'auto'}}
-  auto *c[5] = b; // expected-error{{'c' declared as array of 'auto *'}}
+  auto a[5] = b; // expected-error{{variable 'a' with type 'auto[5]' has 
incompatible initializer of type 'int[5]'}}
+  auto *c[5] = b; // expected-error{{variable 'c' with type 'auto *[5]' has 
incompatible initializer of type 'int[5]'}}

I've seen worse diagnostics, but the phrasing here is interesting -- if you use 
`int a[5] = b;` instead of `auto`, you get `array initializer must be an 
initializer list` as a diagnostic, so I wonder why we're getting such a 
drastically different diagnostic for `auto`. Same for the diagnostic below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147909

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

[PATCH] D147909: [clang] Implement CWG 2397

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

Agree, this test clearly belongs to `clang/test/CXX/drs/dr23xx.cpp`.
There is a `clang/www/make_cxx_dr_status` script to update cxx_dr_status page, 
so you don't have to edit it manually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147909

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin updated this revision to Diff 512480.
jrmolin added a comment.

fix a comment in the documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25429,6 +25429,50 @@
   verifyFormat("auto x = 5s .count() == 5;");
 }
 
+TEST_F(FormatTest, BreakBeforeParameterList) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.AlwaysBreakBeforeFunctionParameters, false);
+
+  // verify that there is no break by default
+  verifyFormat("int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   "int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   Style);
+
+  verifyFormat("void function1(int param1, int param2, int param3) {}\n",
+   "void function1(int param1, int param2, int param3) {}\n",
+   Style);
+
+  // verify that there is a break when told to break
+  Style.AlwaysBreakBeforeFunctionParameters = true;
+  verifyFormat("int function1(\n" // the formatted part
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function2();\n",
+   // the original
+   "int function1(int param1, int param2, int param3);\n"
+   "int function2();\n",
+   Style);
+
+  verifyFormat("void function1(\n" // the formatted part
+   "int param1,\n"
+   "int param2,\n"
+   "int param3) {}\n",
+   // the original
+   "void function1(int param1, int param2, int param3) {}\n",
+   Style);
+
+  // verify that having no parameters doesn't affect the parentheses
+  verifyFormat("void function1() {}\n", // the formatted part
+   "void function1() {}\n", // the original
+   Style);
+
+  verifyFormat("void function1();\n", // the formatted part
+   "void function1();\n", // the original
+   Style);
+}
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -149,6 +149,7 @@
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeFunctionParameters);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4847,6 +4847,14 @@
 return true;
   }
 
+  // If AlwaysBreakBeforeFunctionParameters is true, we want to break before
+  // the next parameter, if there is one.
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+  !Right.is(tok::r_paren) && Left.Previous &&
+  Left.Previous->is(TT_FunctionDeclarationName)) {
+return true;
+  }
+
   // If the last token before a '}', ']', or ')' is a comma or a trailing
   // comment, the intention is to insert a line break after it in order to make
   // shuffling around entries easier. Import statements, especially in
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -873,6 +873,8 @@
Style.AlwaysBreakAfterReturnType);
 IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
Style.AlwaysBreakBeforeMultilineStrings);
+IO.mapOptional("AlwaysBreakBeforeFunctionParameters",
+   Style.AlwaysBreakBeforeFunctionParameters);
 IO.mapOptional("AlwaysBreakTemplateDeclarations",
Style.AlwaysBreakTemplateDeclarations);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
@@ -1331,6 +1333,7 @@
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  LLVMStyle.AlwaysBreakBeforeFunctionParameters = false;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

> When the `.clang-tidy` file is checked into the source control(git). It means 
> that anyone who contributes to the project will need to ensure that they have 
> a version of clang-tidy that will be able to read the file.
> This can cause problems as binaries of clang-tidy aren't always provided for 
> specific versions on certain platforms.

Isn't that expected though? All projects have expectations on the required 
dependencies to build their code. For example LLVM requires a compiler with 
C++17 support and a modern CMake that needs to be downloaded from the CMake 
website on older Ubuntu versions. Granted, those are backwards-compatible 
constraints, but nevertheless require that the user sets them up. It doesn't 
come out of the box in a regular distribution. Same applies to other C++ 
libraries, Python pip packages, etc. That's why typically projects should set 
up a sandboxed environment with pinned versions of dependencies to ensure they 
can always be built and linted at any point in time.

Take also for example Clang compiler, I can see in the release notes that they 
have removed 2 user-facing compiler flags:
https://clang.llvm.org/docs/ReleaseNotes.html#removed-compiler-flags

This means this compiler can no longer be used to compile code that has a 
checked-in CMakeLists.txt (or similar) that uses those compiler flags. How much 
of a problem that is? Should those projects really prevent Clang from cleaning 
technical debt and be user-friendly for the general public?

I do acknowledge that the proposed deprecation is much bigger since it will 
affect essentially all projects. I'm thinking perhaps we can simply extend the 
notice period so everyone has time to update, together with printing some 
warning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147876

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

[PATCH] D143467: [PowerPC] Add target feature requirement to builtins

[Kamau Bridgeman via Phabricator via cfe-commits]

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

Sorry I should have requested changes before for this comment below, but I do 
want these test moved to codegen and expanded, please let me know if anything 
is unclear.

In D143467#4241667 , @kamaub wrote:

> Can you add a PowerPC codegen test case for `__attribute__((target(`? All of 
> the updated test cases seem to only test `-target-feature`.
> The only test case we have for `__attribute((target(` is a sema test 
> `./clang/test/Sema/ppc-attr-target-inline.c`.
>
> Converting the deleted `clang/test/Sema/ppc-mma-builtins.c` and 
> `clang/test/Sema/ppc-paired-vector-builtins.c` to a codegen test cases
> like `clang/test/CodeGen/PowerPC/builtins-ppc-htm.c` using FileCheck seems 
> like a nice solution since it would reintroduce the testing
> for `+paired-vector-memops,-mma` situations, as well as a for 
> `__attribute__((target("no-mma")))`




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143467

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Hans Wennborg via Phabricator via cfe-commits]

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

Again not an expert here, but lgtm.

(Nit: the 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExprCXX.cpp#L1528-L1530
 link in the description seems to point to the wrong code now, since main 
changed. Here is a link for 16.0.1: 
https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D147073#4258384 , @hans wrote:

> Again not an expert here, but lgtm.
>
> (Nit: the 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExprCXX.cpp#L1528-L1530
>  link in the description seems to point to the wrong code now, since main 
> changed. Here is a link for 16.0.1: 
> https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536)

I'm confused -- I thought D147569  resolved 
the issue and so this patch is no longer needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D138777: [clang-tidy] Add check bugprone-multiple-new-in-one-expression.

[Balázs Kéri via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:91
+  ExceptionType, ExceptionReferenceType)));
+  auto BadAllocCatchingTryBlock = cxxTryStmt(hasHandlerFor(CatchBadAllocType));
+

PiotrZSL wrote:
> what about: ```catch(...)```
`hasHandlerFor` checks for it (and it is used in the test).



Comment at: 
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:102
+  hasAnyArgument(
+  expr(HasNewExpr1, unless(equalsBoundNode("arg2"))).bind("arg1")),
+  hasAnyArgument(

PiotrZSL wrote:
> this doesnt look valid, arg2 isn't known at this point yet, so this could be 
> removed.
> and this may not work for case like this:
> 
> ```callSomething({new A,  new B});```  
> Most probably this equalsBoundNode should be on HasNewExpr2 level, to avoid 
> duplicated newExpr, not arguments of call
> 
> and image situation like this:
> 
> ```
> try {
>   something(std::shared_ptr(new int), std::shared_ptr(new int));
> } catch(const std::bad_alloc&) {}
> ```
> this in theory could also produce false-positive.
> 
> other issue is that first call could be anything, maloc, new, calloc, 
> wzalloc, operator new(), it doesn't need to be just new.
> You could try to use some simple way of checking this like, 
> isBeforeInTransationUnit
> 
> And this will also produce false-positive if we use `new(std::nothrow)` on 
> second.
> There are some "utils" to check sequence order, maybe would be good to 
> investigate them.
I am not an expert in how AST matchers work exactly, but this code works with 
the provided tests and the results look correct. I did not experience that two 
`new` in the same argument is matched, this is why the 
`unless(equalsBoundNode(...))` is added. The "arg1" and "arg2" nodes are the 
direct expressions in the function call, not descendants, and a `new` in the 
same argument (any descendant) is not matched twice in this way. Otherwise this 
would show up in failed tests.

The code `something(std::shared_ptr(new int), std::shared_ptr(new 
int))` should produce warning (it may be possible that result of the first `new 
int` is not passed to `shared_ptr` before the other `new int` is called that 
fails, good solution is use of `std::make_shared` in such case). The test code 
`(void)f(g(new A), new B);` is somewhat similar AST, the `new A` should be 
found in all cases because `hasDescendant` is used at `HasNewExpr1` and 2.

Probably `unless(equalsBoundNode("arg2"))` can be removed, it is enough to have 
one of these checks.

`InitListExpr` is not handled by the current code, I need to add this case (it 
has fixed evaluation order).



Comment at: 
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:102
+  hasAnyArgument(
+  expr(HasNewExpr1, unless(equalsBoundNode("arg2"))).bind("arg1")),
+  hasAnyArgument(

balazske wrote:
> PiotrZSL wrote:
> > this doesnt look valid, arg2 isn't known at this point yet, so this could 
> > be removed.
> > and this may not work for case like this:
> > 
> > ```callSomething({new A,  new B});```  
> > Most probably this equalsBoundNode should be on HasNewExpr2 level, to avoid 
> > duplicated newExpr, not arguments of call
> > 
> > and image situation like this:
> > 
> > ```
> > try {
> >   something(std::shared_ptr(new int), std::shared_ptr(new int));
> > } catch(const std::bad_alloc&) {}
> > ```
> > this in theory could also produce false-positive.
> > 
> > other issue is that first call could be anything, maloc, new, calloc, 
> > wzalloc, operator new(), it doesn't need to be just new.
> > You could try to use some simple way of checking this like, 
> > isBeforeInTransationUnit
> > 
> > And this will also produce false-positive if we use `new(std::nothrow)` on 
> > second.
> > There are some "utils" to check sequence order, maybe would be good to 
> > investigate them.
> I am not an expert in how AST matchers work exactly, but this code works with 
> the provided tests and the results look correct. I did not experience that 
> two `new` in the same argument is matched, this is why the 
> `unless(equalsBoundNode(...))` is added. The "arg1" and "arg2" nodes are the 
> direct expressions in the function call, not descendants, and a `new` in the 
> same argument (any descendant) is not matched twice in this way. Otherwise 
> this would show up in failed tests.
> 
> The code `something(std::shared_ptr(new int), std::shared_ptr(new 
> int))` should produce warning (it may be possible that result of the first 
> `new int` is not passed to `shared_ptr` before the other `new int` is called 
> that fails, good solution is use of `std::make_shared` in such case). The 
> test code `(void)f(g(new A), new B);` is somewhat similar AST, the `new A` 
> s

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D147256#4257797 , @hans wrote:

> In D147256#4249527 , @zequanwu 
> wrote:
>
>> - Add a `-use-target-path-separator` flag for llc.
>> - Add test for llc with that flag.
>
> But where does `TM.Options.ObjectFilenameForDebug` come from? Presumably it 
> comes from Clang at some point, so is there any chance we can fix it "at the 
> source" instead?

`TM.Options.ObjectFilenameForDebug` either comes from llc's `-o` or clang-cl's 
`object-file-name=` which is translated from `/Fo[ObjectFileName]`.

For Chromium, the `/Fo[ObjectFileName]` we pass to clang-cl is the same when 
building on Windows and targeting Windows from Linux. The problem comes 
`llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);` in 
`CodeViewDebug.cpp`. That function always convert the path to use host's path 
separator. And I don't think we can write a `remove_dots` function that doesn't 
change path separator, because `\` can only be used as path separator on 
Windows but is a valid path character on Linux.

Or we could just not use `llvm::sys::path::remove_dots`, use the user's input 
as it is for object file path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D147073#4258396 , @aaron.ballman 
wrote:

> In D147073#4258384 , @hans wrote:
>
>> Again not an expert here, but lgtm.
>>
>> (Nit: the 
>> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExprCXX.cpp#L1528-L1530
>>  link in the description seems to point to the wrong code now, since main 
>> changed. Here is a link for 16.0.1: 
>> https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536)
>
> I'm confused -- I thought D147569  resolved 
> the issue and so this patch is no longer needed?

D147569  fixes 
https://github.com/llvm/llvm-project/issues/45481. This one fixes another issue 
crbug.com/1427933. Their stack trace look similar but not caused by the same 
issue.

Updated the link in summary to: 
https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

[Krzysztof Drewniak via Phabricator via cfe-commits]

krzysz00 updated this revision to Diff 512488.
krzysz00 added a comment.

Rebase, since we have addrspace 128


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPU.h
  llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-rtn.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-non-integral-address-spaces.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.atomic.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2darraymsaa.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.3d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.store.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.image.atomic.dim.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.add.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.i8.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.add.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f16.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.load.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.sample.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/addrspacecast-captured.ll
  llvm/test/CodeGen/AMDGPU/annotate-kernel-features-hsa.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.v2f16-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.v2f16-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-intrinsics-mmo-offsets.ll

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 512489.
paulkirth marked 2 inline comments as done.
paulkirth added a comment.

Add spaces to parantheticals. Shorten link to discussion on psABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463

Files:
  clang/docs/ShadowCallStack.rst
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/test/shadowcallstack/lit.cfg.py
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll
  llvm/test/CodeGen/RISCV/saverestore-scs.ll
  llvm/test/CodeGen/RISCV/shadowcallstack.ll

Index: llvm/test/CodeGen/RISCV/shadowcallstack.ll
===
--- llvm/test/CodeGen/RISCV/shadowcallstack.ll
+++ llvm/test/CodeGen/RISCV/shadowcallstack.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV32
-; RUN: llc -mtriple=riscv64 -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV64
 
 define void @f1() shadowcallstack {
@@ -34,9 +34,9 @@
 define i32 @f3() shadowcallstack {
 ; RV32-LABEL: f3:
 ; RV32:   # %bb.0:
-; RV32-NEXT:sw ra, 0(s2)
-; RV32-NEXT:addi s2, s2, 4
-; RV32-NEXT:.cfi_escape 0x16, 0x12, 0x02, 0x82, 0x7c #
+; RV32-NEXT:sw ra, 0(gp)
+; RV32-NEXT:addi gp, gp, 4
+; RV32-NEXT:.cfi_escape 0x16, 0x03, 0x02, 0x73, 0x7c #
 ; RV32-NEXT:addi sp, sp, -16
 ; RV32-NEXT:.cfi_def_cfa_offset 16
 ; RV32-NEXT:sw ra, 12(sp) # 4-byte Folded Spill
@@ -44,16 +44,16 @@
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32-NEXT:addi sp, sp, 16
-; RV32-NEXT:lw ra, -4(s2)
-; RV32-NEXT:addi s2, s2, -4
-; RV32-NEXT:.cfi_restore s2
+; RV32-NEXT:lw ra, -4(gp)
+; RV32-NEXT:addi gp, gp, -4
+; RV32-NEXT:.cfi_restore gp
 ; RV32-NEXT:ret
 ;
 ; RV64-LABEL: f3:
 ; RV64:   # %bb.0:
-; RV64-NEXT:sd ra, 0(s2)
-; RV64-NEXT:addi s2, s2, 8
-; RV64-NEXT:.cfi_escape 0x16, 0x12, 0x02, 0x82, 0x78 #
+; RV64-NEXT:sd ra, 0(gp)
+; RV64-NEXT:addi gp, gp, 8
+; RV64-NEXT:.cfi_escape 0x16, 0x03, 0x02, 0x73, 0x78 #
 ; RV64-NEXT:addi sp, sp, -16
 ; RV64-NEXT:.cfi_def_cfa_offset 16
 ; RV64-NEXT:sd ra, 8(sp) # 8-byte Folded Spill
@@ -61,9 +61,9 @@
 ; RV64-NEXT:call bar@plt
 ; RV64-NEXT:ld ra, 8(sp) # 8-byte Folded Reload
 ; RV64-NEXT:addi sp, sp, 16
-; RV64-NEXT:ld ra, -8(s2)
-; RV64-NEXT:addi s2, s2, -8
-; RV64-NEXT:.cfi_restore s2
+; RV64-NEXT:ld ra, -8(gp)
+; RV64-NEXT:addi gp, gp, -8
+; RV64-NEXT:.cfi_restore gp
 ; RV64-NEXT:ret
   %res = call i32 @bar()
   %res1 = add i32 %res, 1
@@ -73,72 +73,72 @@
 define i32 @f4() shadowcallstack {
 ; RV32-LABEL: f4:
 ; RV32:   # %bb.0:
-; RV32-NEXT:sw ra, 0(s2)
-; RV32-NEXT:addi s2, s2, 4
-; RV32-NEXT:.cfi_escape 0x16, 0x12, 0x02, 0x82, 0x7c #
+; RV32-NEXT:sw ra, 0(gp)
+; RV32-NEXT:addi gp, gp, 4
+; RV32-NEXT:.cfi_escape 0x16, 0x03, 0x02, 0x73, 0x7c #
 ; RV32-NEXT:addi sp, sp, -16
 ; RV32-NEXT:.cfi_def_cfa_offset 16
 ; RV32-NEXT:sw ra, 12(sp) # 4-byte Folded Spill
 ; RV32-NEXT:sw s0, 8(sp) # 4-byte Folded Spill
 ; RV32-NEXT:sw s1, 4(sp) # 4-byte Folded Spill
-; RV32-NEXT:sw s3, 0(sp) # 4-byte Folded Spill
+; RV32-NEXT:sw s2, 0(sp) # 4-byte Folded Spill
 ; RV32-NEXT:.cfi_offset ra, -4
 ; RV32-NEXT:.cfi_offset s0, -8
 ; RV32-NEXT:.cfi_offset s1, -12
-; RV32-NEXT:.cfi_offset s3, -16
+; RV32-NEXT:.cfi_offset s2, -16
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:mv s0, a0
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:mv s1, a0
 ; RV32-NEXT:call bar@plt
-; RV32-NEXT:mv s3, a0
+; RV32-NEXT:mv s2, a0
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:add s0, s0, s1
-; RV32-NEXT:add a0, s3, a0
+; RV32-NEXT:add a0, s2, a0
 ; RV32-NEXT:add a0, s0, a0
 ; RV32-NEXT:lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32-NEXT:lw s0, 8(sp) # 4-byte Folded Reload
 ; RV32-NEXT:lw s1, 4(sp) # 4-byte Folded Reload
-; RV32-NEXT:lw s3, 0(sp) # 4-byte Folded Reload
+; RV32-NEXT:lw s2, 0(sp) # 4-byte Folded Reload
 ; RV32-NEXT:addi sp, sp, 16
-; RV32-NEXT:lw ra, -4(s2)
-; RV32-NEXT:addi s2, s2, -4
-; RV32-NEXT:.cfi_restore s2
+; RV32-NEXT:lw ra, -4(gp)
+; RV32-NEXT:addi gp, gp, -4
+; RV32-NEXT:.cfi_restore gp
 ; RV32-NEXT:ret
 ;
 ; RV64-LABEL: f4:
 ; RV6

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Simeon Ehrig via Phabricator via cfe-commits]

SimeonEhrig added a comment.

Except using an in-memory solution for generated fatbin code, the code looks 
good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146389

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

[PATCH] D148024: [clang-format] Don't modify template arguments on the LHS of assignment

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: owenpan, MyDeveloperDay, HazardyKnusperkeks.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
rymiel requested review of this revision.

After clang-format has determined that an equals sign starts an
expression, it will also go backwards and modify any star/amp/ampamp
binary operators on the left side of the assignment to be
pointers/references instead.

There already exists logic to skip over contents of parentheses and
square brackets, but this patch also expands that logic to apply to
angle brackets. This is so that binary operators inside of template
arguments would not be touched, primary arguments to non-type template
parameters.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148024

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -270,6 +270,18 @@
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
   EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template * = nullptr> void 
f();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1819,7 +1819,7 @@
  Previous && Previous->Previous &&
  !Previous->Previous->isOneOf(tok::comma, tok::semi);
  Previous = Previous->Previous) {
-  if (Previous->isOneOf(tok::r_square, tok::r_paren)) {
+  if (Previous->isOneOf(tok::r_square, tok::r_paren, tok::greater)) {
 Previous = Previous->MatchingParen;
 if (!Previous)
   break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -270,6 +270,18 @@
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
   EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template * = nullptr> void f();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1819,7 +1819,7 @@
  Previous && Previous->Previous &&
  !Previous->Previous->isOneOf(tok::comma, tok::semi);
  Previous = Previous->Previous) {
-  if (Previous->isOneOf(tok::r_square, tok::r_paren)) {
+  if (Previous->isOneOf(tok::r_square, tok::r_paren, tok::greater)) {
 Previous = Previous->MatchingParen;
 if (!Previous)
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D147073#4258426 , @zequanwu wrote:

> In D147073#4258396 , @aaron.ballman 
> wrote:
>
>> In D147073#4258384 , @hans wrote:
>>
>>> Again not an expert here, but lgtm.
>>>
>>> (Nit: the 
>>> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExprCXX.cpp#L1528-L1530
>>>  link in the description seems to point to the wrong code now, since main 
>>> changed. Here is a link for 16.0.1: 
>>> https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536)
>>
>> I'm confused -- I thought D147569  
>> resolved the issue and so this patch is no longer needed?
>
> D147569  fixes 
> https://github.com/llvm/llvm-project/issues/45481. This one fixes another 
> issue crbug.com/1427933. Their stack trace look similar but not caused by the 
> same issue.
>
> Updated the link in summary to: 
> https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536

Thank you for clarifying, I was confused. :-)

I don't think the changes here are correct either -- it's glossing over an 
issue that we're not properly tracking the source location in the AST. Playing 
around with the reduced example is interesting though. If you remove the 
default argument in the `T` constructor, the issue goes away. If you stop using 
a forward declaration in the instantiation of `T`, the issue goes away. If `S1` 
isn't a template, the issue goes away (but the issue will come back if you then 
make `foo()` a template instead of `S1`). So it seems that something about 
template instantiation is dropping the source location information (perhaps) 
and we should be trying to track down where that is to fix the root cause 
rather than work around it here for coverage mapping alone. (The end location 
being incorrect can impact other things that are harder to test because it'll 
be for things like fix-its that don't work properly, which are easy to miss.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D148029: [Clang] Fix crash caused by line splicing in doc comment

[Corentin Jabot via Phabricator via cfe-commits]

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

Because the comment parser does not support slices,
we emit a warning for comments that do contain
a splice within their delimiter, and do not add them as
documentation comment.

Fixes #62054


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148029

Files:
  clang/include/clang/AST/RawCommentList.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/Sema.cpp
  clang/test/Lexer/comment-escape.c

Index: clang/test/Lexer/comment-escape.c
===
--- clang/test/Lexer/comment-escape.c
+++ clang/test/Lexer/comment-escape.c
@@ -1,6 +1,38 @@
-// RUN: %clang -fsyntax-only %s 
+// RUN: %clang -fsyntax-only -Wdocumentation %s
 // rdar://6757323
 // foo \
 
 #define blork 32
 
+// GH62054
+
+/**<*\
+/
+//expected-warning@-2 {{escaped newline between}} \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/**<*\	
+/
+//expected-warning@-2 {{escaped newline between}} \
+//expected-warning@-2 {{backslash and newline separated by space}} \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+
+/*<*\
+/
+//expected-warning@-2 {{escaped newline between}}  \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/*<*\	
+/
+//expected-warning@-2 {{escaped newline between}} \
+//expected-warning@-2 {{backslash and newline separated by space}} \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/\
+*<**/
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/\
+/<*
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -2390,7 +2390,7 @@
   SourceMgr.isInSystemHeader(Comment.getBegin()))
 return;
   RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false);
-  if (RC.isAlmostTrailingComment()) {
+  if (RC.isAlmostTrailingComment() || RC.hasUnsupportedSplice(SourceMgr)) {
 SourceRange MagicMarkerRange(Comment.getBegin(),
  Comment.getBegin().getLocWithOffset(3));
 StringRef MagicMarkerText;
@@ -2401,6 +2401,11 @@
 case RawComment::RCK_OrdinaryC:
   MagicMarkerText = "/**<";
   break;
+case RawComment::RCK_Invalid:
+  // FIXME: are there other scenarios that could produce an invalid
+  // raw comment here?
+  Diag(Comment.getBegin(), diag::warn_splice_in_doxygen_comment);
+  return;
 default:
   llvm_unreachable("if this is an almost Doxygen comment, "
"it should be ordinary");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11377,6 +11377,8 @@
 let CategoryName = "Documentation Issue" in {
 def warn_not_a_doxygen_trailing_member_comment : Warning<
   "not a Doxygen trailing comment">, InGroup, DefaultIgnore;
+def warn_splice_in_doxygen_comment : Warning<
+  "line splicing in Doxygen comments are not supported">, InGroup, DefaultIgnore;
 } // end of documentation issue category
 
 let CategoryName = "Nullability Issue" in {
Index: clang/include/clang/AST/RawCommentList.h
===
--- clang/include/clang/AST/RawCommentList.h
+++ clang/include/clang/AST/RawCommentList.h
@@ -115,6 +115,17 @@
 return extractBriefText(Context);
   }
 
+  bool hasUnsupportedSplice(const SourceManager &SourceMgr) const {
+if (!isInvalid())
+  return false;
+StringRef Text = getRawText(SourceMgr);
+if (Text.size() < 6 || Text[0] != '/')
+  return false;
+if (Text[1] == '*')
+  return Text[Text.size() - 1] != '/' || Text[Text.size() - 2] != '*';
+return Text[1] != '/';
+  }
+
   /// Returns sanitized comment text, suitable for presentation in editor UIs.
   /// E.g. will transform:
   /// // This is a long multiline comment.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143813: [ClangFE] Check that __sync builtins are naturally aligned.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I think the issue is that you're calling EmitPointerWithAlignment() on the 
argument, then calling EmitScalarExpr on the same argument.  Essentially, 
emitting the argument twice.  If emitting the argument has side-effects, that 
will cause an issue.  Sorry, should have spotted that while reviewing. Should 
be straightforward to fix, though; just make CheckAtomicAlignment return the 
computed pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143813

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

[PATCH] D148029: [Clang] Fix crash caused by line splicing in doc comment

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 512505.
cor3ntin added a comment.

Add release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148029

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/RawCommentList.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/Sema.cpp
  clang/test/Lexer/comment-escape.c

Index: clang/test/Lexer/comment-escape.c
===
--- clang/test/Lexer/comment-escape.c
+++ clang/test/Lexer/comment-escape.c
@@ -1,6 +1,38 @@
-// RUN: %clang -fsyntax-only %s 
+// RUN: %clang -fsyntax-only -Wdocumentation %s
 // rdar://6757323
 // foo \
 
 #define blork 32
 
+// GH62054
+
+/**<*\
+/
+//expected-warning@-2 {{escaped newline between}} \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/**<*\	
+/
+//expected-warning@-2 {{escaped newline between}} \
+//expected-warning@-2 {{backslash and newline separated by space}} \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+
+/*<*\
+/
+//expected-warning@-2 {{escaped newline between}}  \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/*<*\	
+/
+//expected-warning@-2 {{escaped newline between}} \
+//expected-warning@-2 {{backslash and newline separated by space}} \
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/\
+*<**/
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
+
+/\
+/<*
+//expected-warning@-2 {{line splicing in Doxygen comments are not supported}}
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -2390,7 +2390,7 @@
   SourceMgr.isInSystemHeader(Comment.getBegin()))
 return;
   RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false);
-  if (RC.isAlmostTrailingComment()) {
+  if (RC.isAlmostTrailingComment() || RC.hasUnsupportedSplice(SourceMgr)) {
 SourceRange MagicMarkerRange(Comment.getBegin(),
  Comment.getBegin().getLocWithOffset(3));
 StringRef MagicMarkerText;
@@ -2401,6 +2401,11 @@
 case RawComment::RCK_OrdinaryC:
   MagicMarkerText = "/**<";
   break;
+case RawComment::RCK_Invalid:
+  // FIXME: are there other scenarios that could produce an invalid
+  // raw comment here?
+  Diag(Comment.getBegin(), diag::warn_splice_in_doxygen_comment);
+  return;
 default:
   llvm_unreachable("if this is an almost Doxygen comment, "
"it should be ordinary");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11377,6 +11377,8 @@
 let CategoryName = "Documentation Issue" in {
 def warn_not_a_doxygen_trailing_member_comment : Warning<
   "not a Doxygen trailing comment">, InGroup, DefaultIgnore;
+def warn_splice_in_doxygen_comment : Warning<
+  "line splicing in Doxygen comments are not supported">, InGroup, DefaultIgnore;
 } // end of documentation issue category
 
 let CategoryName = "Nullability Issue" in {
Index: clang/include/clang/AST/RawCommentList.h
===
--- clang/include/clang/AST/RawCommentList.h
+++ clang/include/clang/AST/RawCommentList.h
@@ -115,6 +115,17 @@
 return extractBriefText(Context);
   }
 
+  bool hasUnsupportedSplice(const SourceManager &SourceMgr) const {
+if (!isInvalid())
+  return false;
+StringRef Text = getRawText(SourceMgr);
+if (Text.size() < 6 || Text[0] != '/')
+  return false;
+if (Text[1] == '*')
+  return Text[Text.size() - 1] != '/' || Text[Text.size() - 2] != '*';
+return Text[1] != '/';
+  }
+
   /// Returns sanitized comment text, suitable for presentation in editor UIs.
   /// E.g. will transform:
   /// // This is a long multiline comment.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -290,6 +290,8 @@
   (`#61142 `_)
 - Clang now better diagnose placeholder types constrained with a concept that is
   not a type concept.
+- Fix crash when a doc comment contains a line splicing.
+  (`#62054 `_)
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148031: [clang][driver][NFC] Add hasShadowCallStack to SanitizerArgs

[Paul Kirth via Phabricator via cfe-commits]

paulkirth created this revision.
paulkirth added reviewers: phosek, abrachet, dvyukov, vitalybuka, aaron.ballman.
Herald added a project: All.
paulkirth requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, we can't check if ShadowCallStack is present in Args the same
way we handle other sanitizers. This is preparatory work for planned
driver changes to how we handle ShadowCallStack.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148031

Files:
  clang/include/clang/Driver/SanitizerArgs.h


Index: clang/include/clang/Driver/SanitizerArgs.h
===
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -119,6 +119,10 @@
 return MemtagMode;
   }
 
+  bool hasShadowCallStack() const {
+return Sanitizers.has(SanitizerKind::ShadowCallStack);
+  }
+
   bool requiresPIE() const;
   bool needsUnwindTables() const;
   bool needsLTO() const;


Index: clang/include/clang/Driver/SanitizerArgs.h
===
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -119,6 +119,10 @@
 return MemtagMode;
   }
 
+  bool hasShadowCallStack() const {
+return Sanitizers.has(SanitizerKind::ShadowCallStack);
+  }
+
   bool requiresPIE() const;
   bool needsUnwindTables() const;
   bool needsLTO() const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147848: [clang] Add test for CWG2370

[Shafik Yaghmour via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/test/CXX/drs/dr23xx.cpp:182
+  typedef N::type N_type;
+  // FIXME: `type` should be searched for in N
+  // friend void N::g(type);

Endill wrote:
> shafik wrote:
> > The implementation seems to all accept this example: 
> > https://godbolt.org/z/vE6bEP6xa
> > 
> > but the examples from the `p1787` have a decidely mixed conformance: 
> > https://godbolt.org/z/dhq7oEKaY
> > 
> > but the `A::F(F)` you point out in your example clang does get wrong and 
> > gcc does not. So at minimum please file bug reports against the examples 
> > that clang does not get right from `p1787` and we need to dig into why your 
> > example above seems to not the same since that is what you intended. 
> Are you sure behavior is different for my `N::g(type)` when compared to 
> `A::f(F)`? Clang is the only one to reject both examples: 
> https://godbolt.org/z/MKb6fE8K5
Oh wow, I someone how missed the commented line of code and thought you were 
referring to the uncomment line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147848

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D147073#4258529 , @aaron.ballman 
wrote:

> In D147073#4258426 , @zequanwu 
> wrote:
>
>> In D147073#4258396 , 
>> @aaron.ballman wrote:
>>
>>> In D147073#4258384 , @hans wrote:
>>>
 Again not an expert here, but lgtm.

 (Nit: the 
 https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExprCXX.cpp#L1528-L1530
  link in the description seems to point to the wrong code now, since main 
 changed. Here is a link for 16.0.1: 
 https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536)
>>>
>>> I'm confused -- I thought D147569  
>>> resolved the issue and so this patch is no longer needed?
>>
>> D147569  fixes 
>> https://github.com/llvm/llvm-project/issues/45481. This one fixes another 
>> issue crbug.com/1427933. Their stack trace look similar but not caused by 
>> the same issue.
>>
>> Updated the link in summary to: 
>> https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536
>
> Thank you for clarifying, I was confused. :-)
>
> I don't think the changes here are correct either -- it's glossing over an 
> issue that we're not properly tracking the source location in the AST. 
> Playing around with the reduced example is interesting though. If you remove 
> the default argument in the `T` constructor, the issue goes away. If you stop 
> using a forward declaration in the instantiation of `T`, the issue goes away. 
> If `S1` isn't a template, the issue goes away (but the issue will come back 
> if you then make `foo()` a template instead of `S1`). So it seems that 
> something about template instantiation is dropping the source location 
> information (perhaps) and we should be trying to track down where that is to 
> fix the root cause rather than work around it here for coverage mapping 
> alone. (The end location being incorrect can impact other things that are 
> harder to test because it'll be for things like fix-its that don't work 
> properly, which are easy to miss.)

I agree that the real fix is to fix the source location information. If I just 
change [1] to `SourceRange Locs = SourceRange(LParenOrBraceLoc, 
RParenOrBraceLoc);`, the crash is gone. However, as the "FIXME" comment in [1] 
says, it's an intended work-around to drop source locations here. So, I'm 
assuming this kind of source location work-around could happen in multiple 
places in clang, and could happen in the future as a temporary work-around. 
Instead of crashing clang coverage for those work-around, we can at least skip 
coverage info for those expressions/statements.

[1]: 
https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1538


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D147920: [clang] Add test for CWG399

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a subscriber: aaron.ballman.
shafik added inline comments.



Comment at: clang/test/CXX/drs/dr3xx.cpp:1439
+
+namespace dr399 { // dr399: 11
+  // NB: reuse dr244 test 

Endill wrote:
> Despite a couple of FIXME in CWG244 test (out of dozens of examples), it 
> claims full availability since Clang 11. I'd take a more conservative 
> approach, declaring partial support, but I think that declaring different 
> availability for the same test would bring unnecessary confusion. So I 
> followed CWG244 availability.
> 
> Alternative is to demote CWG244 to partial, but I'm not sure we should go 
> back on our claims for CWG support that has been out for so long.
I think the bugs are not awful, we should file bug reports if we don't already 
have them. Some of them seem like they should be not too bad to fix.

CC @aaron.ballman to get a second opinion



Comment at: clang/test/CXX/drs/dr3xx.cpp:1492
+// This is technically ill-formed; G is looked up in 'N::' and is not 
found.
+// Rejecting this seems correct, but most compilers accept, so we do also.
+f.N::F::~G(); // expected-error {{qualified destructor name only found in 
lexical scope; omit the qualifier to find this type name by unqualified lookup}}

Endill wrote:
> shafik wrote:
> > You say we accept the next line but it has an `expected-error` on it?
> It's an error because of `-pedantic-errors`. It's a warning by default.
That makes a lot more sense, I was wondering what was I missing.

Can we note that in the comment b/c it is pretty confusing otherwise. 

I wonder if there is a good reason to not make this ill-formed by default? 
Worth a bug report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147920

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

[PATCH] D147920: [clang] Add test for CWG399

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D147920#4257369 , @Endill wrote:

> I think I haven't stressed it enough, but this whole test is copied from 
> dr244, which is written by Richard.

Understood, I appreciate the patience in explaining what I am missing. 
Sometimes that means things could be explained better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147920

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

[PATCH] D147839: [clang] Add test for CWG2007

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D147839#4257394 , @Endill wrote:

> Replace unary `&` with `__builtin_addressof()`. It prevents unnecessary 
> template instantiation (presumably to find overloaded unary `&`), and make 
> Clang compliant since 3.4

Nice approach!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147839

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

[PATCH] D147839: [clang] Add test for CWG2007

[Shafik Yaghmour via Phabricator via cfe-commits]

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

LGTM, thank you again for all the effort in documenting these and adding tests, 
it is much appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147839

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

[PATCH] D147839: [clang] Add test for CWG2007

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

In D147839#4258681 , @shafik wrote:

> In D147839#4257394 , @Endill wrote:
>
>> Replace unary `&` with `__builtin_addressof()`. It prevents unnecessary 
>> template instantiation (presumably to find overloaded unary `&`), and make 
>> Clang compliant since 3.4
>
> Nice approach!

Credits go to Aaron, actually. We discussed this DR during his office hours 
yesterday.

In D147839#4258682 , @shafik wrote:

> LGTM, thank you again for all the effort in documenting these and adding 
> tests, it is much appreciated.

Thank you for giving them thorough review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147839

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

[clang] 393a1c3 - [PS4][clang] Pass -flto-jobs argument to orbis-ld

[Matthew Voss via cfe-commits]

Author: Matthew Voss
Date: 2023-04-11T10:33:16-07:00
New Revision: 393a1c3b4fcded56a1078d2c01c69af2f2ec05cf

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

LOG: [PS4][clang] Pass -flto-jobs argument to orbis-ld

Pass -flto-jobs to orbis-ld correctly.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/PS4CPU.cpp
clang/test/Driver/lto-jobs.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp 
b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 71c6b650e1f52..b280abb0d58b7 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -180,6 +180,14 @@ void tools::PScpu::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
 if (Arg *A = Args.getLastArg(options::OPT_fcrash_diagnostics_dir))
   AddCodeGenFlag(Twine("-crash-diagnostics-dir=") + A->getValue());
 
+StringRef Parallelism = getLTOParallelism(Args, D);
+if (!Parallelism.empty()) {
+  if (IsPS4)
+AddCodeGenFlag(Twine("-threads=") + Parallelism);
+  else
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=jobs=") + 
Parallelism));
+}
+
 if (IsPS4) {
   const char *Prefix = nullptr;
   if (D.getLTOMode() == LTOK_Thin)
@@ -193,12 +201,6 @@ void tools::PScpu::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
 }
   }
 
-  if (IsPS5 && UseLTO) {
-StringRef Parallelism = getLTOParallelism(Args, D);
-if (!Parallelism.empty())
-  CmdArgs.push_back(Args.MakeArgString("-plugin-opt=jobs=" + Parallelism));
-  }
-
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
 TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 

diff  --git a/clang/test/Driver/lto-jobs.c b/clang/test/Driver/lto-jobs.c
index 443f8abced788..5402442ce6972 100644
--- a/clang/test/Driver/lto-jobs.c
+++ b/clang/test/Driver/lto-jobs.c
@@ -7,6 +7,11 @@
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS-ACTION < %t %s
 //
 // CHECK-LINK-THIN-JOBS-ACTION: "-plugin-opt=jobs=5"
+//
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin -flto-jobs=5 2> %t
+// RUN: FileCheck -check-prefix=CHECK-PS4-LINK-THIN-JOBS-ACTION < %t %s
+//
+// CHECK-PS4-LINK-THIN-JOBS-ACTION: "-lto-thin-debug-options= 
-generate-arange-section -threads=5"
 
 // RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin 
-flto-jobs=5 2> %t
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS2-ACTION < %t %s



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

[PATCH] D147660: [PS4][clang] Pass -flto-jobs argument to orbis-ld

[Matthew Voss 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 rG393a1c3b4fcd: [PS4][clang] Pass -flto-jobs argument to 
orbis-ld (authored by ormris).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147660

Files:
  clang/lib/Driver/ToolChains/PS4CPU.cpp
  clang/test/Driver/lto-jobs.c


Index: clang/test/Driver/lto-jobs.c
===
--- clang/test/Driver/lto-jobs.c
+++ clang/test/Driver/lto-jobs.c
@@ -7,6 +7,11 @@
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS-ACTION < %t %s
 //
 // CHECK-LINK-THIN-JOBS-ACTION: "-plugin-opt=jobs=5"
+//
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin -flto-jobs=5 2> %t
+// RUN: FileCheck -check-prefix=CHECK-PS4-LINK-THIN-JOBS-ACTION < %t %s
+//
+// CHECK-PS4-LINK-THIN-JOBS-ACTION: "-lto-thin-debug-options= 
-generate-arange-section -threads=5"
 
 // RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin 
-flto-jobs=5 2> %t
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS2-ACTION < %t %s
Index: clang/lib/Driver/ToolChains/PS4CPU.cpp
===
--- clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -180,6 +180,14 @@
 if (Arg *A = Args.getLastArg(options::OPT_fcrash_diagnostics_dir))
   AddCodeGenFlag(Twine("-crash-diagnostics-dir=") + A->getValue());
 
+StringRef Parallelism = getLTOParallelism(Args, D);
+if (!Parallelism.empty()) {
+  if (IsPS4)
+AddCodeGenFlag(Twine("-threads=") + Parallelism);
+  else
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=jobs=") + 
Parallelism));
+}
+
 if (IsPS4) {
   const char *Prefix = nullptr;
   if (D.getLTOMode() == LTOK_Thin)
@@ -193,12 +201,6 @@
 }
   }
 
-  if (IsPS5 && UseLTO) {
-StringRef Parallelism = getLTOParallelism(Args, D);
-if (!Parallelism.empty())
-  CmdArgs.push_back(Args.MakeArgString("-plugin-opt=jobs=" + Parallelism));
-  }
-
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
 TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 


Index: clang/test/Driver/lto-jobs.c
===
--- clang/test/Driver/lto-jobs.c
+++ clang/test/Driver/lto-jobs.c
@@ -7,6 +7,11 @@
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS-ACTION < %t %s
 //
 // CHECK-LINK-THIN-JOBS-ACTION: "-plugin-opt=jobs=5"
+//
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin -flto-jobs=5 2> %t
+// RUN: FileCheck -check-prefix=CHECK-PS4-LINK-THIN-JOBS-ACTION < %t %s
+//
+// CHECK-PS4-LINK-THIN-JOBS-ACTION: "-lto-thin-debug-options= -generate-arange-section -threads=5"
 
 // RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin -flto-jobs=5 2> %t
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS2-ACTION < %t %s
Index: clang/lib/Driver/ToolChains/PS4CPU.cpp
===
--- clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -180,6 +180,14 @@
 if (Arg *A = Args.getLastArg(options::OPT_fcrash_diagnostics_dir))
   AddCodeGenFlag(Twine("-crash-diagnostics-dir=") + A->getValue());
 
+StringRef Parallelism = getLTOParallelism(Args, D);
+if (!Parallelism.empty()) {
+  if (IsPS4)
+AddCodeGenFlag(Twine("-threads=") + Parallelism);
+  else
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=jobs=") + Parallelism));
+}
+
 if (IsPS4) {
   const char *Prefix = nullptr;
   if (D.getLTOMode() == LTOK_Thin)
@@ -193,12 +201,6 @@
 }
   }
 
-  if (IsPS5 && UseLTO) {
-StringRef Parallelism = getLTOParallelism(Args, D);
-if (!Parallelism.empty())
-  CmdArgs.push_back(Args.MakeArgString("-plugin-opt=jobs=" + Parallelism));
-  }
-
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
 TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147660: [PS4][clang] Pass -flto-jobs argument to orbis-ld

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147660

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

[clang] 5e2d8a3 - [RISCV] Remove getCPUFeaturesExceptStdExt.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2023-04-11T10:41:46-07:00
New Revision: 5e2d8a35288802f94df8c60513e7a835b27e621d

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

LOG: [RISCV] Remove getCPUFeaturesExceptStdExt.

This function was used to force +64bit or -64bit into the feature
string basd on -mcpu.

It's not entirely clear to me why this was needed.  This informationo
is redundant with the triple. RISCVTargetInfo::initFeatureMap
independently recomputes it from the triple for the feature map.

It is ultimately needed in the backend, but that should be handled
by RISCVSubtarget processing the CPU name.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/RISCV.cpp
clang/test/Driver/riscv-cpus.c
llvm/include/llvm/TargetParser/RISCVTargetParser.h
llvm/lib/TargetParser/RISCVTargetParser.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp 
b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index da120d95b42f..88744210 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -53,8 +53,7 @@ static bool getRISCFeaturesFromMcpu(const llvm::Triple 
&Triple, StringRef Mcpu,
 std::vector &Features) {
   bool Is64Bit = Triple.isRISCV64();
   llvm::RISCV::CPUKind CPUKind = llvm::RISCV::parseCPUKind(Mcpu);
-  return llvm::RISCV::checkCPUKind(CPUKind, Is64Bit) &&
- llvm::RISCV::getCPUFeaturesExceptStdExt(CPUKind, Features);
+  return llvm::RISCV::checkCPUKind(CPUKind, Is64Bit);
 }
 
 void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,

diff  --git a/clang/test/Driver/riscv-cpus.c b/clang/test/Driver/riscv-cpus.c
index 7a08b8c10956..a57cbf1e0e25 100644
--- a/clang/test/Driver/riscv-cpus.c
+++ b/clang/test/Driver/riscv-cpus.c
@@ -7,20 +7,17 @@
 // RUN: %clang --target=riscv64 -### -c %s 2>&1 -mcpu=rocket-rv64 | FileCheck 
-check-prefix=MCPU-ROCKET64 %s
 // MCPU-ROCKET64: "-nostdsysteminc" "-target-cpu" "rocket-rv64"
 // MCPU-ROCKET64: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-ROCKET64: "-target-feature" "+64bit"
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=syntacore-scr1-base | 
FileCheck -check-prefix=MCPU-SYNTACORE-SCR1-BASE %s
 // MCPU-SYNTACORE-SCR1-BASE: "-target-cpu" "syntacore-scr1-base"
 // MCPU-SYNTACORE-SCR1-BASE: "-target-feature" "+c"
 // MCPU-SYNTACORE-SCR1-BASE: "-target-feature" "+zicsr" "-target-feature" 
"+zifencei"
-// MCPU-SYNTACORE-SCR1-BASE: "-target-feature" "-64bit"
 // MCPU-SYNTACORE-SCR1-BASE: "-target-abi" "ilp32"
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=syntacore-scr1-max | 
FileCheck -check-prefix=MCPU-SYNTACORE-SCR1-MAX %s
 // MCPU-SYNTACORE-SCR1-MAX: "-target-cpu" "syntacore-scr1-max"
 // MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "+m" "-target-feature" "+c"
 // MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "+zicsr" "-target-feature" 
"+zifencei"
-// MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "-64bit"
 // MCPU-SYNTACORE-SCR1-MAX: "-target-abi" "ilp32"
 
 // We cannot check much for -mcpu=native, but it should be replaced by a valid 
CPU string.
@@ -92,7 +89,6 @@
 // MCPU-ABI-SIFIVE-S21: "-target-feature" "+m" "-target-feature" "+a"
 // MCPU-ABI-SIFIVE-S21: "-target-feature" "+c"
 // MCPU-ABI-SIFIVE-S21: "-target-feature" "+zicsr" "-target-feature" 
"+zifencei"
-// MCPU-ABI-SIFIVE-S21: "-target-feature" "+64bit"
 // MCPU-ABI-SIFIVE-S21: "-target-abi" "lp64"
 
 // mcpu with mabi option
@@ -101,7 +97,6 @@
 // MCPU-ABI-SIFIVE-S51: "-target-feature" "+m" "-target-feature" "+a"
 // MCPU-ABI-SIFIVE-S51: "-target-feature" "+c"
 // MCPU-ABI-SIFIVE-S51: "-target-feature" "+zicsr" "-target-feature" 
"+zifencei"
-// MCPU-ABI-SIFIVE-S51: "-target-feature" "+64bit"
 // MCPU-ABI-SIFIVE-S51: "-target-abi" "lp64"
 
 // mcpu with default march
@@ -110,7 +105,6 @@
 // MCPU-SIFIVE-S54: "-target-feature" "+m" "-target-feature" "+a" 
"-target-feature" "+f" "-target-feature" "+d"
 // MCPU-SIFIVE-S54: "-target-feature" "+c"
 // MCPU-SIFIVE-S54: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-SIFIVE-S54: "-target-feature" "+64bit"
 // MCPU-SIFIVE-S54: "-target-abi" "lp64d"
 
 // mcpu with mabi option
@@ -119,7 +113,6 @@
 // MCPU-SIFIVE-S76: "-target-feature" "+m" "-target-feature" "+a" 
"-target-feature" "+f" "-target-feature" "+d"
 // MCPU-SIFIVE-S76: "-target-feature" "+c"
 // MCPU-SIFIVE-S76: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-SIFIVE-S76: "-target-feature" "+64bit"
 // MCPU-SIFIVE-S76: "-target-abi" "lp64d"
 
 // mcpu with default march
@@ -128,7 +121,6 @@
 // MCPU-SIFIVE-U54: "-target-feature" "+m" "-target-feature" "+a" 
"-target-feature" "+f" "-target-feature" "+d"
 // MCPU-SIFIVE-

[clang] 88d6311 - [RISCV] Print a better error message when a rv32 CPU is used on rv64 and vice versa.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2023-04-11T10:41:50-07:00
New Revision: 88d6311982557acf2dd04ce101aca077ac311012

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

LOG: [RISCV] Print a better error message when a rv32 CPU is used on rv64 and 
vice versa.

Instead of rejecting the CPU outright with no information, try
to diagnose that it doesn't match the triple.

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/Arch/RISCV.cpp
clang/test/Driver/riscv-cpus.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 2c6bb0aa9331..1efe7573028e 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -29,6 +29,8 @@ def err_drv_invalid_arch_name : Error<
   "invalid arch name '%0'">;
 def err_drv_invalid_riscv_arch_name : Error<
   "invalid arch name '%0', %1">;
+def err_drv_invalid_riscv_cpu_name_for_target : Error<
+  "cpu '%0' does not support rv%select{32|64}1">;
 def warn_drv_invalid_arch_name_with_suggestion : Warning<
   "ignoring invalid /arch: argument '%0'; for %select{64|32}1-bit expected one 
of %2">,
   InGroup;

diff  --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp 
b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 88744210..7cf01ad8a72b 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -49,11 +49,20 @@ static bool getArchFeatures(const Driver &D, StringRef Arch,
 }
 
 // Get features except standard extension feature
-static bool getRISCFeaturesFromMcpu(const llvm::Triple &Triple, StringRef Mcpu,
+static void getRISCFeaturesFromMcpu(const Driver &D, const Arg *A,
+const llvm::Triple &Triple,
+StringRef Mcpu,
 std::vector &Features) {
   bool Is64Bit = Triple.isRISCV64();
   llvm::RISCV::CPUKind CPUKind = llvm::RISCV::parseCPUKind(Mcpu);
-  return llvm::RISCV::checkCPUKind(CPUKind, Is64Bit);
+  if (!llvm::RISCV::checkCPUKind(CPUKind, Is64Bit)) {
+// Try inverting Is64Bit in case the CPU is valid, but for the wrong 
target.
+if (llvm::RISCV::checkCPUKind(CPUKind, !Is64Bit))
+  D.Diag(clang::diag::err_drv_invalid_riscv_cpu_name_for_target) << Mcpu 
<< Is64Bit;
+else
+  D.Diag(clang::diag::err_drv_unsupported_option_argument)
+  << A->getSpelling() << Mcpu;
+  }
 }
 
 void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
@@ -70,9 +79,8 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
 StringRef CPU = A->getValue();
 if (CPU == "native")
   CPU = llvm::sys::getHostCPUName();
-if (!getRISCFeaturesFromMcpu(Triple, CPU, Features))
-  D.Diag(clang::diag::err_drv_unsupported_option_argument)
-  << A->getSpelling() << CPU;
+
+getRISCFeaturesFromMcpu(D, A, Triple, CPU, Features);
   }
 
   // Handle features corresponding to "-ffixed-X" options

diff  --git a/clang/test/Driver/riscv-cpus.c b/clang/test/Driver/riscv-cpus.c
index a57cbf1e0e25..b107d89773e1 100644
--- a/clang/test/Driver/riscv-cpus.c
+++ b/clang/test/Driver/riscv-cpus.c
@@ -173,7 +173,7 @@
 // FAIL-MCPU-NAME: error: unsupported argument 'generic-rv321' to option 
'-mcpu='
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv32 
-march=rv64i | FileCheck -check-prefix=MISMATCH-ARCH %s
-// MISMATCH-ARCH: error: unsupported argument 'generic-rv32' to option '-mcpu='
+// MISMATCH-ARCH: cpu 'generic-rv32' does not support rv64
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv64 | FileCheck 
-check-prefix=MISMATCH-MCPU %s
-// MISMATCH-MCPU: error: unsupported argument 'generic-rv64' to option '-mcpu='
+// MISMATCH-MCPU: error: cpu 'generic-rv64' does not support rv32



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

[PATCH] D147986: [RISCV] Print a better error message when a rv32 CPU is used on rv64 and vice versa.

[Craig Topper 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 rG88d631198255: [RISCV] Print a better error message when a 
rv32 CPU is used on rv64 and vice… (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147986

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-cpus.c


Index: clang/test/Driver/riscv-cpus.c
===
--- clang/test/Driver/riscv-cpus.c
+++ clang/test/Driver/riscv-cpus.c
@@ -173,7 +173,7 @@
 // FAIL-MCPU-NAME: error: unsupported argument 'generic-rv321' to option 
'-mcpu='
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv32 
-march=rv64i | FileCheck -check-prefix=MISMATCH-ARCH %s
-// MISMATCH-ARCH: error: unsupported argument 'generic-rv32' to option '-mcpu='
+// MISMATCH-ARCH: cpu 'generic-rv32' does not support rv64
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv64 | FileCheck 
-check-prefix=MISMATCH-MCPU %s
-// MISMATCH-MCPU: error: unsupported argument 'generic-rv64' to option '-mcpu='
+// MISMATCH-MCPU: error: cpu 'generic-rv64' does not support rv32
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -49,11 +49,20 @@
 }
 
 // Get features except standard extension feature
-static bool getRISCFeaturesFromMcpu(const llvm::Triple &Triple, StringRef Mcpu,
+static void getRISCFeaturesFromMcpu(const Driver &D, const Arg *A,
+const llvm::Triple &Triple,
+StringRef Mcpu,
 std::vector &Features) {
   bool Is64Bit = Triple.isRISCV64();
   llvm::RISCV::CPUKind CPUKind = llvm::RISCV::parseCPUKind(Mcpu);
-  return llvm::RISCV::checkCPUKind(CPUKind, Is64Bit);
+  if (!llvm::RISCV::checkCPUKind(CPUKind, Is64Bit)) {
+// Try inverting Is64Bit in case the CPU is valid, but for the wrong 
target.
+if (llvm::RISCV::checkCPUKind(CPUKind, !Is64Bit))
+  D.Diag(clang::diag::err_drv_invalid_riscv_cpu_name_for_target) << Mcpu 
<< Is64Bit;
+else
+  D.Diag(clang::diag::err_drv_unsupported_option_argument)
+  << A->getSpelling() << Mcpu;
+  }
 }
 
 void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
@@ -70,9 +79,8 @@
 StringRef CPU = A->getValue();
 if (CPU == "native")
   CPU = llvm::sys::getHostCPUName();
-if (!getRISCFeaturesFromMcpu(Triple, CPU, Features))
-  D.Diag(clang::diag::err_drv_unsupported_option_argument)
-  << A->getSpelling() << CPU;
+
+getRISCFeaturesFromMcpu(D, A, Triple, CPU, Features);
   }
 
   // Handle features corresponding to "-ffixed-X" options
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -29,6 +29,8 @@
   "invalid arch name '%0'">;
 def err_drv_invalid_riscv_arch_name : Error<
   "invalid arch name '%0', %1">;
+def err_drv_invalid_riscv_cpu_name_for_target : Error<
+  "cpu '%0' does not support rv%select{32|64}1">;
 def warn_drv_invalid_arch_name_with_suggestion : Warning<
   "ignoring invalid /arch: argument '%0'; for %select{64|32}1-bit expected one 
of %2">,
   InGroup;


Index: clang/test/Driver/riscv-cpus.c
===
--- clang/test/Driver/riscv-cpus.c
+++ clang/test/Driver/riscv-cpus.c
@@ -173,7 +173,7 @@
 // FAIL-MCPU-NAME: error: unsupported argument 'generic-rv321' to option '-mcpu='
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv32 -march=rv64i | FileCheck -check-prefix=MISMATCH-ARCH %s
-// MISMATCH-ARCH: error: unsupported argument 'generic-rv32' to option '-mcpu='
+// MISMATCH-ARCH: cpu 'generic-rv32' does not support rv64
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv64 | FileCheck -check-prefix=MISMATCH-MCPU %s
-// MISMATCH-MCPU: error: unsupported argument 'generic-rv64' to option '-mcpu='
+// MISMATCH-MCPU: error: cpu 'generic-rv64' does not support rv32
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -49,11 +49,20 @@
 }
 
 // Get features except standard extension feature
-static bool getRISCFeaturesFromMcpu(const llvm::Triple &Triple, StringRef Mcpu,
+static void getRISCFeaturesFromMcpu(const Driver &D, const Arg *A,
+const llvm::Triple &Triple,
+ 

[PATCH] D147978: [RISCV] Remove getCPUFeaturesExceptStdExt.

[Craig Topper via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e2d8a352888: [RISCV] Remove getCPUFeaturesExceptStdExt. 
(authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147978

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/lib/TargetParser/RISCVTargetParser.cpp

Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -85,22 +85,6 @@
 #include "llvm/TargetParser/RISCVTargetParserDef.inc"
 }
 
-// Get all features except standard extension feature
-bool getCPUFeaturesExceptStdExt(CPUKind Kind,
-std::vector &Features) {
-  const CPUInfo &Info = RISCVCPUInfo[static_cast(Kind)];
-
-  if (Info.isInvalid())
-return false;
-
-  if (Info.is64Bit())
-Features.push_back("+64bit");
-  else
-Features.push_back("-64bit");
-
-  return true;
-}
-
 bool isX18ReservedByDefault(const Triple &TT) {
   // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
   return TT.isOSFuchsia() || TT.isAndroid();
Index: llvm/include/llvm/TargetParser/RISCVTargetParser.h
===
--- llvm/include/llvm/TargetParser/RISCVTargetParser.h
+++ llvm/include/llvm/TargetParser/RISCVTargetParser.h
@@ -39,7 +39,6 @@
 StringRef getMArchFromMcpu(StringRef CPU);
 void fillValidCPUArchList(SmallVectorImpl &Values, bool IsRV64);
 void fillValidTuneCPUArchList(SmallVectorImpl &Values, bool IsRV64);
-bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector &Features);
 
 bool isX18ReservedByDefault(const Triple &TT);
 
Index: clang/test/Driver/riscv-cpus.c
===
--- clang/test/Driver/riscv-cpus.c
+++ clang/test/Driver/riscv-cpus.c
@@ -7,20 +7,17 @@
 // RUN: %clang --target=riscv64 -### -c %s 2>&1 -mcpu=rocket-rv64 | FileCheck -check-prefix=MCPU-ROCKET64 %s
 // MCPU-ROCKET64: "-nostdsysteminc" "-target-cpu" "rocket-rv64"
 // MCPU-ROCKET64: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-ROCKET64: "-target-feature" "+64bit"
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=syntacore-scr1-base | FileCheck -check-prefix=MCPU-SYNTACORE-SCR1-BASE %s
 // MCPU-SYNTACORE-SCR1-BASE: "-target-cpu" "syntacore-scr1-base"
 // MCPU-SYNTACORE-SCR1-BASE: "-target-feature" "+c"
 // MCPU-SYNTACORE-SCR1-BASE: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-SYNTACORE-SCR1-BASE: "-target-feature" "-64bit"
 // MCPU-SYNTACORE-SCR1-BASE: "-target-abi" "ilp32"
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=syntacore-scr1-max | FileCheck -check-prefix=MCPU-SYNTACORE-SCR1-MAX %s
 // MCPU-SYNTACORE-SCR1-MAX: "-target-cpu" "syntacore-scr1-max"
 // MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "+m" "-target-feature" "+c"
 // MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "-64bit"
 // MCPU-SYNTACORE-SCR1-MAX: "-target-abi" "ilp32"
 
 // We cannot check much for -mcpu=native, but it should be replaced by a valid CPU string.
@@ -92,7 +89,6 @@
 // MCPU-ABI-SIFIVE-S21: "-target-feature" "+m" "-target-feature" "+a"
 // MCPU-ABI-SIFIVE-S21: "-target-feature" "+c"
 // MCPU-ABI-SIFIVE-S21: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-ABI-SIFIVE-S21: "-target-feature" "+64bit"
 // MCPU-ABI-SIFIVE-S21: "-target-abi" "lp64"
 
 // mcpu with mabi option
@@ -101,7 +97,6 @@
 // MCPU-ABI-SIFIVE-S51: "-target-feature" "+m" "-target-feature" "+a"
 // MCPU-ABI-SIFIVE-S51: "-target-feature" "+c"
 // MCPU-ABI-SIFIVE-S51: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-ABI-SIFIVE-S51: "-target-feature" "+64bit"
 // MCPU-ABI-SIFIVE-S51: "-target-abi" "lp64"
 
 // mcpu with default march
@@ -110,7 +105,6 @@
 // MCPU-SIFIVE-S54: "-target-feature" "+m" "-target-feature" "+a" "-target-feature" "+f" "-target-feature" "+d"
 // MCPU-SIFIVE-S54: "-target-feature" "+c"
 // MCPU-SIFIVE-S54: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-SIFIVE-S54: "-target-feature" "+64bit"
 // MCPU-SIFIVE-S54: "-target-abi" "lp64d"
 
 // mcpu with mabi option
@@ -119,7 +113,6 @@
 // MCPU-SIFIVE-S76: "-target-feature" "+m" "-target-feature" "+a" "-target-feature" "+f" "-target-feature" "+d"
 // MCPU-SIFIVE-S76: "-target-feature" "+c"
 // MCPU-SIFIVE-S76: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
-// MCPU-SIFIVE-S76: "-target-feature" "+64bit"
 // MCPU-SIFIVE-S76: "-target-abi" "lp64d"
 
 // mcpu with default march
@@ -128,7 +121,6 @@
 // MCPU-SIFIVE-U54: "-target-feature" "+m" "-target-feature" "+a" "-target-feature" "+f" "-target-feature" "+d"
 // MCPU-SIFIVE

[PATCH] D143813: [ClangFE] Check that __sync builtins are naturally aligned.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In D143813#4257943 , @jonpa wrote:

> I don't understand the first argument - I thought it was supposed to be just 
> an address...

It's a statement expression. 
https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

The value of the last subexpression serves as the value of the entire construct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143813

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