[PATCH] D121441: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:1
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -verify -triple powerpc-unkonwn-unknown -emit-llvm -o - %s 
| \

lkail wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > I am not sure about having a `CodeGen` test for this when a 
> > > `-fsyntax-only` `Layout` test would do. I believe that also removes the 
> > > need for `powerpc-registered-target`.
> > I will note that there is a concept of "preferred alignment" or "complete 
> > object alignment" that may differ from "required alignment". A `CodeGen` 
> > test like this can miss issues if the "required alignment" is wrong but the 
> > "preferred alignment" or "complete object alignment" matches what is 
> > expected.
> That's a good point. However currently, I haven't found any tests in 
> `test/Sema` performing such check and I have't figure out a way to add such 
> check. Maybe checking `__alignof__() % (expected_align) == 0` is more 
> practical.
There are tests in `test/Layout` that might be closer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121441

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/include/clang/Sema/Scope.h:518
 
   void addNRVOCandidate(VarDecl *VD) {
+// every candidate except VD is "spoiled" now, remove them from the set

Firstly I am wondering why here doesn't check `NRVO.getInt()` to shortcut 
directly. But later I found it would change the logic in 
`::mergeNRVOIntoParent`:
```
void Scope::mergeNRVOIntoParent() {
  if (VarDecl *Candidate = NRVO.getPointer()) {
if (isDeclScope(Candidate))
  Candidate->setNRVOVariable(true);
  }
  ...
```

It would set NRVO for the candidate in NRVO if it is in current scope. With the 
context of `addNRVOCandidate` here, I could judge that the change would be:
```
X test(bool B) {
  X x; // before: no nrvo, after: no nrvo (same)
  if (B)
return x;
  X y; // before: no nrvo, after: nrvo (better)
  return y; // Now NRVO.getInt()==true and NRVO.getPointer() == y;
}
```

Yeah, the behavior is still 'right'. `y` should be NRVO in this case. But the 
implementation smell bad, if `NRVO.getInt()` is true, we shouldn't do any 
thing. I am not sure if I state my points well. I mean the implementation might 
be correct, but it is hard to understand, read and maintain. It'd better to 
make the reader avoid doing mathmatics when reading the codes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119792

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

[PATCH] D121441: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

[Kai Luo 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 rG9247145fbae7: [PowerPC][NFC] Add atomic alignments and ops 
tests for powerpc (authored by lkail).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121441

Files:
  clang/test/CodeGen/PowerPC/atomic-alignment.c
  clang/test/Sema/atomic-ops.c


Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -4,6 +4,12 @@
 // RUN:   -fsyntax-only -triple=i686-linux-android -std=c11
 // RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
 // RUN:   -fsyntax-only -triple=powerpc64-linux-gnu -std=c11
+// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
+// RUN:   -fsyntax-only -triple=powerpc64-linux-gnu -std=c11 \
+// RUN:   -target-cpu pwr7
+// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
+// RUN:   -fsyntax-only -triple=powerpc64le-linux-gnu -std=c11 \
+// RUN:   -target-cpu pwr8
 
 // Basic parsing/Sema tests for __c11_atomic_*
 
Index: clang/test/CodeGen/PowerPC/atomic-alignment.c
===
--- /dev/null
+++ clang/test/CodeGen/PowerPC/atomic-alignment.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -verify -triple powerpc-unknown-unknown -emit-llvm -o - %s 
| \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC32
+// RUN: %clang_cc1 -verify -triple powerpc64le-unknown-linux -emit-llvm -o - 
%s | \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC64
+// RUN: %clang_cc1 -verify -triple powerpc64-unknown-aix -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC64
+
+// PPC: @c = global i8 0, align 1{{$}}
+_Atomic(char) c; // expected-no-diagnostics
+
+// PPC: @s = global i16 0, align 2{{$}}
+_Atomic(short) s; // expected-no-diagnostics
+
+// PPC: @i = global i32 0, align 4{{$}}
+_Atomic(int) i; // expected-no-diagnostics
+
+// PPC32: @l = global i32 0, align 4{{$}}
+// PPC64: @l = global i64 0, align 8{{$}}
+_Atomic(long) l; // expected-no-diagnostics
+
+// PPC: @ll = global i64 0, align 8{{$}}
+_Atomic(long long) ll; // expected-no-diagnostics
+
+typedef struct {
+  char x[8];
+} O;
+
+// PPC32: @o = global %struct.O zeroinitializer, align 1{{$}}
+// PPC64: @o = global %struct.O zeroinitializer, align 8{{$}}
+_Atomic(O) o; // expected-no-diagnostics
+
+typedef struct {
+  char x[16];
+} Q;
+
+// PPC: @q = global %struct.Q zeroinitializer, align 1{{$}}
+_Atomic(Q) q; // expected-no-diagnostics


Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -4,6 +4,12 @@
 // RUN:   -fsyntax-only -triple=i686-linux-android -std=c11
 // RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
 // RUN:   -fsyntax-only -triple=powerpc64-linux-gnu -std=c11
+// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
+// RUN:   -fsyntax-only -triple=powerpc64-linux-gnu -std=c11 \
+// RUN:   -target-cpu pwr7
+// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
+// RUN:   -fsyntax-only -triple=powerpc64le-linux-gnu -std=c11 \
+// RUN:   -target-cpu pwr8
 
 // Basic parsing/Sema tests for __c11_atomic_*
 
Index: clang/test/CodeGen/PowerPC/atomic-alignment.c
===
--- /dev/null
+++ clang/test/CodeGen/PowerPC/atomic-alignment.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -verify -triple powerpc-unknown-unknown -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC32
+// RUN: %clang_cc1 -verify -triple powerpc64le-unknown-linux -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC64
+// RUN: %clang_cc1 -verify -triple powerpc64-unknown-aix -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC64
+
+// PPC: @c = global i8 0, align 1{{$}}
+_Atomic(char) c; // expected-no-diagnostics
+
+// PPC: @s = global i16 0, align 2{{$}}
+_Atomic(short) s; // expected-no-diagnostics
+
+// PPC: @i = global i32 0, align 4{{$}}
+_Atomic(int) i; // expected-no-diagnostics
+
+// PPC32: @l = global i32 0, align 4{{$}}
+// PPC64: @l = global i64 0, align 8{{$}}
+_Atomic(long) l; // expected-no-diagnostics
+
+// PPC: @ll = global i64 0, align 8{{$}}
+_Atomic(long long) ll; // expected-no-diagnostics
+
+typedef struct {
+  char x[8];
+} O;
+
+// PPC32: @o = global %struct.O zeroinitializer, align 1{{$}}
+// PPC64: @o = global %struct.O zeroinitializer, align 8{{$}}
+_Atomic(O) o; // expected-no-diagnostics
+
+typedef struct {
+  char x[16];
+} Q;
+
+// PPC: @q = global %struct.Q zeroinitializer, align 1{{$}}
+_Atomic(Q) q; // expected-no-diagnostics
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang] 9247145 - [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

[Kai Luo via cfe-commits]

Author: Kai Luo
Date: 2022-03-18T13:22:28+08:00
New Revision: 9247145fbae7c4273acd6b8f3b331716ca80bf18

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

LOG: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

PowerPC is lacking tests checking `_Atomic` alignment in cfe. Adding these 
tests since we're going to make change to align with gcc on Linux.

Reviewed By: hubert.reinterpretcast, jsji

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

Added: 
clang/test/CodeGen/PowerPC/atomic-alignment.c

Modified: 
clang/test/Sema/atomic-ops.c

Removed: 




diff  --git a/clang/test/CodeGen/PowerPC/atomic-alignment.c 
b/clang/test/CodeGen/PowerPC/atomic-alignment.c
new file mode 100644
index 0..cd6985962c39e
--- /dev/null
+++ b/clang/test/CodeGen/PowerPC/atomic-alignment.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -verify -triple powerpc-unknown-unknown -emit-llvm -o - %s 
| \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC32
+// RUN: %clang_cc1 -verify -triple powerpc64le-unknown-linux -emit-llvm -o - 
%s | \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC64
+// RUN: %clang_cc1 -verify -triple powerpc64-unknown-aix -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefixes=PPC,PPC64
+
+// PPC: @c = global i8 0, align 1{{$}}
+_Atomic(char) c; // expected-no-diagnostics
+
+// PPC: @s = global i16 0, align 2{{$}}
+_Atomic(short) s; // expected-no-diagnostics
+
+// PPC: @i = global i32 0, align 4{{$}}
+_Atomic(int) i; // expected-no-diagnostics
+
+// PPC32: @l = global i32 0, align 4{{$}}
+// PPC64: @l = global i64 0, align 8{{$}}
+_Atomic(long) l; // expected-no-diagnostics
+
+// PPC: @ll = global i64 0, align 8{{$}}
+_Atomic(long long) ll; // expected-no-diagnostics
+
+typedef struct {
+  char x[8];
+} O;
+
+// PPC32: @o = global %struct.O zeroinitializer, align 1{{$}}
+// PPC64: @o = global %struct.O zeroinitializer, align 8{{$}}
+_Atomic(O) o; // expected-no-diagnostics
+
+typedef struct {
+  char x[16];
+} Q;
+
+// PPC: @q = global %struct.Q zeroinitializer, align 1{{$}}
+_Atomic(Q) q; // expected-no-diagnostics

diff  --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c
index 160a0c09903d9..a3c156d6663b9 100644
--- a/clang/test/Sema/atomic-ops.c
+++ b/clang/test/Sema/atomic-ops.c
@@ -4,6 +4,12 @@
 // RUN:   -fsyntax-only -triple=i686-linux-android -std=c11
 // RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
 // RUN:   -fsyntax-only -triple=powerpc64-linux-gnu -std=c11
+// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
+// RUN:   -fsyntax-only -triple=powerpc64-linux-gnu -std=c11 \
+// RUN:   -target-cpu pwr7
+// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \
+// RUN:   -fsyntax-only -triple=powerpc64le-linux-gnu -std=c11 \
+// RUN:   -target-cpu pwr8
 
 // Basic parsing/Sema tests for __c11_atomic_*
 



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

[PATCH] D121441: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

[Kai Luo via Phabricator via cfe-commits]

lkail added inline comments.



Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:1
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -verify -triple powerpc-unkonwn-unknown -emit-llvm -o - %s 
| \

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > I am not sure about having a `CodeGen` test for this when a `-fsyntax-only` 
> > `Layout` test would do. I believe that also removes the need for 
> > `powerpc-registered-target`.
> I will note that there is a concept of "preferred alignment" or "complete 
> object alignment" that may differ from "required alignment". A `CodeGen` test 
> like this can miss issues if the "required alignment" is wrong but the 
> "preferred alignment" or "complete object alignment" matches what is expected.
That's a good point. However currently, I haven't found any tests in 
`test/Sema` performing such check and I have't figure out a way to add such 
check. Maybe checking `__alignof__() % (expected_align) == 0` is more practical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121441

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

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu added a comment.

In D121445#3388842 , @rengolin wrote:

> I'm surprised these tests are passing for you. Perhaps you're not building or 
> running them all.
>
> To make sure you're running your tests, you need to build both clang and llvm 
> (`-DLLVM_ENABLE_PROJECTS=clang`) and run ninja/make `check-all`.
>
> You can also run `lit` directly on each test, but I can't remember how to do 
> that now...

I have enabled clang project building and run check-all test. Like ` 
-DLLVM_ENABLE_ASSERTIONS=On -DLLVM_ENABLE_PROJECTS="clang;llvm" 
-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="CSKY"`. So I am also surprised why those 
cases passed.


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

https://reviews.llvm.org/D121445

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

[PATCH] D121096: [C++20][Modules][HU 2/5] Support searching Header Units in user or system search paths.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2855-2857
+  assert((DashX.getHeaderUnit() == InputKind::HeaderUnit_None ||
+  Inputs.size() == 1) &&
+ "Expected only one input file for header unit");

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > ChuanqiXu wrote:
> > > > So the compiler would crash if there is multiple inputs for header 
> > > > unit? I feel it is not most friendly. How about emitting an error here?
> > > well the driver is supposed to catch these problems (in the case of C++20 
> > > modules mode it will generate several compile jobs, one per header).
> > > 
> > > I added an error for this tho.
> > Oh, so if the compiler wouldn't crash in that case, I think the original 
> > assertion is acceptable. And the current error is dead code, which is bad. 
> > Let's take the original assertion.
> actually, it does protect against the case that the frontend is called 
> directly with XClang or so, so it's not dead code, and it is better for the 
> compiler to emit a diagnostic rather than crash (if built with assertions).
> 
If it is not dead codes, I feel better if we could add tests to cover it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121096

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

[PATCH] D121097: [C++20][Modules][HU 3/5] Emit module macros for header units.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I feel good if we could add  the test from: http://eel.is/c++draft/cpp.import#8.




Comment at: clang/include/clang/Serialization/ASTWriter.h:21
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/SourceLocation.h"

This is redundant too.



Comment at: clang/lib/Serialization/ASTWriter.cpp:2358-2378
+bool EmittedModuleMacros = false;
+if (IsHeaderUnit) {
+  // This is for the main TU when it is a C++20 header unit.
+  // We preserve the final state of defined macros, and we do not emit ones
+  // that are undefined.
+  if (!MD || shouldIgnoreMacro(MD, IsModule, PP) ||
+  MD->getKind() == MacroDirective::MD_Undefine)

iains wrote:
> ChuanqiXu wrote:
> > Is it possible to merge the implementation with the following for PCH? It 
> > looks like there are some redundancies.
> Well, that was what I had originally, I actually split it out as it is now 
> because the difference in the logic around which macros are written out was 
> making the code pretty confusing to read.  If there's a strong feeling about 
> this, perhaps we can see if there's some way to factor it (perhaps with some 
> place-holder vars).
> 
I agree with you. I tried to refactor it myself and find things didn't get 
better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121097

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I've looked into the details as much as I can and I don't find explicit errors.

---

It is better to add tests in clang/unittest by using 
`VarDecl::isNRVOVariable()`.




Comment at: clang/include/clang/Sema/Scope.h:533-535
+if (NRVO.getPointer() != nullptr && NRVO.getPointer() != NewNRVO)
+  NRVO.setInt(true);
+NRVO.setPointer(NewNRVO);


The suggested changes smell better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119792

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

[PATCH] D121497: Lex: add support for `{,u}i128` Microsoft extension

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

I don't know how "intsafe.h" looks like and how it is involved with the modules 
(it's not in winsdk.modulemap as far as I can tell). My immediate guess would 
be that "intsafe.h" doesn't include all the headers that are supposed to 
provide necessary macros. And if "intsafe.h" is in a separate module, for the 
code

  #define SOME_MACRO 1
  #include "intsafe.h"

"intsafe.h" won't see `SOME_MACRO`. That's how it can end up with different 
macro expansions. But that is just a guess (that can be waay wrong) and you 
could have checked this scenario already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121497

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

[PATCH] D121425: [VFS] Add a new getVFSFromYAMLs API

[Ben Barham via Phabricator via cfe-commits]

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

Blocked on the dependent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121425

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

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai accepted this revision.
vsapsai added a comment.

Thanks for your patience and for all the explanations, I have no other comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

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

[PATCH] D121749: [clang-format][docs] Regenerate ClangFormatStyleOptions.rst

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:361
 
-  * ``bool AlignCompound`` Only for ``AlignConsecutiveAssignments``.  Whether 
compound
-assignments like ``+=`` are aligned along with ``=``.
+  * ``bool AlignCompound`` Only for ``AlignConsecutiveAssignments``.  Whether 
compound assignments
+like ``+=`` are aligned along with ``=``.

MyDeveloperDay wrote:
> owenpan wrote:
> > curdeius wrote:
> > > curdeius wrote:
> > > > sstwcw wrote:
> > > > > MyDeveloperDay wrote:
> > > > > > Is this wider than 80 chars?
> > > > > The comment in the source code is within 80 columns.  The python 
> > > > > script prepended the option name.  It's the same case as line 1541.
> > > > It is indeed. But it was already longer than 80 chars.
> > > > I'll have a look at the script to see how to fix this.
> > > At the same time, does it really matter if it's auto-generated?
> > It should not matter, especially because it’s also a non-source file.
> I seem to remember once before I made a change to clang-tidy and they liked 
> the rst to be no wider than 80 chars, but that might have been hand generated 
> rst.  If I remember rightly @Eugene.Zelenko picked me up on it, So I actually 
> developed a script (which never landed) that checks the rst for me.  {D55523}
> 
> I still use this from time to time and try and clean up.
> 
> ```
> Checking docs/ClangFormatStyleOptions.rst...
> warning: line 9 title and markup non matching lengths
> warning: line 21 multiple blank lines
> warning: line 35 maximize 80 characters by joining:'[When using 
> ``-style=file:``, :program:`clang-format` for]' and 
> '[each...]
> 
> warning: line 36 maximize 80 characters by joining:'[each input file will use 
> the format file located at ``.]' and '[The...]
> 
> warning: line 103 multiple blank lines
> warning: line 121 multiple blank lines
> warning: line 130 multiple blank lines
> warning: line 139 multiple blank lines
> warning: line 181 maximize 80 characters by 
> joining:'[**AccessModifierOffset** (``Integer``) :versionbadge:`clang-format 
> 3.3`]' and '[The...]
> 
> warning: line 184 is in excess of 80 characters (87) : 
> **AlignAfterOpenBracket** (``BracketAlignmentStyle``) 
> :versionbadge:`clang-format 3.8`
> ...
> warning: line 228 multiple blank lines
> warning: line 229 trailing whitespace
> warning: line 233 multiple blank lines
> warning: line 234 multiple blank lines
> warning: line 235 is in excess of 80 characters (96) : 
> **AlignArrayOfStructures** (``ArrayInitializerAlignmentStyle``) 
> :versionbadge:`clang-format 13`
> ...
> warning: line 236 maximize 80 characters by joining:'[  if not ``None``, when 
> using initialization for an array of structs]' and '[aligns...]
> 
> warning: line 251 contains double spaces
> warning: line 253 contains double spaces
> warning: line 263 contains double spaces
> warning: line 265 contains double spaces
> warning: line 271 multiple blank lines
> warning: line 272 multiple blank lines
> warning: line 273 is in excess of 80 characters (93) : 
> **AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) 
> :versionbadge:`clang-format 3.8`
> ...
> warning: line 280 contains double spaces
> warning: line 282 contains double spaces
> warning: line 295 maximize 80 characters by joining:'[  For example, to align 
> across empty lines and not across comments, either]' and '[of...]
> 
> warning: line 311 contains double spaces
> warning: line 312 contains double spaces
> warning: line 314 contains double spaces
> warning: line 315 contains double spaces
> warning: line 317 contains double spaces
> warning: line 319 contains double spaces
> warning: line 322 contains double spaces
> warning: line 323 contains double spaces
> warning: line 325 contains double spaces
> warning: line 326 contains double spaces
> warning: line 334 contains double spaces
> warning: line 336 contains double spaces
> warning: line 338 contains double spaces
> warning: line 341 contains double spaces
> warning: line 343 contains double spaces
> warning: line 352 contains double spaces
> warning: line 361 contains double spaces
> warning: line 367 contains double spaces
> warning: line 368 contains double spaces
> warning: line 374 contains double spaces
> warning: line 381 contains double spaces
> warning: line 382 contains double spaces
> warning: line 384 contains double spaces
> warning: line 391 contains double spaces
> warning: line 394 multiple blank lines
> warning: line 395 is in excess of 80 characters (90) : 
> **AlignConsecutiveBitFields** (``AlignConsecutiveStyle``) 
> :versionbadge:`clang-format 11`
> ...
> warning: line 398 maximize 80 characters by joining:'[  ``Consecutive`` will 
> align the bitfield separators of consecutive lines.]' and '[This...]
> 
> warning: line 404 contains double spaces
> warning: line 405 contains double spaces
> warning: line 418 maximize 80 characters by joining:'[  For example, to align 
> across empty 

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D121588#3390956 , @vsapsai wrote:

> Thanks for explaining the desired interplay between PCH and PCM. The problem 
> I'm seeing is that it can be hard to find correct compiler flags to achieve 
> the desired result (what is enabled and what is disabled). But that is out of 
> scope for this change.

IMHO, that is an unfortunate consequence of having ≈ 60+ module-related command 
line flags; we do need to rationalise this.  This is being discussed elsewhere 
[D120540 ] (and probably it's better to leave 
that conversation in one place).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

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

[PATCH] D121873: [clang][extract-api] Add enum support

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Hey, apologies for missing the initial patch in D119479 
.

This sounds really interesting!
The lack of docs make it very difficult to understand what this library is for 
and how to use it if you don't already know.
It's not clear what the SymbolGraph library is (I think there's a doc missing 
from the directory). The patches are tagged with [extract-api] so it must be 
related, but if it's something distinct it should be documented and reusable, 
and if it's the same why is it not called ExtractAPI :-)

The main entry point seems to be API.h, which has only a trivial file comment 
and no comments on what the meaning of the structs and fields are. Especially 
if this is part of clang proper, it's going to be read and maintained and 
debugged by a lot of people who have never heard of SymbolGraph and don't know 
swift.

It's also not clear from looking at this why this is part of clang itself and 
not a libTooling-style external tool - it seems similar in scope and 
applicability to a lot of the things in clang-tools-extra.

I know a lot of these things don't feel like they need much explanation when 
you're and your reviewer are familiar with it, so apologies that this is 
tedious, but the cost of undocumented code like this in shared projects is high.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121873

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

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/test/Driver/cxx20-header-units-01.cpp:7
+
+// RUN: %clang++ -### -std=c++20 -xc++-header-unit-header 
%S/Inputs/header-unit-01.hh 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ABS %s -DTDIR=%S/Inputs

vsapsai wrote:
> iains wrote:
> > vsapsai wrote:
> > > What should happen in case of inconsistencies like `%clang++ -### 
> > > -std=c++20 -xc++-system-header %S/Inputs/header-unit-01.hh`? Or 
> > > `-xc++-system-header foo.h`?
> > If the user states that `%S/Inputs/header-unit-01.hh` is a system header, I 
> > do not think that the driver is in a position to argue?
> > 
> > `  -xc++-system-header foo.h `  again the user has made an explicit 
> > statement..
> > 
> > I suppose that we can always diagnose these things with warnings - but I do 
> > not think we can easily reject them (and we risk producing unhelpful 
> > output).
> What is the intended use of `-xc++-user-header` and `-xc++-system-header`? 
> Are these supposed to be commonly-used flags or as an escape hatch of some 
> sort? Because if it is commonly-used, I have concerns about usability that 
> forces users to track user and system headers correctly, and duplicates the 
> information already encoded in header search paths.
> 
> Also I think it is out of scope but for usability it is important to know the 
> consequences of using a wrong flag and how one can diagnose it. Have no idea 
> what you've already planned in this area, just mentioning it because I'm 
> scarred by inscrutable clang behavior.
These are "generally internal" marking to be specific about how files should be 
treated.

The usual user-facing commands would be `-fmodule-header={user, system}`

So the answer is no, they would not be commonly used - pretty much the same as 
`-x c++` or so, only necessary if the file type is ambiguous to the driver in 
some way.

If the user wants a specific header to be searched for in the user or system 
paths, then it is reasonable that they have to indicate this.  Actually, 
probably the largest benefit is that the user would not need to know where 
system headers were stored -- so when trying to build a header unit for, say, 
'vector' it would greatly simplify the process,

I hope that the behaviour should not be in any way inscrutable - there are some 
things that the user (or build system) has to communicate to the driver, it 
cannot deduce them (and where to search for headers is in that category).



the consequences of using the wrong flag in this case would be a failure to 
find the header file (the diagnostic would be emitted by the FE rather than the 
driver, but that would be transparent to a regular user).   The logic here is 
that using the lookup paths is not a job for the driver, because the FE already 
knows and has mechanisms for it.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

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

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 416364.
iains added a comment.

adjust testcase path separators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

Files:
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/FrontendOptions.cpp
  clang/test/Driver/Inputs/header-unit-01.hh
  clang/test/Driver/cxx20-header-units-01.cpp

Index: clang/test/Driver/cxx20-header-units-01.cpp
===
--- /dev/null
+++ clang/test/Driver/cxx20-header-units-01.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -std=c++20 -xc++-user-header foo.h 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-USER %s
+
+// RUN: %clang -### -std=c++20 -xc++-system-header vector 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-SYSTEM %s
+
+// RUN: %clang -### -std=c++20 \
+// RUN:   -xc++-header-unit-header %/S/Inputs/header-unit-01.hh 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ABS %s -DTDIR=%/S/Inputs
+
+// CHECK-USER: "-emit-header-unit"
+// CHECK-USER-SAME: "-o" "foo.pcm"
+// CHECK-USER-SAME: "-x" "c++-user-header" "foo.h"
+// CHECK-SYSTEM: "-emit-header-unit"
+// CHECK-SYSTEM-SAME: "-o" "vector.pcm"
+// CHECK-SYSTEM-SAME: "-x" "c++-system-header" "vector"
+// CHECK-ABS: "-emit-header-unit"
+// CHECK-ABS-SAME: "-o" "header-unit-01.pcm"
+// CHECK-ABS-SAME: "-x" "c++-header-unit-header" "[[TDIR]]/header-unit-01.hh"
Index: clang/lib/Frontend/FrontendOptions.cpp
===
--- clang/lib/Frontend/FrontendOptions.cpp
+++ clang/lib/Frontend/FrontendOptions.cpp
@@ -27,7 +27,7 @@
   .Cases("C", "cc", "cp", Language::CXX)
   .Cases("cpp", "CPP", "c++", "cxx", "hpp", "hxx", Language::CXX)
   .Case("cppm", Language::CXX)
-  .Case("iim", InputKind(Language::CXX).getPreprocessed())
+  .Cases("iim", "iih", InputKind(Language::CXX).getPreprocessed())
   .Case("cl", Language::OpenCL)
   .Case("clcpp", Language::OpenCLCXX)
   .Cases("cu", "cuh", Language::CUDA)
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -65,9 +65,16 @@
   return Id == TY_CXXModule || Id == TY_PP_CXXModule;
 }
 
+static bool isPreprocessedHeaderUnitType(ID Id) {
+  return Id == TY_CXXSHeader || Id == TY_CXXUHeader || Id == TY_CXXHUHeader ||
+ Id == TY_PP_CXXHeaderUnit;
+}
+
 types::ID types::getPrecompiledType(ID Id) {
   if (isPreprocessedModuleType(Id))
 return TY_ModuleFile;
+  if (isPreprocessedHeaderUnitType(Id))
+return TY_HeaderUnit;
   if (onlyPrecompileType(Id))
 return TY_PCH;
   return TY_INVALID;
@@ -139,6 +146,10 @@
   case TY_CLHeader:
   case TY_ObjCHeader: case TY_PP_ObjCHeader:
   case TY_CXXHeader: case TY_PP_CXXHeader:
+  case TY_CXXSHeader:
+  case TY_CXXUHeader:
+  case TY_CXXHUHeader:
+  case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
   case TY_CXXModule: case TY_PP_CXXModule:
   case TY_AST: case TY_ModuleFile: case TY_PCH:
@@ -210,6 +221,10 @@
   case TY_CXX: case TY_PP_CXX:
   case TY_ObjCXX: case TY_PP_ObjCXX: case TY_PP_ObjCXX_Alias:
   case TY_CXXHeader: case TY_PP_CXXHeader:
+  case TY_CXXSHeader:
+  case TY_CXXUHeader:
+  case TY_CXXHUHeader:
+  case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
   case TY_CXXModule: case TY_PP_CXXModule:
   case TY_CUDA: case TY_PP_CUDA: case TY_CUDA_DEVICE:
@@ -323,6 +338,7 @@
.Case("hpp", TY_CXXHeader)
.Case("hxx", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
+   .Case("iih", TY_PP_CXXHeaderUnit)
.Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)
.Case("obj", TY_Object)
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4607,6 +4607,8 @@
   CmdArgs.push_back(IsHeaderModulePrecompile
 ? "-emit-header-module"
 : "-emit-module-interface");
+else if (JA.getType() == types::TY_HeaderUnit)
+  CmdArgs.push_back("-emit-header-unit");
 else
   CmdArgs.push_back("-emit-pch");
   } else if (isa(JA)) {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2209,6 +2209,11 @@
   if (Value == "-")
 return true;
 
+  // If it's a header to be found in the system or user search path, then defer
+  // complaints about its absence until those searches can be done.
+  if (Ty == types::TY_CXXSHeader || Ty == types::TY_CXXUHeader)
+return true;
+
   if (getVFS().exists(Value))
 return true;
 

[PATCH] D121748: [clang][Sema] Better support for ObjC++ in Sema::LookupName

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

I see, thanks! Let me think a bit more about this change and test it on our 
codebase to see if this is a viable Sema change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121748

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

[clang] 5d2ce76 - Use llvm::append_range instead of push_back loops where applicable. NFCI.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-03-18T01:25:34+01:00
New Revision: 5d2ce7663b10c107328a4ae0c678165209e64619

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

LOG: Use llvm::append_range instead of push_back loops where applicable. NFCI.

Added: 


Modified: 
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/include/clang/ASTMatchers/ASTMatchersInternal.h
clang/include/clang/Analysis/CloneDetection.h
clang/include/clang/Tooling/DiagnosticsYaml.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/VTableBuilder.cpp
clang/lib/Analysis/CFG.cpp
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGNonTrivialStruct.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Hexagon.cpp
clang/lib/Sema/SemaCUDA.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/SemaType.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
llvm/include/llvm/ExecutionEngine/JITLink/MemoryFlags.h

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 6664a5bcfe7fb..c16bc5f0eafde 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7928,8 +7928,7 @@ AST_MATCHER_P(Stmt, forFunction, 
internal::Matcher,
 return true;
   }
 } else {
-  for (const auto  : Finder->getASTContext().getParents(CurNode))
-Stack.push_back(Parent);
+  llvm::append_range(Stack, Finder->getASTContext().getParents(CurNode));
 }
   }
   return false;
@@ -7987,8 +7986,7 @@ AST_MATCHER_P(Stmt, forCallable, internal::Matcher, 
InnerMatcher) {
 return true;
   }
 } else {
-  for (const auto  : Finder->getASTContext().getParents(CurNode))
-Stack.push_back(Parent);
+  llvm::append_range(Stack, Finder->getASTContext().getParents(CurNode));
 }
   }
   return false;

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h 
b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index 5648c716c539e..49de9a458c3db 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -132,10 +132,7 @@ struct VariadicFunction {
   // We also allow calls with an already created array, in case the caller
   // already had it.
   ResultT operator()(ArrayRef Args) const {
-SmallVector InnerArgs;
-for (const ArgT  : Args)
-  InnerArgs.push_back();
-return Func(InnerArgs);
+return Func(llvm::to_vector<8>(llvm::make_pointer_range(Args)));
   }
 
 private:

diff  --git a/clang/include/clang/Analysis/CloneDetection.h 
b/clang/include/clang/Analysis/CloneDetection.h
index b2911a5b44eb9..ffd496c5c9f65 100644
--- a/clang/include/clang/Analysis/CloneDetection.h
+++ b/clang/include/clang/Analysis/CloneDetection.h
@@ -208,13 +208,7 @@ class CloneDetector {
 // The initial assumption is that there is only one clone group and every
 // statement is a clone of the others. This clone group will then be
 // split up with the help of the constraints.
-CloneGroup AllClones;
-AllClones.reserve(Sequences.size());
-for (const auto  : Sequences) {
-  AllClones.push_back(C);
-}
-
-Result.push_back(AllClones);
+Result.push_back(Sequences);
 
 constrainClones(Result, ConstraintList...);
   }

diff  --git a/clang/include/clang/Tooling/DiagnosticsYaml.h 
b/clang/include/clang/Tooling/DiagnosticsYaml.h
index 3f257d84f8136..88f81e1f62999 100644
--- a/clang/include/clang/Tooling/DiagnosticsYaml.h
+++ b/clang/include/clang/Tooling/DiagnosticsYaml.h
@@ -42,8 +42,7 @@ template <> struct 
MappingTraits {
 Io.mapOptional("FileOffset", M.FileOffset);
 std::vector Fixes;
 for (auto  : M.Fix) {
-  for (auto  : Replacements.second)
-Fixes.push_back(Replacement);
+  llvm::append_range(Fixes, Replacements.second);
 }
 Io.mapRequired("Replacements", Fixes);
 for (auto  : Fixes) {

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 77b21746c83fc..3ba9f40a52a36 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -739,8 +739,8 @@ canonicalizeImmediatelyDeclaredConstraint(const ASTContext 
, Expr *IDC,
 // template concept C = true;
 // template T> 

[PATCH] D121969: Pass split-machine-functions to code generator when flto is used

[Junfeng Dong via Phabricator via cfe-commits]

junfd updated this revision to Diff 416363.
junfd added a comment.

Follow clang format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121969

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/split-machine-functions.c


Index: clang/test/Driver/split-machine-functions.c
===
--- /dev/null
+++ clang/test/Driver/split-machine-functions.c
@@ -0,0 +1,8 @@
+// Test options pass-through with lto
+// RUN: %clang -### -flto -fsplit-machine-functions %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-PASS
+
+// Test no pass-through to ld without lto
+// RUN: %clang -### -fsplit-machine-functions %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-NOPASS
+
+// CHECK-PASS:  "-plugin-opt=-split-machine-functions"
+// CHECK-NOPASS-NOT:"-plugin-opt=-split-machine-functions"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -574,6 +574,10 @@
 CmdArgs.push_back("-plugin-opt=-data-sections");
   }
 
+  if (Args.hasArg(options::OPT_fsplit_machine_functions)) {
+CmdArgs.push_back("-plugin-opt=-split-machine-functions");
+  }
+
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
 StringRef FName = A->getValue();
 if (!llvm::sys::fs::exists(FName))


Index: clang/test/Driver/split-machine-functions.c
===
--- /dev/null
+++ clang/test/Driver/split-machine-functions.c
@@ -0,0 +1,8 @@
+// Test options pass-through with lto
+// RUN: %clang -### -flto -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+
+// Test no pass-through to ld without lto
+// RUN: %clang -### -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
+
+// CHECK-PASS:  "-plugin-opt=-split-machine-functions"
+// CHECK-NOPASS-NOT:"-plugin-opt=-split-machine-functions"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -574,6 +574,10 @@
 CmdArgs.push_back("-plugin-opt=-data-sections");
   }
 
+  if (Args.hasArg(options::OPT_fsplit_machine_functions)) {
+CmdArgs.push_back("-plugin-opt=-split-machine-functions");
+  }
+
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
 StringRef FName = A->getValue();
 if (!llvm::sys::fs::exists(FName))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D121936#3391001 , @zixuw wrote:

> In D121936#3390920 , @dang wrote:
>
>> In D121936#3389991 , @ributzka 
>> wrote:
>>
>>> Nice! Thank you for adding support for multiple headers. Is this a step 
>>> towards the json input file list?
>>
>> My understanding was that for this iteration at least we were going with 
>> just feeding the headers directly to clang without an input file list. Maybe 
>> we should talk about this offline as I might be missing something.
>
> In my initial proposed design, clang-extract-api takes in multiple headers 
> directly as inputs. What would be the need to have a json file list?

It might be better if we need to pass in information about specific headers. We 
can discuss more offline but I don't think it is necessary for this iteration 
at least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

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

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

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

These pass for me locally, so I'm reverting for now and will dig into this 
tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests: http://45.33.8.238/linux/71396/step_7.txt

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

In D121936#3390920 , @dang wrote:

> In D121936#3389991 , @ributzka 
> wrote:
>
>> Nice! Thank you for adding support for multiple headers. Is this a step 
>> towards the json input file list?
>
> My understanding was that for this iteration at least we were going with just 
> feeding the headers directly to clang without an input file list. Maybe we 
> should talk about this offline as I might be missing something.

In my initial proposed design, clang-extract-api takes in multiple headers 
directly as inputs. What would be the need to have a json file list?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

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

[PATCH] D121873: [clang][extract-api] Add enum support

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/SymbolGraph/Serialization.cpp:321
+  case RelationshipKind::MemberOf:
+Relationship["kind"] = "memberOf";
+break;

dang wrote:
> Since we are going to add more cases to this switch, would you mind doing 
> something along the lines of:
> ```
> const char *KindString = "";
> switch(Kind) {
>   default:
>  llvm_unreachable("Unhandled relationship kind in Symbol Graph!");
>  break;
>   case RelationshipKind::MemberOf: 
> KindString = "memberOf";
> break;
> }
> 
> Relationship["kind"] = KindString
> ```
> or a method in the serializer for getting the string representation of the 
> relationship kind?
Definitely! That's a good idea. I'm going to have a 'getString' sort of method 
in the serializer to do this.



Comment at: clang/lib/SymbolGraph/Serialization.cpp:343
+  if (!Enum)
+return;
+

dang wrote:
> Quick design question: Do we want to be silently failing in these situations 
> (especially since this shouldn't be happening)? Let me know what you think.
I'm using this check to intentionally skip symbols that we do not want to spit 
out, for example unconditionally unavailable symbols, or after we have support 
for typedef records, anonymous enum decls that's declared with a `typedef` so 
that we don't have duplicate information, etc.
`Optional serializeAPIRecord` does this check now, and if we 
`Serializer::shouldSkip` it, `None` will be returned. So it is expected, not 
really silently failing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121873

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

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

[Paul Kirth via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7749d4713a5: [misexpect] Re-implement MisExpect Diagnostics 
(authored by paulkirth).

Changed prior to commit:
  https://reviews.llvm.org/D115907?vs=416356=416360#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

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

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for explaining the desired interplay between PCH and PCM. The problem 
I'm seeing is that it can be hard to find correct compiler flags to achieve the 
desired result (what is enabled and what is disabled). But that is out of scope 
for this change.




Comment at: clang/include/clang/Driver/Types.def:66
 TYPE("c++-header",   CXXHeader,PP_CXXHeader,"hh", 
phases::Preprocess, phases::Precompile)
-TYPE("objective-c++-header-cpp-output", PP_ObjCXXHeader, INVALID, "mii",  
phases::Precompile)
+TYPE("c++-header-unit-cpp-output", PP_CXXHeaderUnit,INVALID,"iih",
phases::Precompile)
+TYPE("c++-header-unit-header",   CXXHUHeader,  PP_CXXHeaderUnit,"hh", 
phases::Preprocess, phases::Precompile)

iains wrote:
> vsapsai wrote:
> > Sorry, it's not really related to your change but do you have a rule where 
> > "ii" should go? It's just we have both "mii" and "iim" and I want to make 
> > sure it should be "iih" and not "hii". I haven't tried to find a pattern 
> > here myself, asking you first.
> > Sorry, it's not really related to your change but do you have a rule where 
> > "ii" should go? It's just we have both "mii" and "iim" and I want to make 
> > sure it should be "iih" and not "hii". I haven't tried to find a pattern 
> > here myself, asking you first.
> 
> My understanding is this:
> 
> ObjectiveC/C++ append "I" and "ii" (mi and mii)
> 
> C++ Modules-related code pre-pends "ii".
> 
> so that a pre-processed header unit ==> "iih"
> and a pre-processed module ==> "iim".
> 
> 
Thanks for the explanation. Now I see it should be "iih" and not "hii". One 
could argue that modular-related stuff can be expressed by appending "m", like 
".ii" -> ".iim". But I don't know what that would mean for PP_CXXHeaderUnit 
("iihm"?) and I think "iih" is reasonable.



Comment at: clang/test/Driver/cxx20-header-units-01.cpp:7
+
+// RUN: %clang++ -### -std=c++20 -xc++-header-unit-header 
%S/Inputs/header-unit-01.hh 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ABS %s -DTDIR=%S/Inputs

iains wrote:
> vsapsai wrote:
> > What should happen in case of inconsistencies like `%clang++ -### 
> > -std=c++20 -xc++-system-header %S/Inputs/header-unit-01.hh`? Or 
> > `-xc++-system-header foo.h`?
> If the user states that `%S/Inputs/header-unit-01.hh` is a system header, I 
> do not think that the driver is in a position to argue?
> 
> `  -xc++-system-header foo.h `  again the user has made an explicit 
> statement..
> 
> I suppose that we can always diagnose these things with warnings - but I do 
> not think we can easily reject them (and we risk producing unhelpful output).
What is the intended use of `-xc++-user-header` and `-xc++-system-header`? Are 
these supposed to be commonly-used flags or as an escape hatch of some sort? 
Because if it is commonly-used, I have concerns about usability that forces 
users to track user and system headers correctly, and duplicates the 
information already encoded in header search paths.

Also I think it is out of scope but for usability it is important to know the 
consequences of using a wrong flag and how one can diagnose it. Have no idea 
what you've already planned in this area, just mentioning it because I'm 
scarred by inscrutable clang behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 416359.
dang added a comment.

Add test to check that command line with different header kinds gets diagnosed
appropriately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Action.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/extract-api-multiheader-kind-diag.h
  clang/test/Driver/extract-api-multiheader.h
  clang/test/Driver/extract-api.c
  clang/test/Driver/extract-api.h
  clang/test/SymbolGraph/global_record.c

Index: clang/test/SymbolGraph/global_record.c
===
--- clang/test/SymbolGraph/global_record.c
+++ clang/test/SymbolGraph/global_record.c
@@ -3,7 +3,7 @@
 // RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
 // RUN: %t/reference.output.json
 // RUN: %clang -extract-api -target arm64-apple-macosx \
-// RUN: %t/input.c -o %t/output.json | FileCheck -allow-empty %s
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
@@ -13,7 +13,7 @@
 // CHECK-NOT: error:
 // CHECK-NOT: warning:
 
-//--- input.c
+//--- input.h
 int num;
 
 /**
@@ -80,7 +80,7 @@
   "location": {
 "character": 5,
 "line": 1,
-"uri": "file://INPUT_DIR/input.c"
+"uri": "file://INPUT_DIR/input.h"
   },
   "names": {
 "subHeading": [
@@ -272,7 +272,7 @@
   "location": {
 "character": 6,
 "line": 9,
-"uri": "file://INPUT_DIR/input.c"
+"uri": "file://INPUT_DIR/input.h"
   },
   "names": {
 "subHeading": [
Index: clang/test/Driver/extract-api.h
===
--- clang/test/Driver/extract-api.h
+++ clang/test/Driver/extract-api.h
@@ -3,8 +3,8 @@
 // RUN: FileCheck -check-prefix EXTRACT-API-PHASES -input-file %t %s
 
 // EXTRACT-API-PHASES: 0: input,
-// EXTRACT-API-PHASES: , c
-// EXTRACT-API-PHASES: 1: preprocessor, {0}, cpp-output
-// EXTRACT-API-PHASES: 2: compiler, {1}, api-information
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 1: preprocessor, {0}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 2: api-extractor, {1}, api-information
 // EXTRACT-API-PHASES-NOT: 3:
 // EXTRACT-API-PHASES: END
Index: clang/test/Driver/extract-api-multiheader.h
===
--- /dev/null
+++ clang/test/Driver/extract-api-multiheader.h
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang -target x86_64-unknown-unknown -ccc-print-phases -extract-api %t/first-header.h %t/second-header.h 2> %t1
+// RUN: echo 'END' >> %t1
+// RUN: FileCheck -check-prefix EXTRACT-API-PHASES -input-file %t1 %s
+
+// EXTRACT-API-PHASES: 0: input
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 1: preprocessor, {0}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 2: input
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 3: preprocessor, {2}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 4: api-extractor, {1, 3}, api-information
+// EXTRACT-API-PHASES-NOT: 5:
+// EXTRACT-API-PHASES: END
+
+//--- first-header.h
+
+void dummy_function(void);
+
+//--- second-header.h
+
+void other_dummy_function(void);
Index: clang/test/Driver/extract-api-multiheader-kind-diag.h
===
--- /dev/null
+++ clang/test/Driver/extract-api-multiheader-kind-diag.h
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: not %clang -target x86_64-unknown-unknown -extract-api %t/first-header.h -x objective-c-header %t/second-header.h 2>&1 | FileCheck %s
+
+// CHECK: error: header file
+// CHECK-SAME: input 'objective-c-header' does not match the type of prior input in api extraction; use '-x c-header' to override
+
+//--- first-header.h
+
+void dummy_function(void);
+
+//--- second-header.h
+
+void other_dummy_function(void);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -28,6 +28,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
+#include "clang/Driver/Action.h"
 #include "clang/Driver/Distro.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/InputInfo.h"
@@ -4387,8 +4388,9 @@
   // CUDA/HIP compilation may have multiple inputs (source file + results of
   // device-side compilations). OpenMP device jobs also 

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

[Paul Kirth via Phabricator via cfe-commits]

paulkirth marked 5 inline comments as done.
paulkirth added inline comments.



Comment at: llvm/docs/MisExpect.rst:54
+
+.. option:: -pgo-warn-misexpect
+

tejohnson wrote:
> paulkirth wrote:
> > tejohnson wrote:
> > > Looking at the code, the -pgo-warn-misexpect seems to be useful for 
> > > internal LLVM testing via 'opt', to mimic -Wmisexpect, so it probably 
> > > should be documented as such.
> > Yes, its useful to test llvm, but shouldn't `opt` be usable in the same 
> > way? To me it seems useful for opt to support the warning directly too, but 
> > I'm happy to defer here if you think that's confusing or shouldn't be the 
> > case.
> Oh I'm not disagreeing with having the internal option and using it for opt, 
> that's very useful. My comment was in the context of not having the 
> user-facing clang documentation with the clang driver level option (which you 
> since added). I was just suggesting you add a note that this option is an 
> alternative to the clang option for use when e.g. testing via opt. Since 
> there is now separate clang documentation I think it is less important now.
Thanks for the clarification.



Comment at: llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll:3
+
+; RUN: opt < %s -lower-expect -pgo-instr-use 
-pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 
-pass-remarks=misexpect 2>&1 | FileCheck %s
+

tejohnson wrote:
> tejohnson wrote:
> > The old PM is gone in the optimization pipeline, so these old PM lines in 
> > this and other tests and the comment about the new PM can go away.
> I see you removed the NewPM-style invocations and kept the legacy ones - 
> afaict opt will now convert the legacy style pass invocations to the new PM, 
> but I think it is preferable to use the New PM style invocations.
yes, I misread your earlier comment.  I have restored the New PM style opt 
invocations, and removed the legacy PM ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

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

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 416356.
paulkirth marked 4 inline comments as done.
paulkirth added a comment.

Fixup remaining outstanding issues

- fix typos
- add misexpect to clang release notes
- restore new pm style opt invocations in llvm tests
- move single header function into implementation file
- clarify comment in test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D121936#3389991 , @ributzka wrote:

> Nice! Thank you for adding support for multiple headers. Is this a step 
> towards the json input file list?

My understanding was that for this iteration at least we were going with just 
feeding the headers directly to clang without an input file list. Maybe we 
should talk about this offline as I might be missing something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/Driver/Phases.h:17
   /// compilation process which interact with user options.
-  enum ID {
-Preprocess,

ributzka wrote:
> Is this an unrelated formatting change?
Fixed it in the latest diff. I had modified this in my first attempt at getting 
this to work so git-clang-format got confused...



Comment at: clang/lib/Driver/Phases.cpp:19
   case Precompile: return "precompiler";
-  case Compile: return "compiler";
   case Backend: return "backend";

ributzka wrote:
> Unrelated format change?
Same as the other change...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

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

[PATCH] D121556: [randstruct] Add randomize structure layout support

[Bill Wendling via Phabricator via cfe-commits]

void marked 7 inline comments as done.
void added inline comments.



Comment at: clang/include/clang/AST/Decl.h:4067
+
+  void setIsRandomized(bool V) const { RecordDeclBits.IsRandomized = V; }
+

aaron.ballman wrote:
> A setter that is marked `const` does not spark joy for me... Can we use 
> friendship rather than mutability to solve this?
It was done to prevent a `const_cast`. It looks like the other setters are 
non-const. I'll see if I can get around that.



Comment at: clang/include/clang/AST/Randstruct.h:34
+
+extern std::string SEED;
+

aaron.ballman wrote:
> Doing this with a global variable is unfortunate; it could make things harder 
> when we multithread the frontend. Can we solve this without a global? (And if 
> not, does this global need to be a `ManagedStatic`?)
Maybe this could be moved to `LangOpts`?



Comment at: clang/include/clang/Basic/Attr.td:3938-3939
+def RandomizeLayout : InheritableAttr {
+  let Spellings = [GCC<"randomize_layout">, Declspec<"randomize_layout">,
+Keyword<"randomize_layout">];
+  let Subjects = SubjectList<[Record]>;

aaron.ballman wrote:
> Microsoft does not implement this `__declspec`, correct? If they don't, we 
> shouldn't either (even if GCC does) unless there's a *very* good reason to do 
> so. That's Microsoft's design space we're encroaching on.
> 
> I'd also like to understand the justification for adding a new keyword for 
> this.
Good point. I'll remove those.



Comment at: clang/lib/AST/DeclBase.cpp:28
 #include "clang/AST/ExternalASTSource.h"
+#include "clang/AST/Randstruct.h"
 #include "clang/AST/Stmt.h"

aaron.ballman wrote:
> Is this include necessary?
Hmm...that must have been from a previous change. Removed.



Comment at: clang/lib/AST/Randstruct.cpp:72
+  // All of the Buckets produced by best-effort cache-line algorithm.
+  // TODO: Figure out a better initial size.
+  SmallVector, 16> Buckets;

aaron.ballman wrote:
> This size seems as defensible as most others; do you plan to do more here, or 
> should this comment be removed?
It's probably not worth looking into unless it becomes an issue. I'll remove it.



Comment at: clang/lib/AST/Randstruct.cpp:99
+
+if (FD->isBitField() && !FD->isZeroLengthBitField(Context)) {
+  // Start a bitfield run if this is the first bitfield we have found.

aaron.ballman wrote:
> Oh cool, we do bit-field runs!
> 
> How should we handle anonymous bit-fields (if anything special should be done 
> for them at all)? People usually use them for padding between bit-fields, so 
> it's not clear to me how to treat them when randomizing.
Good question! I'm not sure either. On one hand, I'm rather concerned about 
changing bitfield order in general, but it appears to be something that GCC's 
plugin does, so...




Comment at: clang/lib/AST/Randstruct.cpp:179
+
+  SmallVector RandomizedFields;
+

aaron.ballman wrote:
> This one seems a bit large to me; any reason not to use `16` again?
The code above looks to apply mostly to bitfield runs. This is for all fields 
in a structure. I assumed (without proof) that this will always be larger than 
the bitfield runs. :-)



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2063-2069
+  if (RD->isRandomized()) {
+for (Decl *D : FinalOrdering)
+  RD->removeDecl(D);
+
+for (Decl *D : FinalOrdering)
+  RD->addDecl(D);
+  }

aaron.ballman wrote:
> I think this works okay for C, but I think if we ever attempted to provide 
> this functionality in C++, the calls to `addDecl()` would be rather expensive 
> because it eventually calls `addedMember()` which calculates information 
> about the structure (whether it's copy constructible, has a trivial 
> destructor, etc) and we don't need to redo any of that work.
> 
> However, for now, I think this is fine.
God help the person who does this for C++! :-)

Joking aside, supporting C++ will probably result in a almost total rewrite of 
this feature.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2503
+
+  // FIXME: Any way to get a handle to a RecordDecl struct here?
+  clang::randstruct::checkForBadCasts(S, AC);

aaron.ballman wrote:
> No, this function is only called when popping a function scope, and so the 
> only declaration it has access to is the `FunctionDecl`. Or do I 
> misunderstand the question?
Sounds good to me. Is this the best place for this check then?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8550-8556
+  case ParsedAttr::AT_RandomizeLayout:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_NoRandomizeLayout:
+// Drop the "randomize_layout" attribute if it's on the decl.
+D->dropAttr();
+break;

aaron.ballman 

[PATCH] D121556: [randstruct] Add randomize structure layout support

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 416354.
void marked 3 inline comments as done.
void added a comment.

General cleanup of code. Removing unneeded headers and comments. Removing a 
"const" from a setter. Not defining this for MS's declspecs or making the 
attributes keywords.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121556

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Randstruct.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Randstruct.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/RandstructTest.cpp

Index: clang/unittests/AST/RandstructTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/RandstructTest.cpp
@@ -0,0 +1,409 @@
+//===- unittest/AST/RandstructTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for Clang's structure field layout randomization.
+//
+//===--===//
+
+/*
+ * Build this test suite by running `make ASTTests` in the build folder.
+ *
+ * Run this test suite by running the following in the build folder:
+ * ` ./tools/clang/unittests/AST/ASTTests
+ * --gtest_filter=StructureLayoutRandomization*`
+ */
+
+#include "clang/AST/Randstruct.h"
+#include "gtest/gtest.h"
+
+#include "DeclMatcher.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Testing/CommandLineArgs.h"
+#include "clang/Tooling/Tooling.h"
+
+#include 
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::randstruct;
+
+using field_names = std::vector;
+
+namespace {
+
+std::unique_ptr makeAST(const std::string ) {
+  std::vector Args = getCommandLineArgsForTesting(Lang_C99);
+  Args.push_back("-frandstruct-seed=1234567890abcdef");
+  return tooling::buildASTFromCodeWithArgs(SourceCode, Args, "input.cc");
+}
+
+RecordDecl *getRecordDeclFromAST(const ASTContext , const std::string ) {
+  return FirstDeclMatcher().match(C.getTranslationUnitDecl(),
+  recordDecl(hasName(Name)));
+}
+
+std::vector getFieldNamesFromRecord(const RecordDecl *RD) {
+  std::vector Fields;
+
+  Fields.reserve(8);
+  for (auto *Field : RD->fields())
+Fields.push_back(Field->getNameAsString());
+
+  return Fields;
+}
+
+bool isSubsequence(const field_names , const field_names ) {
+  unsigned SeqLen = Seq.size();
+  unsigned SubLen = Subseq.size();
+
+  bool IsSubseq = false;
+  for (unsigned I = 0; I < SeqLen; ++I)
+if (Seq[I] == Subseq[0]) {
+  IsSubseq = true;
+  for (unsigned J = 0; J + I < SeqLen && J < SubLen; ++J) {
+if (Seq[J + I] != Subseq[J]) {
+  IsSubseq = false;
+  break;
+}
+  }
+}
+
+  return IsSubseq;
+}
+
+} // end anonymous namespace
+
+namespace clang {
+namespace ast_matchers {
+
+#define RANDSTRUCT_TEST_SUITE_TEST StructureLayoutRandomizationTestSuiteTest
+
+TEST(RANDSTRUCT_TEST_SUITE_TEST, CanDetermineIfSubsequenceExists) {
+  const field_names Seq = {"a", "b", "c", "d"};
+
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a", "b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(Seq, {"a"}));
+  ASSERT_FALSE(isSubsequence(Seq, {"a", "d"}));
+}
+
+#define RANDSTRUCT_TEST StructureLayoutRandomization
+
+TEST(RANDSTRUCT_TEST, UnmarkedStruct) {
+  const std::unique_ptr AST = makeAST(R"c(
+struct test {
+int bacon;
+long lettuce;
+long long tomato;
+float mayonnaise;
+};
+  )c");
+
+  const RecordDecl *RD = getRecordDeclFromAST(AST->getASTContext(), "test");
+  const field_names Expected = {"bacon", "lettuce", "tomato", "mayonnaise"};
+
+  ASSERT_FALSE(RD->getAttr());
+  ASSERT_FALSE(RD->isRandomized());
+  ASSERT_EQ(Expected, getFieldNamesFromRecord(RD));
+}
+
+TEST(RANDSTRUCT_TEST, MarkedNoRandomize) {
+  const std::unique_ptr AST = makeAST(R"c(
+

[PATCH] D121736: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

[Julian Lettner via Phabricator via cfe-commits]

yln added a comment.

In D121736#3389251 , @RKSimon wrote:

> LGTM - but please hang around after you commit

Thanks!  Looks like this it worked out.  Let me know if there is any other 
problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121736

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

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@hubert.reinterpretcast I added the tests, please let me know what you think. 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

In D118743#3290591 , 
@LegalizeAdulthood wrote:

> You've probably had this check in development for some time, but
> we just updated the documentation for contributing to clang-tidy
> and it might help to go over it for your new check to see if there
> is anything that stands out.
> https://clang.llvm.org/extra/clang-tidy/Contributing.html

Thanks, this is excellent documentation! I learned a plenty of new things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118743

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

[PATCH] D121969: Pass split-machine-functions to code generator when flto is used

[Junfeng Dong via Phabricator via cfe-commits]

junfd created this revision.
Herald added a subscriber: inglorion.
Herald added a project: All.
junfd 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/D121969

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/split-machine-functions.c


Index: clang/test/Driver/split-machine-functions.c
===
--- /dev/null
+++ clang/test/Driver/split-machine-functions.c
@@ -0,0 +1,8 @@
+// Test options pass-through with lto
+// RUN: %clang -### -flto -fsplit-machine-functions %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-PASS
+
+// Test no pass-through to ld without lto
+// RUN: %clang -### -fsplit-machine-functions %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-NOPASS
+
+// CHECK-PASS:  "-plugin-opt=-split-machine-functions"
+// CHECK-NOPASS-NOT:"-plugin-opt=-split-machine-functions"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -574,6 +574,10 @@
 CmdArgs.push_back("-plugin-opt=-data-sections");
   }
 
+  if (Args.hasArg(options::OPT_fsplit_machine_functions)) {
+  CmdArgs.push_back("-plugin-opt=-split-machine-functions");
+  }
+
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
 StringRef FName = A->getValue();
 if (!llvm::sys::fs::exists(FName))


Index: clang/test/Driver/split-machine-functions.c
===
--- /dev/null
+++ clang/test/Driver/split-machine-functions.c
@@ -0,0 +1,8 @@
+// Test options pass-through with lto
+// RUN: %clang -### -flto -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+
+// Test no pass-through to ld without lto
+// RUN: %clang -### -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
+
+// CHECK-PASS:  "-plugin-opt=-split-machine-functions"
+// CHECK-NOPASS-NOT:"-plugin-opt=-split-machine-functions"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -574,6 +574,10 @@
 CmdArgs.push_back("-plugin-opt=-data-sections");
   }
 
+  if (Args.hasArg(options::OPT_fsplit_machine_functions)) {
+  CmdArgs.push_back("-plugin-opt=-split-machine-functions");
+  }
+
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
 StringRef FName = A->getValue();
 if (!llvm::sys::fs::exists(FName))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp:64-68
+  unless(hasType(isVolatileQualified())), // non-volatile
+  unless(isTemplateVariable()),   // non-template
+  unless(isVarInline()),  // non-inline
+  unless(isExternC()),   // not "extern C" variable
+  unless(isInAnonymousNamespace())); // not within an anonymous namespace

LegalizeAdulthood wrote:
> Comment: Why don't we have a variadic `unless(...)` matcher?
> 
> Is there any utility to reordering these `unless()` clauses in
> order of "most likely encountered first" to get an early out
> when matching?
Thanks, it is more convenient!

> reordering these unless() clauses in order of "most likely encountered first" 
I reordered them based on my own approximal estimation. We could make a 
benchmark to observe the impact of different orders, but it goes beyond this 
patch.



Comment at: 
clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.h:19
+
+/// Finds non-extern non-inline function and variable definitions in header
+/// files, which can lead to potential ODR violations.

LegalizeAdulthood wrote:
> This block comment mentions functions, but the documentation only mentions 
> variables.
Oops, that's a typo: of course there are nothing abouth functions anywhere in 
the checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118743

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

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron updated this revision to Diff 416347.
Izaron marked 7 inline comments as done.
Izaron added a comment.
Herald added a project: All.

Fix issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118743

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  
clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-use-inline-const-variables-in-headers.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-inline-const-variables-in-headers.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-inline-const-variables-in-headers.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-inline-const-variables-in-headers.hpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s -std=c++17 modernize-use-inline-const-variables-in-headers %t
+
+const int ConstFoo = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: global constant 'ConstFoo' should be marked as 'inline' [modernize-use-inline-const-variables-in-headers]
+// CHECK-FIXES: {{^}}inline const int ConstFoo = 1;{{$}}
+
+namespace N {
+constexpr int NamespaceFoo = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: global constant 'NamespaceFoo' should be marked as 'inline' [modernize-use-inline-const-variables-in-headers]
+// CHECK-FIXES: {{^}}inline constexpr int NamespaceFoo = 1;{{$}}
+} // namespace N
+
+extern const int ExternFoo;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: global constant 'ExternFoo' should be converted to C++17 'inline variable' [modernize-use-inline-const-variables-in-headers]
+
+struct S {
+  // no warning: the variable is not at file scope
+  static const int StructFoo = 1;
+};
+
+// no warning: non-const global variables have external linkage
+int NonConstFoo = 1;
+
+// no warning: volatile global variables have external linkage
+const volatile int VolatileFoo = 1;
+
+// no warning: templates and their instantiations have external linkage
+template 
+const auto TemplateFoo = sizeof(T);
+
+// no warning: already fixed
+inline const int InlineFoo = 1;
+
+// no warning: C has no 'inline variables'
+extern "C" {
+const int CFoo0 = 1;
+}
+extern "C" const int CFoo1 = 1;
+
+// no warning: 'inline' is invisible when within an unnamed namespace
+namespace {
+const int AnonNamespaceFoo = 1;
+} // namespace
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-inline-const-variables-in-headers.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-use-inline-const-variables-in-headers.rst
@@ -0,0 +1,69 @@
+.. title:: clang-tidy - modernize-use-inline-const-variables-in-headers
+
+modernize-use-inline-const-variables-in-headers
+===
+
+
+Suggests switching to C++17 ``inline variables`` for non-inline const
+variable definitions and extern const variable declarations in header files.
+Non-inline const variables make a separate instance of the variable for each
+translation unit that includes the header, which can lead to subtle violation
+of the ODR. Extern const variables are a deprecated way to define a constant
+since C++17.
+
+.. code-block:: c++
+
+   // Foo.h
+   const int ConstFoo = 1; // Warning: should be marked as 'inline'
+
+   namespace N {
+ constexpr int NamespaceFoo = 1; // Warning: should be marked as 'inline'
+   }
+
+   extern const int ExternFoo; // Warning: should be converted to C++17 'inline variable'
+
+   struct S {
+ static const int StructFoo = 1; // OK: the variable is not at file scope
+   };
+
+   int NonConstFoo = 1; // No warning: non-const global variables have external linkage
+
+   const volatile int VolatileFoo = 1; // No warning: volatile global variables have external linkage
+
+   template
+   const auto TemplateFoo = sizeof(T); // OK: templates and their instantiations have external linkage
+
+   inline const int InlineFoo = 1; // no warning: already fixed
+
+   // No warning: C has no 'inline variables'
+   extern "C" {
+ const int CFoo0 = 1;
+   }
+   extern "C" const int CFoo1 = 1;
+
+   // No warning: 'inline' is invisible when within an unnamed namespace
+   namespace {
+ const int AnonNamespaceFoo = 1;
+   }
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave 

[PATCH] D121963: [clang] [OpenMP] Diagnose use of 'target_clones' in OpenMP variant declarations.

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121963

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

[PATCH] D121962: [clang] [OpenMP] Extend OpenMP variant declaration tests.

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121962

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

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

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Add Clang release note as well please.




Comment at: clang/docs/MisExpect.rst:58
+   Relaxes misexpect checking to tolerate profiling values within N% of the 
+   expected branch weight. e.g., a vlaue of ``N=5`` allows misexpect to check 
against
+   ``0.95 * Threshold``

value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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

[PATCH] D121965: Add validation for number of arguments of __builtin_memcpy_inline

[Roy Jacobson via Phabricator via cfe-commits]

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

__builtin_memcpy_inline doesn't use the usual builtin argument validation code,
so it crashed when receiving wrong number of argument. Add the missing 
validation
check.

Open issue: https://github.com/llvm/llvm-project/issues/52949


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121965

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtins-memcpy-inline.cpp


Index: clang/test/Sema/builtins-memcpy-inline.cpp
===
--- clang/test/Sema/builtins-memcpy-inline.cpp
+++ clang/test/Sema/builtins-memcpy-inline.cpp
@@ -42,3 +42,8 @@
   __builtin_memcpy_inline(ptr, a, 5);
   __builtin_memcpy_inline(a, ptr, 5);
 }
+
+void test_memcpy_inline_num_args(void *dst, void *src) {
+ __builtin_memcpy_inline(); // expected-error {{too few arguments to function 
call}}
+ __builtin_memcpy_inline(dst, src, 4, NULL); // expected-error {{too many 
arguments to function call}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1679,7 +1679,10 @@
 if ((ICEArguments & (1 << ArgNo)) == 0) continue;
 
 llvm::APSInt Result;
-if (SemaBuiltinConstantArg(TheCall, ArgNo, Result))
+// If we don't have enough arguments, continue so we can issue better
+// diagnostic in checkArgCount(...)
+if (ArgNo < TheCall->getNumArgs() &&
+SemaBuiltinConstantArg(TheCall, ArgNo, Result))
   return true;
 ICEArguments &= ~(1 << ArgNo);
   }
@@ -1943,6 +1946,8 @@
   case Builtin::BI__builtin_nontemporal_store:
 return SemaBuiltinNontemporalOverloaded(TheCallResult);
   case Builtin::BI__builtin_memcpy_inline: {
+if (checkArgCount(*this, TheCall, 3))
+  return ExprError();
 auto ArgArrayConversionFailed = [&](unsigned Arg) {
   ExprResult ArgExpr =
   DefaultFunctionArrayLvalueConversion(TheCall->getArg(Arg));


Index: clang/test/Sema/builtins-memcpy-inline.cpp
===
--- clang/test/Sema/builtins-memcpy-inline.cpp
+++ clang/test/Sema/builtins-memcpy-inline.cpp
@@ -42,3 +42,8 @@
   __builtin_memcpy_inline(ptr, a, 5);
   __builtin_memcpy_inline(a, ptr, 5);
 }
+
+void test_memcpy_inline_num_args(void *dst, void *src) {
+ __builtin_memcpy_inline(); // expected-error {{too few arguments to function call}}
+ __builtin_memcpy_inline(dst, src, 4, NULL); // expected-error {{too many arguments to function call}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1679,7 +1679,10 @@
 if ((ICEArguments & (1 << ArgNo)) == 0) continue;
 
 llvm::APSInt Result;
-if (SemaBuiltinConstantArg(TheCall, ArgNo, Result))
+// If we don't have enough arguments, continue so we can issue better
+// diagnostic in checkArgCount(...)
+if (ArgNo < TheCall->getNumArgs() &&
+SemaBuiltinConstantArg(TheCall, ArgNo, Result))
   return true;
 ICEArguments &= ~(1 << ArgNo);
   }
@@ -1943,6 +1946,8 @@
   case Builtin::BI__builtin_nontemporal_store:
 return SemaBuiltinNontemporalOverloaded(TheCallResult);
   case Builtin::BI__builtin_memcpy_inline: {
+if (checkArgCount(*this, TheCall, 3))
+  return ExprError();
 auto ArgArrayConversionFailed = [&](unsigned Arg) {
   ExprResult ArgExpr =
   DefaultFunctionArrayLvalueConversion(TheCall->getArg(Arg));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Teresa Johnson via Phabricator via cfe-commits]

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

lgtm

Just some minor cleanup before submitting as noted below.




Comment at: clang/test/Profile/misexpect-branch.c:3
+
+// test likely branch
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata

Comment is a little unclear, as it is testing both likely and unlikely branches.



Comment at: clang/test/Profile/misexpect-branch.c:9
+// there should be no diagnostics when the tolerance is sufficiently high, or 
when -Wmisexpect is not requested
+//
+// RUN: %clang_cc1 %s -O2 -o - -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify=foo 
-fdiagnostics-misexpect-tolerance=10 -Wmisexpect 
-debug-info-kind=line-tables-only

nit: stray empty comment line?



Comment at: clang/test/Profile/misexpect-branch.c:25
+  int x = 0;
+  if (likely(rando % (outer_loop * inner_loop) == 0)) { // exact-warning-re 
{{Potential performance regression from use of __builtin_expect(): Annotation 
was correct on {{.+}}% ({{[0-9]+ / [0-9]+}}) of profiled executions.}}
+x = baz(rando);

paulkirth wrote:
> tejohnson wrote:
> > Can you add a case where an unlikely() is wrong?
> I've added one, but I'm not sure it tests anything meaningfully, since an 
> unlikely path being too hot is really the same check as a likely path being 
> too cold. 
Right, it is just for completeness to test / illustrate that the handling works 
in both directions.



Comment at: llvm/docs/MisExpect.rst:2
+===
+Misexpect
+===

paulkirth wrote:
> tejohnson wrote:
> > paulkirth wrote:
> > > tejohnson wrote:
> > > > I don't see a mention of -Wmisexpect in the document. Should there be 
> > > > user-facing clang documentation, this seems more specific to the LLVM 
> > > > implementation?
> > > I will update the patch to include similar documentation this for clang.
> > > 
> > > Additionally, my understanding is that documentation for warnings is 
> > > auto-generated from the tablegen, so that at least will be available 
> > > automatically.
> > > 
> > > 
> > Should the clang documentation already be added to this patch? I couldn't 
> > find it.
> In the last update the clang documentation was a bit too close to the llvm 
> documentation. I think they are distinct enough now to be separately useful, 
> but maybe it's better to merge the two?
Yeah there is a fair amount of overlap, but I at least would want the clang 
documentation you added. Seems fine to have the llvm internals documentation 
too, though.



Comment at: llvm/docs/MisExpect.rst:54
+
+.. option:: -pgo-warn-misexpect
+

paulkirth wrote:
> tejohnson wrote:
> > Looking at the code, the -pgo-warn-misexpect seems to be useful for 
> > internal LLVM testing via 'opt', to mimic -Wmisexpect, so it probably 
> > should be documented as such.
> Yes, its useful to test llvm, but shouldn't `opt` be usable in the same way? 
> To me it seems useful for opt to support the warning directly too, but I'm 
> happy to defer here if you think that's confusing or shouldn't be the case.
Oh I'm not disagreeing with having the internal option and using it for opt, 
that's very useful. My comment was in the context of not having the user-facing 
clang documentation with the clang driver level option (which you since added). 
I was just suggesting you add a note that this option is an alternative to the 
clang option for use when e.g. testing via opt. Since there is now separate 
clang documentation I think it is less important now.



Comment at: llvm/include/llvm/Transforms/Utils/MisExpectToleranceParser.h:27
+// Valid option values are integers in the range [0, 100)
+inline Expected> parseToleranceOption(StringRef Arg) {
+  int64_t Val;

Given that there is only one callsite to this helper, I think just move it into 
the calling source file and avoid adding a new header.



Comment at: llvm/lib/IR/LLVMContextImpl.h:1384
+  /// The percentage of difference between profiling branch weights and
+  // llvm.expect branch weights to tollerate when emiting MisExpect diagnostics
+  Optional DiagnosticsMisExpectTolerance = 0;

s/tollerate/tolerate/



Comment at: llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll:3
+
+; RUN: opt < %s -lower-expect -pgo-instr-use 
-pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 
-pass-remarks=misexpect 2>&1 | FileCheck %s
+

tejohnson wrote:
> The old PM is gone in the optimization pipeline, so these old PM lines in 
> this and other tests and the comment about the new PM can go away.
I see you removed the NewPM-style invocations and kept the legacy ones - afaict 
opt will now convert the legacy style pass invocations to the new PM, but I 
think 

[PATCH] D121963: [clang] [OpenMP] Diagnose use of 'target_clones' in OpenMP variant declarations.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
tahonermann published this revision for review.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Previously, OpenMP variant declarations for a function declaration that included
the 'cpu_dispatch', 'cpu_specific', or 'target' attributes was diagnosed, but
one with the 'target_clones' attribute was not. Now fixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121963

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_messages.c


Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -162,9 +162,8 @@
 #pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
 __attribute__((target("default"))) void incompat_attr_target_default(void); // 
expected-error {{'#pragma omp declare variant' is not compatible with any 
target-specific attributes}}
 
-// FIXME: No diagnostics are produced for use of the 'target_clones' attribute 
in an OMP variant declaration.
 #pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
-__attribute__((target_clones("sse,default"))) void 
incompat_attr_target_clones(void);
+__attribute__((target_clones("sse,default"))) void 
incompat_attr_target_clones(void); // expected-error {{'#pragma omp declare 
variant' is not compatible with any target-specific attributes}}
 
 void marked(void);
 void not_marked(void);
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7032,11 +7032,11 @@
   }
 
   auto & = [](const FunctionDecl *FD) {
-return FD->hasAttrs() &&
-   (FD->hasAttr() || FD->hasAttr() ||
-FD->hasAttr());
+// The 'target' attribute needs to be separately checked because it does
+// not always signify a multiversion function declaration.
+return FD->isMultiVersion() || FD->hasAttr();
   };
-  // OpenMP is not compatible with CPU-specific attributes.
+  // OpenMP is not compatible with multiversion function attributes.
   if (HasMultiVersionAttributes(FD)) {
 Diag(FD->getLocation(), diag::err_omp_declare_variant_incompat_attributes)
 << SR;


Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -162,9 +162,8 @@
 #pragma omp declare variant(incompat_attr_variant) match(implementation={vendor(llvm)})
 __attribute__((target("default"))) void incompat_attr_target_default(void); // expected-error {{'#pragma omp declare variant' is not compatible with any target-specific attributes}}
 
-// FIXME: No diagnostics are produced for use of the 'target_clones' attribute in an OMP variant declaration.
 #pragma omp declare variant(incompat_attr_variant) match(implementation={vendor(llvm)})
-__attribute__((target_clones("sse,default"))) void incompat_attr_target_clones(void);
+__attribute__((target_clones("sse,default"))) void incompat_attr_target_clones(void); // expected-error {{'#pragma omp declare variant' is not compatible with any target-specific attributes}}
 
 void marked(void);
 void not_marked(void);
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7032,11 +7032,11 @@
   }
 
   auto & = [](const FunctionDecl *FD) {
-return FD->hasAttrs() &&
-   (FD->hasAttr() || FD->hasAttr() ||
-FD->hasAttr());
+// The 'target' attribute needs to be separately checked because it does
+// not always signify a multiversion function declaration.
+return FD->isMultiVersion() || FD->hasAttr();
   };
-  // OpenMP is not compatible with CPU-specific attributes.
+  // OpenMP is not compatible with multiversion function attributes.
   if (HasMultiVersionAttributes(FD)) {
 Diag(FD->getLocation(), diag::err_omp_declare_variant_incompat_attributes)
 << SR;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121962: [clang] [OpenMP] Extend OpenMP variant declaration tests.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
tahonermann published this revision for review.
tahonermann added inline comments.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.



Comment at: clang/test/OpenMP/declare_variant_messages.c:165
+
+// FIXME: No diagnostics are produced for use of the 'target_clones' attribute 
in an OMP variant declaration.
+#pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})

This FIXME comment is removed with the changes made in D121963.


This change extends the existing diagnostic tests for OpenMP variant
declarations to cover diagnostics for declarations that include
multiversion function attributes. The new tests demonstrate a missing
check for the 'target_clones' attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121962

Files:
  clang/test/OpenMP/declare_variant_messages.c


Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -142,6 +142,30 @@
 #pragma omp declare variant(diff_ret_variant) match(xxx={}) // expected-error 
{{variant in '#pragma omp declare variant' with type 'int (void)' is 
incompatible with type 'void (void)'}} expected-warning {{'xxx' is not a valid 
context set in a `declare variant`; set ignored}} expected-note {{context set 
options are: 'construct' 'device' 'implementation' 'user'}} expected-note {{the 
ignored set spans until here}}
 void diff_ret(void);
 
+void incompat_attr_variant(void);
+
+#pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
+__attribute__((cpu_dispatch(generic))) void incompat_attr_cpu_dispatch(void); 
// expected-error {{'#pragma omp declare variant' is not compatible with any 
target-specific attributes}}
+
+#pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
+__attribute__((cpu_specific(generic))) void incompat_attr_cpu_specific(void); 
// expected-error {{'#pragma omp declare variant' is not compatible with any 
target-specific attributes}}
+
+// 'incompat_attr_target' is not a multiversion function until...
+#pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
+__attribute__((target("mmx"))) void incompat_attr_target(void); // 
expected-error {{'#pragma omp declare variant' is not compatible with any 
target-specific attributes}}
+
+// This declaration makes it one.
+#pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
+__attribute__((target("sse"))) void incompat_attr_target(void); // 
expected-error {{'#pragma omp declare variant' is not compatible with any 
target-specific attributes}}
+
+// 'incompat_attr_target_default' is always a multiversion function.
+#pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
+__attribute__((target("default"))) void incompat_attr_target_default(void); // 
expected-error {{'#pragma omp declare variant' is not compatible with any 
target-specific attributes}}
+
+// FIXME: No diagnostics are produced for use of the 'target_clones' attribute 
in an OMP variant declaration.
+#pragma omp declare variant(incompat_attr_variant) 
match(implementation={vendor(llvm)})
+__attribute__((target_clones("sse,default"))) void 
incompat_attr_target_clones(void);
+
 void marked(void);
 void not_marked(void);
 


Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -142,6 +142,30 @@
 #pragma omp declare variant(diff_ret_variant) match(xxx={}) // expected-error {{variant in '#pragma omp declare variant' with type 'int (void)' is incompatible with type 'void (void)'}} expected-warning {{'xxx' is not a valid context set in a `declare variant`; set ignored}} expected-note {{context set options are: 'construct' 'device' 'implementation' 'user'}} expected-note {{the ignored set spans until here}}
 void diff_ret(void);
 
+void incompat_attr_variant(void);
+
+#pragma omp declare variant(incompat_attr_variant) match(implementation={vendor(llvm)})
+__attribute__((cpu_dispatch(generic))) void incompat_attr_cpu_dispatch(void); // expected-error {{'#pragma omp declare variant' is not compatible with any target-specific attributes}}
+
+#pragma omp declare variant(incompat_attr_variant) match(implementation={vendor(llvm)})
+__attribute__((cpu_specific(generic))) void incompat_attr_cpu_specific(void); // expected-error {{'#pragma omp declare variant' is not compatible with any target-specific attributes}}
+
+// 'incompat_attr_target' 

[PATCH] D121961: [clang] Produce a "multiversion" annotation in textual AST output.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change adds a "multiversion" annotation to textual AST output.
For example:

  FunctionDecl 0xb6628b0  col:5 multiversion foo 'int (void)'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121961

Files:
  clang/lib/AST/TextNodeDumper.cpp


Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -283,6 +283,8 @@
   OS << " constexpr";
 if (FD->isConsteval())
   OS << " consteval";
+if (FD->isMultiVersion())
+  OS << " multiversion";
   }
 
   if (!isa(*D)) {


Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -283,6 +283,8 @@
   OS << " constexpr";
 if (FD->isConsteval())
   OS << " consteval";
+if (FD->isMultiVersion())
+  OS << " multiversion";
   }
 
   if (!isa(*D)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121960: [clang] NFC: Rename 'MVType' variables to 'MVKind' for consistency with their type.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121960

Files:
  clang/lib/Sema/SemaDecl.cpp

Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10365,14 +10365,14 @@
 // Provide a white-list of attributes that are allowed to be combined with
 // multiversion functions.
 static bool AttrCompatibleWithMultiVersion(attr::Kind Kind,
-   MultiVersionKind MVType) {
+   MultiVersionKind MVKind) {
   // Note: this list/diagnosis must match the list in
   // checkMultiversionAttributesAllSame.
   switch (Kind) {
   default:
 return false;
   case attr::Used:
-return MVType == MultiVersionKind::Target;
+return MVKind == MultiVersionKind::Target;
   case attr::NonNull:
   case attr::NoThrow:
 return true;
@@ -10382,10 +10382,10 @@
 static bool checkNonMultiVersionCompatAttributes(Sema ,
  const FunctionDecl *FD,
  const FunctionDecl *CausedFD,
- MultiVersionKind MVType) {
-  const auto Diagnose = [FD, CausedFD, MVType](Sema , const Attr *A) {
+ MultiVersionKind MVKind) {
+  const auto Diagnose = [FD, CausedFD, MVKind](Sema , const Attr *A) {
 S.Diag(FD->getLocation(), diag::err_multiversion_disallowed_other_attr)
-<< static_cast(MVType) << A;
+<< static_cast(MVKind) << A;
 if (CausedFD)
   S.Diag(CausedFD->getLocation(), diag::note_multiversioning_caused_here);
 return true;
@@ -10395,20 +10395,20 @@
 switch (A->getKind()) {
 case attr::CPUDispatch:
 case attr::CPUSpecific:
-  if (MVType != MultiVersionKind::CPUDispatch &&
-  MVType != MultiVersionKind::CPUSpecific)
+  if (MVKind != MultiVersionKind::CPUDispatch &&
+  MVKind != MultiVersionKind::CPUSpecific)
 return Diagnose(S, A);
   break;
 case attr::Target:
-  if (MVType != MultiVersionKind::Target)
+  if (MVKind != MultiVersionKind::Target)
 return Diagnose(S, A);
   break;
 case attr::TargetClones:
-  if (MVType != MultiVersionKind::TargetClones)
+  if (MVKind != MultiVersionKind::TargetClones)
 return Diagnose(S, A);
   break;
 default:
-  if (!AttrCompatibleWithMultiVersion(A->getKind(), MVType))
+  if (!AttrCompatibleWithMultiVersion(A->getKind(), MVKind))
 return Diagnose(S, A);
   break;
 }
@@ -10532,7 +10532,7 @@
 static bool CheckMultiVersionAdditionalRules(Sema , const FunctionDecl *OldFD,
  const FunctionDecl *NewFD,
  bool CausesMV,
- MultiVersionKind MVType) {
+ MultiVersionKind MVKind) {
   if (!S.getASTContext().getTargetInfo().supportsMultiVersioning()) {
 S.Diag(NewFD->getLocation(), diag::err_multiversion_not_supported);
 if (OldFD)
@@ -10540,15 +10540,15 @@
 return true;
   }
 
-  bool IsCPUSpecificCPUDispatchMVType =
-  MVType == MultiVersionKind::CPUDispatch ||
-  MVType == MultiVersionKind::CPUSpecific;
+  bool IsCPUSpecificCPUDispatchMVKind =
+  MVKind == MultiVersionKind::CPUDispatch ||
+  MVKind == MultiVersionKind::CPUSpecific;
 
   if (CausesMV && OldFD &&
-  checkNonMultiVersionCompatAttributes(S, OldFD, NewFD, MVType))
+  checkNonMultiVersionCompatAttributes(S, OldFD, NewFD, MVKind))
 return true;
 
-  if (checkNonMultiVersionCompatAttributes(S, NewFD, nullptr, MVType))
+  if (checkNonMultiVersionCompatAttributes(S, NewFD, nullptr, MVKind))
 return true;
 
   // Only allow transition to MultiVersion if it hasn't been used.
@@ -10561,11 +10561,11 @@
   S.PDiag(diag::note_multiversioning_caused_here)),
   PartialDiagnosticAt(NewFD->getLocation(),
   S.PDiag(diag::err_multiversion_doesnt_support)
-  << static_cast(MVType)),
+  << static_cast(MVKind)),
   PartialDiagnosticAt(NewFD->getLocation(),
   S.PDiag(diag::err_multiversion_diff)),
   /*TemplatesSupported=*/false,
-  /*ConstexprSupported=*/!IsCPUSpecificCPUDispatchMVType,
+  /*ConstexprSupported=*/!IsCPUSpecificCPUDispatchMVKind,
   /*CLinkageMayDiffer=*/false);
 }
 
@@ -10576,22 +10576,22 @@
 ///
 /// Returns true if there was an error, false otherwise.
 static bool CheckMultiVersionFirstFunction(Sema , 

[PATCH] D121959: [clang] Add missing diagnostics for invalid overloads of multiversion functions in C.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann published this revision for review.
tahonermann added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change resolves both LLVM issue 
https://github.com/llvm/llvm-project/issues/50192 and Intel issue 
CMPLRLLVM-28177.


Previously, an attempt to declare an overload of a multiversion function
in C was not properly diagnosed. In some cases, diagnostics were simply
missing. In other cases the following assertion failure occured...

  Assertion `(Previous.empty() || llvm::any_of(Previous, [](const NamedDecl 
*ND) { return ND->hasAttr(); })) && "Non-redecls shouldn't happen without 
overloadable present"' failed.

... or the following diagnostic was spuriously issued.

  error: at most one overload for a given name may lack the 'overloadable' 
attribute

The diagnostics issued in some cases could be improved. When the function
type of a redeclaration does not match the prior declaration, it would be
preferable to diagnose the type mismatch before diagnosing mismatched
attributes. Diagnostics are also missing for some cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121959

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/attr-cpuspecific.c
  clang/test/Sema/attr-target-clones.c
  clang/test/Sema/attr-target-mv.c

Index: clang/test/Sema/attr-target-mv.c
===
--- clang/test/Sema/attr-target-mv.c
+++ clang/test/Sema/attr-target-mv.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -Wno-strict-prototypes -fsyntax-only -verify %s
-// XFAIL: asserts
 
 void __attribute__((target("sse4.2"))) no_default(void);
 void __attribute__((target("arch=sandybridge")))  no_default(void);
@@ -109,13 +108,9 @@
 void __attribute__((target("sse4.2"))) addtl_attrs6(int*);
 void __attribute__((target("arch=sandybridge"), nothrow, used, nonnull)) addtl_attrs6(int*);
 
-// FIXME: Declaration of a non-overloadable function when more than one
-// FIXME: multiversion function declarations are present results in an
-// FIXME: assertion failure.
 int __attribute__((target("sse4.2"))) bad_overload1(void);
 int __attribute__((target("arch=sandybridge"))) bad_overload1(void);
-// expected-error@+2 {{at most one overload for a given name may lack the 'overloadable' attribute}}
-// expected-note@-2 {{previous unmarked overload of function is here}}
+// expected-error@+1 {{function declaration is missing 'target' attribute in a multiversioned function}}
 int bad_overload1(int);
 
 int bad_overload2(int);
Index: clang/test/Sema/attr-target-clones.c
===
--- clang/test/Sema/attr-target-clones.c
+++ clang/test/Sema/attr-target-clones.c
@@ -90,9 +90,13 @@
 void bad_overload1(int p) {}
 
 void bad_overload2(int p) {}
+// expected-error@+2 {{conflicting types for 'bad_overload2'}}
+// expected-note@-2 {{previous definition is here}}
 void bad_overload2(void) __attribute__((target_clones("mmx", "sse4.2", "default")));
 
 void bad_overload3(void) __attribute__((target_clones("mmx", "sse4.2", "default")));
+// expected-error@+2 {{conflicting types for 'bad_overload3'}}
+// expected-note@-2 {{previous declaration is here}}
 void bad_overload3(int) __attribute__((target_clones("mmx", "sse4.2", "default")));
 
 void good_overload1(void) __attribute__((target_clones("mmx", "sse4.2", "default")));
Index: clang/test/Sema/attr-cpuspecific.c
===
--- clang/test/Sema/attr-cpuspecific.c
+++ clang/test/Sema/attr-cpuspecific.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -Wno-strict-prototypes -fsyntax-only -verify %s -Wnonnull
-// XFAIL: asserts
 
 void __attribute__((cpu_specific(ivybridge))) no_default(void);
 void __attribute__((cpu_specific(sandybridge)))  no_default(void);
@@ -120,13 +119,9 @@
 
 void __attribute__((cpu_specific(atom), nothrow, nonnull(1))) addtl_attrs(int*);
 
-// FIXME: Declaration of a non-overloadable function when more than one
-// FIXME: multiversion function declarations are present results in an
-// FIXME: assertion failure.
 int __attribute__((cpu_specific(atom))) bad_overload1(void);
 int __attribute__((cpu_specific(ivybridge))) bad_overload1(void);
-// expected-error@+2 {{at most one overload for a given name may lack the 'overloadable' attribute}}
-// expected-note@-2 {{previous unmarked overload of function is here}}
+// expected-error@+1 {{function declaration is missing 'cpu_specific' or 'cpu_dispatch' attribute in a multiversioned function}}
 int bad_overload1(int);
 
 int bad_overload2(int);
@@ -135,13 +130,9 @@
 int __attribute__((cpu_specific(atom))) bad_overload2(void);
 int __attribute__((cpu_specific(ivybridge))) bad_overload2(void);
 
-// FIXME: Declaration of a non-overloadable function when more than one

[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 416332.
iains added a comment.

adress review comments, adjust test case for windows compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588

Files:
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/FrontendOptions.cpp
  clang/test/Driver/Inputs/header-unit-01.hh
  clang/test/Driver/cxx20-header-units-01.cpp

Index: clang/test/Driver/cxx20-header-units-01.cpp
===
--- /dev/null
+++ clang/test/Driver/cxx20-header-units-01.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -std=c++20 -xc++-user-header foo.h 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-USER %s
+
+// RUN: %clang -### -std=c++20 -xc++-system-header vector 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-SYSTEM %s
+
+// RUN: %clang -### -std=c++20 \
+// RUN:   -xc++-header-unit-header %S/Inputs/header-unit-01.hh 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ABS %s -DTDIR=%S/Inputs
+
+// CHECK-USER: "-emit-header-unit"
+// CHECK-USER-SAME: "-o" "foo.pcm"
+// CHECK-USER-SAME: "-x" "c++-user-header" "foo.h"
+// CHECK-SYSTEM: "-emit-header-unit"
+// CHECK-SYSTEM-SAME: "-o" "vector.pcm"
+// CHECK-SYSTEM-SAME: "-x" "c++-system-header" "vector"
+// CHECK-ABS: "-emit-header-unit"
+// CHECK-ABS-SAME: "-o" "header-unit-01.pcm"
+// CHECK-ABS-SAME: "-x" "c++-header-unit-header" "[[TDIR]]/header-unit-01.hh"
Index: clang/lib/Frontend/FrontendOptions.cpp
===
--- clang/lib/Frontend/FrontendOptions.cpp
+++ clang/lib/Frontend/FrontendOptions.cpp
@@ -27,7 +27,7 @@
   .Cases("C", "cc", "cp", Language::CXX)
   .Cases("cpp", "CPP", "c++", "cxx", "hpp", "hxx", Language::CXX)
   .Case("cppm", Language::CXX)
-  .Case("iim", InputKind(Language::CXX).getPreprocessed())
+  .Cases("iim", "iih", InputKind(Language::CXX).getPreprocessed())
   .Case("cl", Language::OpenCL)
   .Case("clcpp", Language::OpenCLCXX)
   .Cases("cu", "cuh", Language::CUDA)
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -65,9 +65,16 @@
   return Id == TY_CXXModule || Id == TY_PP_CXXModule;
 }
 
+static bool isPreprocessedHeaderUnitType(ID Id) {
+  return Id == TY_CXXSHeader || Id == TY_CXXUHeader || Id == TY_CXXHUHeader ||
+ Id == TY_PP_CXXHeaderUnit;
+}
+
 types::ID types::getPrecompiledType(ID Id) {
   if (isPreprocessedModuleType(Id))
 return TY_ModuleFile;
+  if (isPreprocessedHeaderUnitType(Id))
+return TY_HeaderUnit;
   if (onlyPrecompileType(Id))
 return TY_PCH;
   return TY_INVALID;
@@ -139,6 +146,10 @@
   case TY_CLHeader:
   case TY_ObjCHeader: case TY_PP_ObjCHeader:
   case TY_CXXHeader: case TY_PP_CXXHeader:
+  case TY_CXXSHeader:
+  case TY_CXXUHeader:
+  case TY_CXXHUHeader:
+  case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
   case TY_CXXModule: case TY_PP_CXXModule:
   case TY_AST: case TY_ModuleFile: case TY_PCH:
@@ -210,6 +221,10 @@
   case TY_CXX: case TY_PP_CXX:
   case TY_ObjCXX: case TY_PP_ObjCXX: case TY_PP_ObjCXX_Alias:
   case TY_CXXHeader: case TY_PP_CXXHeader:
+  case TY_CXXSHeader:
+  case TY_CXXUHeader:
+  case TY_CXXHUHeader:
+  case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
   case TY_CXXModule: case TY_PP_CXXModule:
   case TY_CUDA: case TY_PP_CUDA: case TY_CUDA_DEVICE:
@@ -323,6 +338,7 @@
.Case("hpp", TY_CXXHeader)
.Case("hxx", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
+   .Case("iih", TY_PP_CXXHeaderUnit)
.Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)
.Case("obj", TY_Object)
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4607,6 +4607,8 @@
   CmdArgs.push_back(IsHeaderModulePrecompile
 ? "-emit-header-module"
 : "-emit-module-interface");
+else if (JA.getType() == types::TY_HeaderUnit)
+  CmdArgs.push_back("-emit-header-unit");
 else
   CmdArgs.push_back("-emit-pch");
   } else if (isa(JA)) {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2209,6 +2209,11 @@
   if (Value == "-")
 return true;
 
+  // If it's a header to be found in the system or user search path, then defer
+  // complaints about its absence until those searches can be done.
+  if (Ty == types::TY_CXXSHeader || Ty == types::TY_CXXUHeader)
+return true;
+
   if 

[PATCH] D121958: [clang] NFC: Remove forced type merging in multiversion function checks.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Checking of multiversion function declarations performed by various functions
in clang/lib/Sema/SemaDecl.cpp previously forced the valus of a passed in
'MergeTypeWithPrevious' reference argument in several scenarios. This was
unnecessary and possibly incorrect in the one case that the value
was forced to 'true' (though seemingly unobservably so).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121958

Files:
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10597,7 +10597,7 @@
 
 static bool CheckTargetCausesMultiVersioning(
 Sema , FunctionDecl *OldFD, FunctionDecl *NewFD, const TargetAttr *NewTA,
-bool , NamedDecl *, bool ,
+bool , NamedDecl *,
 LookupResult ) {
   const auto *OldTA = OldFD->getAttr();
   ParsedTargetAttr NewParsed = NewTA->parse();
@@ -10663,7 +10663,6 @@
   OldFD->setIsMultiVersion();
   NewFD->setIsMultiVersion();
   Redeclaration = false;
-  MergeTypeWithPrevious = false;
   OldDecl = nullptr;
   Previous.clear();
   return false;
@@ -10688,7 +10687,7 @@
 MultiVersionKind NewMVType, const TargetAttr *NewTA,
 const CPUDispatchAttr *NewCPUDisp, const CPUSpecificAttr *NewCPUSpec,
 const TargetClonesAttr *NewClones, bool , NamedDecl 
*,
-bool , LookupResult ) {
+LookupResult ) {
 
   MultiVersionKind OldMVType = OldFD->getMultiVersionKind();
   // Disallow mixing of multiversioning types.
@@ -10744,7 +10743,6 @@
   const auto *CurClones = CurFD->getAttr();
   Redeclaration = true;
   OldDecl = CurFD;
-  MergeTypeWithPrevious = true;
   NewFD->setIsMultiVersion();
 
   if (CurClones && NewClones &&
@@ -10847,7 +10845,6 @@
 
   NewFD->setIsMultiVersion();
   Redeclaration = false;
-  MergeTypeWithPrevious = false;
   OldDecl = nullptr;
   Previous.clear();
   return false;
@@ -10861,7 +10858,6 @@
 /// Returns true if there was an error, false otherwise.
 static bool CheckMultiVersionFunction(Sema , FunctionDecl *NewFD,
   bool , NamedDecl *,
-  bool ,
   LookupResult ) {
   const auto *NewTA = NewFD->getAttr();
   const auto *NewCPUDisp = NewFD->getAttr();
@@ -10911,7 +10907,7 @@
 case MultiVersionKind::Target:
   return CheckTargetCausesMultiVersioning(S, OldFD, NewFD, NewTA,
   Redeclaration, OldDecl,
-  MergeTypeWithPrevious, Previous);
+  Previous);
 case MultiVersionKind::TargetClones:
   if (OldFD->isUsed(false)) {
 NewFD->setInvalidDecl();
@@ -10931,7 +10927,7 @@
   // still compatible with previous declarations.
   return CheckMultiVersionAdditionalDecl(
   S, OldFD, NewFD, MVType, NewTA, NewCPUDisp, NewCPUSpec, NewClones,
-  Redeclaration, OldDecl, MergeTypeWithPrevious, Previous);
+  Redeclaration, OldDecl, Previous);
 }
 
 /// Perform semantic checking of a new function declaration.
@@ -11022,7 +11018,7 @@
   }
 
   if (CheckMultiVersionFunction(*this, NewFD, Redeclaration, OldDecl,
-MergeTypeWithPrevious, Previous))
+Previous))
 return Redeclaration;
 
   // PPC MMA non-pointer types are not allowed as function return types.


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10597,7 +10597,7 @@
 
 static bool CheckTargetCausesMultiVersioning(
 Sema , FunctionDecl *OldFD, FunctionDecl *NewFD, const TargetAttr *NewTA,
-bool , NamedDecl *, bool ,
+bool , NamedDecl *,
 LookupResult ) {
   const auto *OldTA = OldFD->getAttr();
   ParsedTargetAttr NewParsed = NewTA->parse();
@@ -10663,7 +10663,6 @@
   OldFD->setIsMultiVersion();
   NewFD->setIsMultiVersion();
   Redeclaration = false;
-  MergeTypeWithPrevious = false;
   OldDecl = nullptr;
   Previous.clear();
   return false;
@@ -10688,7 +10687,7 @@
 MultiVersionKind NewMVType, const TargetAttr *NewTA,
 const CPUDispatchAttr *NewCPUDisp, const CPUSpecificAttr *NewCPUSpec,
 const TargetClonesAttr *NewClones, bool , NamedDecl *,
-bool , LookupResult ) {
+LookupResult ) {
 
   MultiVersionKind OldMVType = OldFD->getMultiVersionKind();
   // Disallow mixing of multiversioning types.
@@ -10744,7 +10743,6 @@
   const auto *CurClones = CurFD->getAttr();
   Redeclaration = true;
   OldDecl = CurFD;
-  MergeTypeWithPrevious = true;
   NewFD->setIsMultiVersion();
 
  

[PATCH] D121957: [clang] NFC: Redundant code removal in SemaDecl.cpp, CheckTargetCausesMultiVersioning().

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann published this revision for review.
tahonermann added inline comments.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.



Comment at: clang/lib/Sema/SemaDecl.cpp:10522-10527
   if (!S.getASTContext().getTargetInfo().supportsMultiVersioning()) {
 S.Diag(NewFD->getLocation(), diag::err_multiversion_not_supported);
 if (OldFD)
   S.Diag(OldFD->getLocation(), diag::note_previous_declaration);
 return true;
   }

This is the code that makes the removed code redundant.


This change removes redundant code in the definition of
CheckTargetCausesMultiVersioning() in SemaDecl.cpp. The removed code checked
for multiversion function support. The code immediately following the removed
code is a call to CheckMultiVersionAdditionalRules(); that function performs
the same check on entry. In both cases, the consequences of missing multiversion
function support results in the same diagnostic message being issued and the
applicable function declaration being marked as invalid.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121957

Files:
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10611,13 +10611,6 @@
 return false;
 
   // Otherwise, this decl causes MultiVersioning.
-  if (!S.getASTContext().getTargetInfo().supportsMultiVersioning()) {
-S.Diag(NewFD->getLocation(), diag::err_multiversion_not_supported);
-S.Diag(OldFD->getLocation(), diag::note_previous_declaration);
-NewFD->setInvalidDecl();
-return true;
-  }
-
   if (CheckMultiVersionAdditionalRules(S, OldFD, NewFD, true,
MultiVersionKind::Target)) {
 NewFD->setInvalidDecl();


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10611,13 +10611,6 @@
 return false;
 
   // Otherwise, this decl causes MultiVersioning.
-  if (!S.getASTContext().getTargetInfo().supportsMultiVersioning()) {
-S.Diag(NewFD->getLocation(), diag::err_multiversion_not_supported);
-S.Diag(OldFD->getLocation(), diag::note_previous_declaration);
-NewFD->setInvalidDecl();
-return true;
-  }
-
   if (CheckMultiVersionAdditionalRules(S, OldFD, NewFD, true,
MultiVersionKind::Target)) {
 NewFD->setInvalidDecl();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121733: Clean pathnames in FileManager.

[Paul Pluzhnikov via Phabricator via cfe-commits]

ppluzhnikov updated this revision to Diff 416330.
ppluzhnikov added a comment.

Use single quotes in sed -- don't want shell expansion there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

Files:
  clang-tools-extra/test/clang-apply-replacements/order-dependent.cpp
  clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
  clang-tools-extra/test/clang-tidy/checkers/abseil-no-namespace.cpp
  clang/bindings/python/tests/cindex/test_translation_unit.py
  clang/lib/Basic/FileManager.cpp
  clang/test/Frontend/dependency-gen.c
  clang/test/Index/skip-parsed-bodies/compile_commands.json
  clang/test/Modules/filename.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -1593,21 +1593,22 @@
   return L.getFilePath() < R.getFilePath();
 });
 
-  ASSERT_EQ(Changes[0].getFilePath(), "./input.h");
-  EXPECT_THAT(Changes[0].getInsertedHeaders(), IsEmpty());
-  EXPECT_THAT(Changes[0].getRemovedHeaders(), IsEmpty());
+  const auto _h = Changes[1];
+  ASSERT_EQ(change_h.getFilePath(), "input.h");
+  EXPECT_THAT(change_h.getInsertedHeaders(), IsEmpty());
+  EXPECT_THAT(change_h.getRemovedHeaders(), IsEmpty());
   llvm::Expected UpdatedCode =
-  clang::tooling::applyAllReplacements(Header,
-   Changes[0].getReplacements());
+  clang::tooling::applyAllReplacements(Header, change_h.getReplacements());
   ASSERT_TRUE(static_cast(UpdatedCode))
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
   EXPECT_EQ(format(*UpdatedCode), "");
 
-  ASSERT_EQ(Changes[1].getFilePath(), "input.cc");
-  EXPECT_THAT(Changes[1].getInsertedHeaders(), IsEmpty());
-  EXPECT_THAT(Changes[1].getRemovedHeaders(), IsEmpty());
-  UpdatedCode = clang::tooling::applyAllReplacements(
-  Source, Changes[1].getReplacements());
+  const auto _cc = Changes[0];
+  ASSERT_EQ(change_cc.getFilePath(), "input.cc");
+  EXPECT_THAT(change_cc.getInsertedHeaders(), IsEmpty());
+  EXPECT_THAT(change_cc.getRemovedHeaders(), IsEmpty());
+  UpdatedCode =
+  clang::tooling::applyAllReplacements(Source, change_cc.getReplacements());
   ASSERT_TRUE(static_cast(UpdatedCode))
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
   EXPECT_EQ(format(*UpdatedCode), format("#include \"input.h\"\n"));
@@ -1630,7 +1631,7 @@
   {{"input.h", Header}}));
 
   ASSERT_EQ(Changes.size(), 1U);
-  ASSERT_EQ(Changes[0].getFilePath(), "./input.h");
+  ASSERT_EQ(Changes[0].getFilePath(), "input.h");
   EXPECT_THAT(Changes[0].getInsertedHeaders(), ElementsAre("header.h"));
   EXPECT_THAT(Changes[0].getRemovedHeaders(), IsEmpty());
   llvm::Expected UpdatedCode =
@@ -1679,7 +1680,7 @@
   ResultOf([](const AtomicChange ) { return C.getFilePath(); },
"input.cc"),
   ResultOf([](const AtomicChange ) { return C.getFilePath(); },
-   "./input.h";
+   "input.h";
 }
 
 } // namespace
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -292,7 +292,7 @@
   {"int main() {}",
R"(expanded tokens:
   int main ( ) { }
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 int main ( ) { }
   no mappings.
@@ -301,7 +301,7 @@
   {"\t\n  int\t\n  main\t\n  (\t\n  )\t\n{\t\n  }\t\n",
R"(expanded tokens:
   int main ( ) { }
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 int main ( ) { }
   no mappings.
@@ -313,7 +313,7 @@
   )cpp",
R"(expanded tokens:
   
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 # pragma GCC visibility push ( public ) # pragma GCC visibility pop
   mappings:
@@ -322,7 +322,7 @@
   // Empty files should not crash.
   {R"cpp()cpp", R"(expanded tokens:
   
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 
   no mappings.
@@ -336,7 +336,7 @@
 )cpp",
   R"(expanded tokens:
   a
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 a # define MACRO ( ) A # B
   mappings:
@@ -401,7 +401,7 @@
   std::string Expected =
   "expanded tokens:\n"
   "  int a ;\n"
-  "file './input.cpp'\n"
+  "file 'input.cpp'\n"
   "  spelled tokens:\n"
   "# define FOO a # include \"unresolved_file.h\" # undef FOO "
   "# ifdef X # else # endif # ifndef Y # endif # if 1 # elif 2 # else "
@@ -420,7 +420,7 @@
   )cpp",
R"(expanded tokens:
   int const a ;
-file './input.cpp'

[PATCH] D121955: [clang] NFC: Dead code removal in SemaDecl.cpp, CheckMultiVersionFunction().

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann published this revision for review.
tahonermann added inline comments.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.



Comment at: clang/lib/Sema/SemaDecl.cpp:10919-10921
   return CheckTargetCausesMultiVersioning(S, OldFD, NewFD, NewTA,
   Redeclaration, OldDecl,
   MergeTypeWithPrevious, Previous);

These lines are what makes the removed code dead code.


This change removes dead code in the definition of CheckMultiVersionFunction()
in clang/lib/Sema/SemaDecl.cpp. The removed code was made dead by commit
fc53eb69c26cdd7efa6b629c187d04326f0448ca 
: "Reapply 
'Implement target_clones multiversioning'".
See the added code just above the code being deleted; it contains the same
return statement with the previous condition now distributed across an if
statement and a switch statement.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121955

Files:
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10932,11 +10932,6 @@
   break;
 }
   }
-  // Handle the target potentially causes multiversioning case.
-  if (!OldFD->isMultiVersion() && MVType == MultiVersionKind::Target)
-return CheckTargetCausesMultiVersioning(S, OldFD, NewFD, NewTA,
-Redeclaration, OldDecl,
-MergeTypeWithPrevious, Previous);
 
   // At this point, we have a multiversion function decl (in OldFD) AND an
   // appropriate attribute in the current function decl.  Resolve that these 
are


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10932,11 +10932,6 @@
   break;
 }
   }
-  // Handle the target potentially causes multiversioning case.
-  if (!OldFD->isMultiVersion() && MVType == MultiVersionKind::Target)
-return CheckTargetCausesMultiVersioning(S, OldFD, NewFD, NewTA,
-Redeclaration, OldDecl,
-MergeTypeWithPrevious, Previous);
 
   // At this point, we have a multiversion function decl (in OldFD) AND an
   // appropriate attribute in the current function decl.  Resolve that these are
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121954: [clang] Add test cases for multiversion function overload scenarios in C.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
Herald added a project: All.
tahonermann added reviewers: erichkeane, aaron.ballman.
tahonermann published this revision for review.
tahonermann added inline comments.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.



Comment at: clang/test/Sema/attr-cpuspecific.c:2
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -Wno-strict-prototypes 
-fsyntax-only -verify %s -Wnonnull
+// XFAIL: asserts
 

This XFAIL line is removed with the changes in D121959.



Comment at: clang/test/Sema/attr-cpuspecific.c:123-125
+// FIXME: Declaration of a non-overloadable function when more than one
+// FIXME: multiversion function declarations are present results in an
+// FIXME: assertion failure.

These FIXME comments are removed with the changes in D121959.



Comment at: clang/test/Sema/attr-target-mv.c:2
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -Wno-strict-prototypes 
-fsyntax-only -verify %s
+// XFAIL: asserts
 

This XFAIL line is removed with the changes in D121959.



Comment at: clang/test/Sema/attr-target-mv.c:112-114
+// FIXME: Declaration of a non-overloadable function when more than one
+// FIXME: multiversion function declarations are present results in an
+// FIXME: assertion failure.

These FIXME comments are removed with the changes in D121959.


This change adds test cases to validate diagnostics for overloaded sets
that contain declarations of multiversion functions. Many of the added test
cases exercise declarations that are intended to be valid. Others are
intended to be valid if and when restrictions on multiversion functions
being declared with the overloadable attribute are lifted.

Several of the new test cases currently trigger the following assertion
failure in SemaDecl.cpp; the relevant test is therefore marked as an
expected failure pending a fix.

  Assertion `(Previous.empty() || llvm::any_of(Previous, [](const NamedDecl 
*ND) { return ND->hasAttr(); })) && "Non-redecls shouldn't happen without 
overloadable present"' failed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121954

Files:
  clang/test/Sema/attr-cpuspecific.c
  clang/test/Sema/attr-target-clones.c
  clang/test/Sema/attr-target-mv.c

Index: clang/test/Sema/attr-target-mv.c
===
--- clang/test/Sema/attr-target-mv.c
+++ clang/test/Sema/attr-target-mv.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -Wno-strict-prototypes -fsyntax-only -verify %s
+// XFAIL: asserts
 
 void __attribute__((target("sse4.2"))) no_default(void);
 void __attribute__((target("arch=sandybridge")))  no_default(void);
@@ -108,3 +109,60 @@
 void __attribute__((target("sse4.2"))) addtl_attrs6(int*);
 void __attribute__((target("arch=sandybridge"), nothrow, used, nonnull)) addtl_attrs6(int*);
 
+// FIXME: Declaration of a non-overloadable function when more than one
+// FIXME: multiversion function declarations are present results in an
+// FIXME: assertion failure.
+int __attribute__((target("sse4.2"))) bad_overload1(void);
+int __attribute__((target("arch=sandybridge"))) bad_overload1(void);
+// expected-error@+2 {{at most one overload for a given name may lack the 'overloadable' attribute}}
+// expected-note@-2 {{previous unmarked overload of function is here}}
+int bad_overload1(int);
+
+int bad_overload2(int);
+// expected-error@+2 {{conflicting types for 'bad_overload2'}}
+// expected-note@-2 {{previous declaration is here}}
+int __attribute__((target("sse4.2"))) bad_overload2(void);
+// expected-error@+2 {{conflicting types for 'bad_overload2'}}
+// expected-note@-5 {{previous declaration is here}}
+int __attribute__((target("arch=sandybridge"))) bad_overload2(void);
+
+// expected-error@+2 {{attribute 'target' multiversioning cannot be combined with attribute 'overloadable'}}
+// expected-note@+2 {{function multiversioning caused by this declaration}}
+int __attribute__((__overloadable__)) __attribute__((target("sse4.2"))) bad_overload3(void);
+int __attribute__((target("arch=sandybridge"))) bad_overload3(void);
+
+int __attribute__((target("sse4.2"))) bad_overload4(void);
+// expected-error@+1 {{attribute 'target' multiversioning cannot be combined with attribute 'overloadable'}}
+int __attribute__((__overloadable__)) __attribute__((target("arch=sandybridge"))) bad_overload4(void);
+
+int __attribute__((target("sse4.2"))) bad_overload5(void);
+int __attribute__((target("arch=sandybridge"))) bad_overload5(int);
+
+int __attribute__((target("sse4.2"))) good_overload1(void);
+int __attribute__((target("arch=sandybridge"))) good_overload1(void);
+int __attribute__((__overloadable__)) good_overload1(int);
+
+int __attribute__((__overloadable__)) good_overload2(int);
+int __attribute__((target("sse4.2"))) good_overload2(void);
+int __attribute__((target("arch=sandybridge"))) 

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

[Matt Arsenault via Phabricator via cfe-commits]

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

A spurious warning still isn't ideal but this is an improvement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

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

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

[Scott Linder via Phabricator via cfe-commits]

scott.linder added inline comments.



Comment at: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:567
 
-  if (auto HostcallFunction = M.getFunction("__ockl_hostcall_internal")) {
+  if (auto *HostcallFunction = M.getFunction("__ockl_hostcall_internal")) {
 for (auto  : HostcallFunction->uses()) {

I will land this separately, as it is unrelated and NFC



Comment at: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:570
   if (auto *CI = dyn_cast(U.getUser())) {
-M.getContext().emitError(
-CI, "Cannot use both printf and hostcall in the same module");
+M.getContext().diagnose(DiagnosticInfoInlineAsm(
+*CI, "Cannot use both printf and hostcall in the same module",

I'm also not certain this is the the best option to classify the diagnostic, 
but it is what was used for the error so I just left it as-is. Suggestions 
welcome!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

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

[PATCH] D120857: WIP [randstruct] Add randomize structure layout support

[Bill Wendling via Phabricator via cfe-commits]

void abandoned this revision.
void added a comment.

In D120857#3389022 , @aaron.ballman 
wrote:

> This seems like a duplicate of https://reviews.llvm.org/D121556 -- should 
> this review be abandoned?

Yes. Thanks :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120857

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

[clang] dd5895c - AMDGPU: Use the implicit kernargs for code object version 5

[Changpeng Fang via cfe-commits]

Author: Changpeng Fang
Date: 2022-03-17T14:12:36-07:00
New Revision: dd5895cc39864393f8ca357bc4e23e8d7b5b9723

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

LOG: AMDGPU: Use the implicit kernargs for code object version 5

Summary:
  Specifically, for trap handling, for targets that do not support 
getDoorbellID,
we load the queue_ptr from the implicit kernarg, and move queue_ptr to s[0:1].
To get aperture bases when targets do not have aperture registers, we load
private_base or shared_base directly from the implicit kernarg. In clang, we use
implicitarg_ptr + offsets to implement __builtin_amdgcn_workgroup_size_{xyz}.

Reviewers: arsenm, sameerds, yaxunl

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

Added: 

llvm/test/CodeGen/AMDGPU/GlobalISel/implicit-kernarg-backend-usage-global-isel.ll
llvm/test/CodeGen/AMDGPU/implicit-kernarg-backend-usage.ll

Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
llvm/lib/Target/AMDGPU/SIDefines.h
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
llvm/lib/Target/AMDGPU/SIISelLowering.h
llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 4ac7b6e79ff3e..39e88482db94d 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -16258,12 +16258,31 @@ Value *EmitAMDGPUDispatchPtr(CodeGenFunction ,
   return CGF.Builder.CreateAddrSpaceCast(Call, RetTy);
 }
 
+Value *EmitAMDGPUImplicitArgPtr(CodeGenFunction ) {
+  auto *F = CGF.CGM.getIntrinsic(Intrinsic::amdgcn_implicitarg_ptr);
+  auto *Call = CGF.Builder.CreateCall(F);
+  Call->addRetAttr(
+  Attribute::getWithDereferenceableBytes(Call->getContext(), 256));
+  Call->addRetAttr(Attribute::getWithAlignment(Call->getContext(), Align(8)));
+  return Call;
+}
+
 // \p Index is 0, 1, and 2 for x, y, and z dimension, respectively.
 Value *EmitAMDGPUWorkGroupSize(CodeGenFunction , unsigned Index) {
-  const unsigned XOffset = 4;
-  auto *DP = EmitAMDGPUDispatchPtr(CGF);
-  // Indexing the HSA kernel_dispatch_packet struct.
-  auto *Offset = llvm::ConstantInt::get(CGF.Int32Ty, XOffset + Index * 2);
+  bool IsCOV_5 = CGF.getTarget().getTargetOpts().CodeObjectVersion ==
+ clang::TargetOptions::COV_5;
+  Constant *Offset;
+  Value *DP;
+  if (IsCOV_5) {
+// Indexing the implicit kernarg segment.
+Offset = llvm::ConstantInt::get(CGF.Int32Ty, 12 + Index * 2);
+DP = EmitAMDGPUImplicitArgPtr(CGF);
+  } else {
+// Indexing the HSA kernel_dispatch_packet struct.
+Offset = llvm::ConstantInt::get(CGF.Int32Ty, 4 + Index * 2);
+DP = EmitAMDGPUDispatchPtr(CGF);
+  }
+
   auto *GEP = CGF.Builder.CreateGEP(CGF.Int8Ty, DP, Offset);
   auto *DstTy =
   CGF.Int16Ty->getPointerTo(GEP->getType()->getPointerAddressSpace());

diff  --git a/clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu 
b/clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
index 5928320b89f00..4c1c4c883a152 100644
--- a/clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
+++ b/clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
@@ -1,17 +1,31 @@
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa \
 // RUN: -fcuda-is-device -emit-llvm -o - -x hip %s \
-// RUN: | FileCheck %s
+// RUN: | FileCheck -check-prefix=PRECOV5 %s
+
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa \
+// RUN: -fcuda-is-device -mcode-object-version=5 -emit-llvm -o - -x hip %s 
\
+// RUN: | FileCheck -check-prefix=COV5 %s
 
 #include "Inputs/cuda.h"
 
-// CHECK-LABEL: test_get_workgroup_size
-// CHECK: call align 4 dereferenceable(64) i8 addrspace(4)* 
@llvm.amdgcn.dispatch.ptr()
-// CHECK: getelementptr i8, i8 addrspace(4)* %{{.*}}, i32 4
-// CHECK: load i16, i16 addrspace(4)* %{{.*}}, align 2, !range 
[[$WS_RANGE:![0-9]*]], !invariant.load
-// CHECK: getelementptr i8, i8 addrspace(4)* %{{.*}}, i32 6
-// CHECK: load i16, i16 addrspace(4)* %{{.*}}, align 2, !range 
[[$WS_RANGE:![0-9]*]], !invariant.load
-// CHECK: getelementptr i8, i8 addrspace(4)* %{{.*}}, i32 8
-// CHECK: load i16, i16 addrspace(4)* %{{.*}}, align 2, !range 
[[$WS_RANGE:![0-9]*]], !invariant.load
+// PRECOV5-LABEL: test_get_workgroup_size
+// PRECOV5: call align 4 dereferenceable(64) i8 addrspace(4)* 
@llvm.amdgcn.dispatch.ptr()
+// PRECOV5: getelementptr i8, i8 addrspace(4)* %{{.*}}, i32 4
+// PRECOV5: load i16, i16 addrspace(4)* %{{.*}}, align 2, !range 
[[$WS_RANGE:![0-9]*]], !invariant.load
+// PRECOV5: getelementptr i8, i8 

[PATCH] D120265: AMDGPU: Use the implicit kernargs for code object version 5

[Changpeng Fang 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 rGdd5895cc3986: AMDGPU: Use the implicit kernargs for code 
object version 5 (authored by cfang).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D120265?vs=416080=416315#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120265

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
  llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
  llvm/lib/Target/AMDGPU/SIDefines.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
  
llvm/test/CodeGen/AMDGPU/GlobalISel/implicit-kernarg-backend-usage-global-isel.ll
  llvm/test/CodeGen/AMDGPU/implicit-kernarg-backend-usage.ll

Index: llvm/test/CodeGen/AMDGPU/implicit-kernarg-backend-usage.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/implicit-kernarg-backend-usage.ll
@@ -0,0 +1,550 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx803 --amdhsa-code-object-version=3 < %s | FileCheck --check-prefix=GFX8V3 %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx803 --amdhsa-code-object-version=4 < %s | FileCheck --check-prefix=GFX8V4 %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx803 --amdhsa-code-object-version=5 < %s | FileCheck --check-prefix=GFX8V5 %s
+
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx906 --amdhsa-code-object-version=3 < %s | FileCheck --check-prefixes=GFX9V3 %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx906 --amdhsa-code-object-version=4 < %s | FileCheck --check-prefixes=GFX9V4 %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx906 --amdhsa-code-object-version=5 < %s | FileCheck --check-prefixes=GFX9V5 %s
+
+define amdgpu_kernel void @addrspacecast(i32 addrspace(5)* %ptr.private, i32 addrspace(3)* %ptr.local) {
+; GFX8V3-LABEL: addrspacecast:
+; GFX8V3:   ; %bb.0:
+; GFX8V3-NEXT:s_load_dwordx2 s[0:1], s[6:7], 0x0
+; GFX8V3-NEXT:s_load_dword s2, s[4:5], 0x44
+; GFX8V3-NEXT:s_load_dword s3, s[4:5], 0x40
+; GFX8V3-NEXT:v_mov_b32_e32 v4, 1
+; GFX8V3-NEXT:s_waitcnt lgkmcnt(0)
+; GFX8V3-NEXT:s_cmp_lg_u32 s0, -1
+; GFX8V3-NEXT:v_mov_b32_e32 v0, s2
+; GFX8V3-NEXT:s_cselect_b64 vcc, -1, 0
+; GFX8V3-NEXT:v_cndmask_b32_e32 v1, 0, v0, vcc
+; GFX8V3-NEXT:v_mov_b32_e32 v0, s0
+; GFX8V3-NEXT:s_cmp_lg_u32 s1, -1
+; GFX8V3-NEXT:v_cndmask_b32_e32 v0, 0, v0, vcc
+; GFX8V3-NEXT:v_mov_b32_e32 v2, s3
+; GFX8V3-NEXT:s_cselect_b64 vcc, -1, 0
+; GFX8V3-NEXT:v_cndmask_b32_e32 v3, 0, v2, vcc
+; GFX8V3-NEXT:v_mov_b32_e32 v2, s1
+; GFX8V3-NEXT:v_cndmask_b32_e32 v2, 0, v2, vcc
+; GFX8V3-NEXT:flat_store_dword v[0:1], v4
+; GFX8V3-NEXT:s_waitcnt vmcnt(0)
+; GFX8V3-NEXT:v_mov_b32_e32 v0, 2
+; GFX8V3-NEXT:flat_store_dword v[2:3], v0
+; GFX8V3-NEXT:s_waitcnt vmcnt(0)
+; GFX8V3-NEXT:s_endpgm
+;
+; GFX8V4-LABEL: addrspacecast:
+; GFX8V4:   ; %bb.0:
+; GFX8V4-NEXT:s_load_dwordx2 s[0:1], s[6:7], 0x0
+; GFX8V4-NEXT:s_load_dword s2, s[4:5], 0x44
+; GFX8V4-NEXT:s_load_dword s3, s[4:5], 0x40
+; GFX8V4-NEXT:v_mov_b32_e32 v4, 1
+; GFX8V4-NEXT:s_waitcnt lgkmcnt(0)
+; GFX8V4-NEXT:s_cmp_lg_u32 s0, -1
+; GFX8V4-NEXT:v_mov_b32_e32 v0, s2
+; GFX8V4-NEXT:s_cselect_b64 vcc, -1, 0
+; GFX8V4-NEXT:v_cndmask_b32_e32 v1, 0, v0, vcc
+; GFX8V4-NEXT:v_mov_b32_e32 v0, s0
+; GFX8V4-NEXT:s_cmp_lg_u32 s1, -1
+; GFX8V4-NEXT:v_cndmask_b32_e32 v0, 0, v0, vcc
+; GFX8V4-NEXT:v_mov_b32_e32 v2, s3
+; GFX8V4-NEXT:s_cselect_b64 vcc, -1, 0
+; GFX8V4-NEXT:v_cndmask_b32_e32 v3, 0, v2, vcc
+; GFX8V4-NEXT:v_mov_b32_e32 v2, s1
+; GFX8V4-NEXT:v_cndmask_b32_e32 v2, 0, v2, vcc
+; GFX8V4-NEXT:flat_store_dword v[0:1], v4
+; GFX8V4-NEXT:s_waitcnt vmcnt(0)
+; GFX8V4-NEXT:v_mov_b32_e32 v0, 2
+; GFX8V4-NEXT:flat_store_dword v[2:3], v0
+; GFX8V4-NEXT:s_waitcnt vmcnt(0)
+; GFX8V4-NEXT:s_endpgm
+;
+; GFX8V5-LABEL: addrspacecast:
+; GFX8V5:   ; %bb.0:
+; GFX8V5-NEXT:s_load_dwordx2 s[0:1], s[4:5], 0x0
+; GFX8V5-NEXT:s_load_dword s2, s[4:5], 0xc8
+; GFX8V5-NEXT:s_load_dword s3, s[4:5], 0xcc
+; GFX8V5-NEXT:v_mov_b32_e32 v4, 1
+; GFX8V5-NEXT:s_waitcnt lgkmcnt(0)
+; GFX8V5-NEXT:s_cmp_lg_u32 s0, -1
+; GFX8V5-NEXT:v_mov_b32_e32 v0, s2
+; GFX8V5-NEXT:s_cselect_b64 vcc, -1, 0
+; GFX8V5-NEXT:v_cndmask_b32_e32 v1, 0, v0, vcc
+; GFX8V5-NEXT:v_mov_b32_e32 v0, s0
+; 

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

[Scott Linder via Phabricator via cfe-commits]

scott.linder added reviewers: sameerds, arsenm, rampitec, b-sumner, kzhuravl, 
yaxunl.
scott.linder added a comment.

For reference, the error I'm changing to a warning was added as part of 
https://reviews.llvm.org/D70038, and the issue that made me consider this 
approach appears when building OCL conformance tests at -O0

Another approach would be to do some more work in Clang or early in the backend 
to strip out the unused device-libs functions which are calling 
`__ockl_hostcall_internal`. I started with that thought, but backed off as it 
seemed the less we could do at -O0 the better. The limitation is also being 
resolved in later HSA ABI versions, so this would be a legacy path almost as 
soon as we added it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

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

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

[Scott Linder via Phabricator via cfe-commits]

scott.linder created this revision.
Herald added subscribers: foad, kerbowa, hiraditya, t-tye, Anastasia, tpr, 
dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl, arsenm.
Herald added a project: All.
scott.linder requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wdng.
Herald added projects: clang, LLVM.

We are currently overly conservative, and reject modules which contain
dead uses of hostcall. This has the effect of preventing compilation at
-O0 of a use of printf in an otherwise empty module (i.e. one containing
no use of hostcall).

In a non-single-source world it is not possible to determine whether the
call is dead in the compiler, even if we wanted to.

Instead, make the error a warning, as the compiler cannot diagnose it
without some false positives, but it will remain an issue the user
should be aware of when producing pre-V5 HSA ABI code objects.

Expand testing to modules with printf+no-hostcall, printf+dead-hostcall,
and printf+hostcall to exercise the warning.

Add clang tests to tie the above tests to OpenCL source. Specifically,
the test llvm/test/CodeGen/AMDGPU/opencl-printf-dead-hostcall.ll
represents the case which previously failed at -O0.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121951

Files:
  clang/test/CodeGenOpenCL/amdgpu-printf.cl
  llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
  llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
  llvm/test/CodeGen/AMDGPU/opencl-printf-dead-hostcall.ll
  llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll

Index: llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
===
--- llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
+++ llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
@@ -1,18 +1,17 @@
-; RUN: not opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
+; RUN: opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
 
-@.str = private unnamed_addr addrspace(2) constant [6 x i8] c"%s:%d\00", align 1
+@.str = private unnamed_addr addrspace(4) constant [6 x i8] c"%s:%d\00", align 1
 
 define amdgpu_kernel void @test_kernel(i32 %n) {
 entry:
-  %str = alloca [9 x i8], align 1
-  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8]* %str, i32 0, i32 0
-  %call1 = call i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str, i32 0, i32 0), i8* %arraydecay, i32 %n)
-  %call2 = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
+  %str = alloca [9 x i8], align 1, addrspace(5)
+  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8] addrspace(5)* %str, i32 0, i32 0
+  %call = call i32 (i8 addrspace(4)*, ...) @printf(i8 addrspace(4)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(4)* @.str, i32 0, i32 0), i8 addrspace(5)* %arraydecay, i32 %n)
   ret void
 }
 
-declare i32 @printf(i8 addrspace(2)*, ...)
+declare i32 @printf(i8 addrspace(4)*, ...)
 
 declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
 
-; CHECK: error: Cannot use both printf and hostcall in the same module
+; CHECK-NOT: warning: Cannot use both printf and hostcall in the same module
Index: llvm/test/CodeGen/AMDGPU/opencl-printf-dead-hostcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/opencl-printf-dead-hostcall.ll
@@ -0,0 +1,22 @@
+; RUN: opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
+
+@.str = private unnamed_addr addrspace(4) constant [6 x i8] c"%s:%d\00", align 1
+
+define amdgpu_kernel void @test_kernel(i32 %n) {
+entry:
+  %str = alloca [9 x i8], align 1, addrspace(5)
+  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8] addrspace(5)* %str, i32 0, i32 0
+  %call = call i32 (i8 addrspace(4)*, ...) @printf(i8 addrspace(4)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(4)* @.str, i32 0, i32 0), i8 addrspace(5)* %arraydecay, i32 %n)
+  ret void
+}
+
+define amdgpu_kernel void @dead_function() {
+  %call = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
+  ret void
+}
+
+declare i32 @printf(i8 addrspace(4)*, ...)
+
+declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
+
+; CHECK: warning: Cannot use both printf and hostcall in the same module
Index: llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
@@ -0,0 +1,18 @@
+; RUN: opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | 

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

Let me explain a bit more =)

The optimizations from this patch are allowed by the Standard (always have been 
allowed). So there is no need to restrict it under a flag or C++ version.

@erichkeane your comment indeed would be applicable if I allowed NRVO for 
non-movable types, that is currently prohibited by the Standard, but allowed in 
the proposal. Luckily my patch doesn't do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119792

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

[PATCH] D121916: [clang-format] [doc] Add script to automatically update help output in ClangFormat.rst.

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

HazardyKnusperkeks added inline comments.



Comment at: clang/tools/clang-format/ClangFormat.cpp:105
+SortIncludes("sort-includes",
+ cl::desc("If set, overrides the include sorting behavior\n"
+  "determined by the SortIncludes style flag"),

Couldn't one split the string in python? I think an arbitrary position to split 
the help isn't nice. I for one have often the terminal over half the monitor 
spread, sometimes even the complete monitor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121916

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

[PATCH] D121907: [clang-format] Use an enum for context types.

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

HazardyKnusperkeks added a comment.

In D121907#3390226 , @MyDeveloperDay 
wrote:

> So out of interest, what is the reason? my assumption is that you wanted to 
> add more for Verilog and you felt adding the extra bools was the wrong design 
> and its better an an enum
>
>   bool InCpp11AttributeSpecifier = false;
>   bool InCSharpAttributeSpecifier = false;
>
> Does the fact that some aren't exclusive make you think its ok to split it 
> into enums and bools is ok?  (no real opinion just wondered what you and 
> others think?)

It is really helpful to see that these are exclusive. It helps reasoning about 
the code.




Comment at: clang/lib/Format/TokenAnnotator.cpp:1498
 bool CaretFound = false;
-bool IsForEachMacro = false;
+// These two are not handled like the rest in ContextType because they are
+// not exclusive.

This is a comment for the change, afterwards it's strange to see for newcomers. 
Since it's without `Context`. *scnr*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121907

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements

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

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:1431-1442
+TT_AttributeMacro, TT_BracedListLBrace, TT_ClassLBrace,
+TT_CompoundRequirementLBrace, TT_ConditionLParen, TT_EnumLBrace,
+TT_FatArrow, TT_ForEachMacro, TT_FunctionLBrace,
+TT_FunctionLikeOrFreestandingMacro, TT_IfMacro,
+TT_ImplicitStringLiteral, TT_InlineASMBrace, TT_LambdaArrow,
+TT_LambdaLBrace, TT_LambdaLSquare, TT_NamespaceMacro,
+TT_ObjCStringLiteral, TT_OverloadedOperator, TT_RecordLBrace,

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > Unrelated Change.
> Sorry.  This part has changed several times in the main line since I started 
> working on this patch.  After a few merge conflicts I got tired and started 
> running sort uniq on this part.
Nothing against that, but in a separate patch.
Although my goal is the get this list smaller again with the finalized type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D121753: [clang-format] Use a macro for non-C keywords

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:992-999
+  enum {
+ATTR_JS_KEYWORD = 0x1,
+ATTR_CSHARP_KEYWORD = 0x2,
+  };
+  unsigned getAttrs(const FormatToken ) const {
+auto At = KeywordAttr.find(Tok.Tok.getIdentifierInfo());
+return At == KeywordAttr.end() ? 0u : At->second;

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > No fan of DEFINE_LIKE_NAMES. More a fan of scoped::enums. ;)
> It looks like I would have to use the `KeywordLanguage::` prefix throughout 
> the macro list and also define an operator to combine categories for keywords 
> that several languages have.  Do you know of a simpler way?  With the current 
> enum, the DEFINE_LIKE_NAMES can't be used directly outside 
> `AdditionalKeywords`.
You can always drop the `class` and are back at integers.



Comment at: clang/lib/Format/FormatToken.h:992-999
+  enum {
+ATTR_JS_KEYWORD = 0x1,
+ATTR_CSHARP_KEYWORD = 0x2,
+  };
+  unsigned getAttrs(const FormatToken ) const {
+auto At = KeywordAttr.find(Tok.Tok.getIdentifierInfo());
+return At == KeywordAttr.end() ? 0u : At->second;

HazardyKnusperkeks wrote:
> sstwcw wrote:
> > HazardyKnusperkeks wrote:
> > > No fan of DEFINE_LIKE_NAMES. More a fan of scoped::enums. ;)
> > It looks like I would have to use the `KeywordLanguage::` prefix throughout 
> > the macro list and also define an operator to combine categories for 
> > keywords that several languages have.  Do you know of a simpler way?  With 
> > the current enum, the DEFINE_LIKE_NAMES can't be used directly outside 
> > `AdditionalKeywords`.
> You can always drop the `class` and are back at integers.
> With the current enum, the DEFINE_LIKE_NAMES can't be used directly outside 
> `AdditionalKeywords`.

This one I don't understand? They are public and can be used, can't they? And 
the type I can get with `decltype`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121753

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

[clang] b4cc3b1 - [OpenMP][FIX] Make metadata and attribute check lines less detailed

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2022-03-17T14:58:22-05:00
New Revision: b4cc3b1dd8d7200640210513263b28187f810703

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

LOG: [OpenMP][FIX] Make metadata and attribute check lines less detailed

The update_cc script should really do this automatically :(

Added: 


Modified: 
clang/test/OpenMP/amdgcn_target_global_constructor.cpp

Removed: 




diff  --git a/clang/test/OpenMP/amdgcn_target_global_constructor.cpp 
b/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
index 12d538e00ff76..544406dc7f43f 100644
--- a/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
+++ b/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
@@ -87,11 +87,11 @@ S A;
 // CHECK-NEXT:ret void
 //
 //.
-// CHECK: attributes #0 = { convergent noinline nounwind 
"frame-pointer"="none" "kernel" "min-legal-vector-width"="0" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" }
-// CHECK: attributes #1 = { convergent noinline nounwind optnone 
"frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" }
-// CHECK: attributes #2 = { convergent "frame-pointer"="none" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" }
-// CHECK: attributes #3 = { convergent }
-// CHECK: attributes #4 = { convergent nounwind }
+// CHECK: attributes #0 = { convergent
+// CHECK: attributes #1 = { convergent
+// CHECK: attributes #2 = { convergent
+// CHECK: attributes #3 = { convergent
+// CHECK: attributes #4 = { convergent
 //.
 // CHECK: !0 = !{i32 0, {{.*}}, !"__omp_offloading_{{.*}}_ctor", i32 19, i32 1}
 // CHECK: !1 = !{i32 0, {{.*}}, !"__omp_offloading_{{.*}}_dtor", i32 19, i32 2}
@@ -101,5 +101,5 @@ S A;
 // CHECK: !5 = !{i32 1, !"wchar_size", i32 4}
 // CHECK: !6 = !{i32 7, !"openmp", i32 50}
 // CHECK: !7 = !{i32 7, !"openmp-device", i32 50}
-// CHECK: !8 = !{!"clang version 15.0.0"}
+// CHECK: !8 = !{!"clang version
 //.



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

[PATCH] D121749: [clang-format][docs] Regenerate ClangFormatStyleOptions.rst

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: Eugene.Zelenko.
MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:361
 
-  * ``bool AlignCompound`` Only for ``AlignConsecutiveAssignments``.  Whether 
compound
-assignments like ``+=`` are aligned along with ``=``.
+  * ``bool AlignCompound`` Only for ``AlignConsecutiveAssignments``.  Whether 
compound assignments
+like ``+=`` are aligned along with ``=``.

owenpan wrote:
> curdeius wrote:
> > curdeius wrote:
> > > sstwcw wrote:
> > > > MyDeveloperDay wrote:
> > > > > Is this wider than 80 chars?
> > > > The comment in the source code is within 80 columns.  The python script 
> > > > prepended the option name.  It's the same case as line 1541.
> > > It is indeed. But it was already longer than 80 chars.
> > > I'll have a look at the script to see how to fix this.
> > At the same time, does it really matter if it's auto-generated?
> It should not matter, especially because it’s also a non-source file.
I seem to remember once before I made a change to clang-tidy and they liked the 
rst to be no wider than 80 chars, but that might have been hand generated rst.  
If I remember rightly @Eugene.Zelenko picked me up on it, So I actually 
developed a script (which never landed) that checks the rst for me.  {D55523}

I still use this from time to time and try and clean up.

```
Checking docs/ClangFormatStyleOptions.rst...
warning: line 9 title and markup non matching lengths
warning: line 21 multiple blank lines
warning: line 35 maximize 80 characters by joining:'[When using 
``-style=file:``, :program:`clang-format` for]' and '[each...]

warning: line 36 maximize 80 characters by joining:'[each input file will use 
the format file located at ``.]' and '[The...]

warning: line 103 multiple blank lines
warning: line 121 multiple blank lines
warning: line 130 multiple blank lines
warning: line 139 multiple blank lines
warning: line 181 maximize 80 characters by joining:'[**AccessModifierOffset** 
(``Integer``) :versionbadge:`clang-format 3.3`]' and '[The...]

warning: line 184 is in excess of 80 characters (87) : 
**AlignAfterOpenBracket** (``BracketAlignmentStyle``) 
:versionbadge:`clang-format 3.8`
...
warning: line 228 multiple blank lines
warning: line 229 trailing whitespace
warning: line 233 multiple blank lines
warning: line 234 multiple blank lines
warning: line 235 is in excess of 80 characters (96) : 
**AlignArrayOfStructures** (``ArrayInitializerAlignmentStyle``) 
:versionbadge:`clang-format 13`
...
warning: line 236 maximize 80 characters by joining:'[  if not ``None``, when 
using initialization for an array of structs]' and '[aligns...]

warning: line 251 contains double spaces
warning: line 253 contains double spaces
warning: line 263 contains double spaces
warning: line 265 contains double spaces
warning: line 271 multiple blank lines
warning: line 272 multiple blank lines
warning: line 273 is in excess of 80 characters (93) : 
**AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 3.8`
...
warning: line 280 contains double spaces
warning: line 282 contains double spaces
warning: line 295 maximize 80 characters by joining:'[  For example, to align 
across empty lines and not across comments, either]' and '[of...]

warning: line 311 contains double spaces
warning: line 312 contains double spaces
warning: line 314 contains double spaces
warning: line 315 contains double spaces
warning: line 317 contains double spaces
warning: line 319 contains double spaces
warning: line 322 contains double spaces
warning: line 323 contains double spaces
warning: line 325 contains double spaces
warning: line 326 contains double spaces
warning: line 334 contains double spaces
warning: line 336 contains double spaces
warning: line 338 contains double spaces
warning: line 341 contains double spaces
warning: line 343 contains double spaces
warning: line 352 contains double spaces
warning: line 361 contains double spaces
warning: line 367 contains double spaces
warning: line 368 contains double spaces
warning: line 374 contains double spaces
warning: line 381 contains double spaces
warning: line 382 contains double spaces
warning: line 384 contains double spaces
warning: line 391 contains double spaces
warning: line 394 multiple blank lines
warning: line 395 is in excess of 80 characters (90) : 
**AlignConsecutiveBitFields** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 11`
...
warning: line 398 maximize 80 characters by joining:'[  ``Consecutive`` will 
align the bitfield separators of consecutive lines.]' and '[This...]

warning: line 404 contains double spaces
warning: line 405 contains double spaces
warning: line 418 maximize 80 characters by joining:'[  For example, to align 
across empty lines and not across comments, either]' and '[of...]

warning: line 434 contains double spaces
warning: line 435 contains double spaces
warning: line 437 contains double 

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

[Joseph Huber via Phabricator via cfe-commits]

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

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cuda-openmp-driver.cu

Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -0,0 +1,16 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings -fgpu-rdc \
+// RUN:-foffload-new-driver --offload-arch=sm_35 --offload-arch=sm_70 %s 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK %s
+
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[PTX_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_35]]"], output: "[[CUBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_35]]", "[[PTX_SM_35]]"], output: "[[FATBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]"], output: "[[PTX_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_70:.+]]"], output: "[[CUBIN_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_70]]", "[[PTX_SM_70:.+]]"], output: "[[FATBIN_SM_70:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT]]", "[[FATBIN_SM_35]]", "[[FATBIN_SM_70]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4391,6 +4391,7 @@
   // one input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
   bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
+  bool IsCudaHost = JA.isHostOffloading(Action::OFK_Cuda);
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
@@ -4414,6 +4415,7 @@
 
   InputInfoList ModuleHeaderInputs;
   InputInfoList OpenMPHostInputs;
+  InputInfoList CudaHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
   for (const InputInfo  : Inputs) {
@@ -4428,6 +4430,9 @@
 << types::getTypeName(Expected);
   }
   ModuleHeaderInputs.push_back(I);
+} else if (IsCudaHost && Args.hasArg(options::OPT_fopenmp_new_driver)) {
+  CudaDeviceInput = 
+  CudaHostInputs.push_back(I);
 } else if ((IsCuda || IsHIP) && !CudaDeviceInput) {
   CudaDeviceInput = 
 } else if (IsOpenMPDevice && !OpenMPDeviceInput) {
@@ -6982,6 +6987,19 @@
   TC->getTripleString() + "." +
   TCArgs.getLastArgValue(options::OPT_march_EQ) + "." + InputName));
 }
+  } else if (IsCudaHost && !CudaHostInputs.empty()) {
+const ToolChain *TC = C.getSingleOffloadToolChain();
+for (const auto  : CudaHostInputs) {
+  assert(InputFile.isFilename() && "Offloading requires a filename");
+  StringRef File =
+  C.getArgs().MakeArgString(TC->getInputFilename(InputFile));
+  StringRef InputName = Clang::getBaseInputStem(Args, Inputs);
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fembed-offload-object=" + File + "," +
+  Action::GetOffloadKindName(Action::OFK_Cuda) + "." +
+  TC->getTripleString() + "." +
+  InputFile.getAction()->getOffloadingArch() + "." + InputName));
+}
   }
 
   if (Triple.isAMDGPU()) {
@@ -8234,14 +8252,17 @@
   ArgStringList CmdArgs;
 
   // Pass the CUDA path to the linker wrapper tool.
-  for (auto  : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
-const ToolChain *TC = I.second;
-if (TC->getTriple().isNVPTX()) {
-  CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
-  if (CudaInstallation.isValid())
-CmdArgs.push_back(Args.MakeArgString(
-"--cuda-path=" + CudaInstallation.getInstallPath()));
-  break;
+  for (Action::OffloadKind Kind : {Action::OFK_Cuda, Action::OFK_OpenMP}) {
+auto TCRange = C.getOffloadToolChains(Kind);
+for (auto  : llvm::make_range(TCRange.first, TCRange.second)) {
+  const ToolChain *TC = I.second;
+  if (TC->getTriple().isNVPTX()) {
+CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
+if (CudaInstallation.isValid())
+  CmdArgs.push_back(Args.MakeArgString(
+  "--cuda-path=" + CudaInstallation.getInstallPath()));
+break;
+  }
 

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2138
+
+// These keywords are deliberately not included here. They are either
+// included in determineStarAmpUsage or determinePlusMinusCaretUsage.

I'm not sure about this. Why not handle them here too?



Comment at: clang/lib/Format/TokenAnnotator.cpp:2146-2147
+//   know how they can be followed by a star or amp.
+// co_await, delete - It doesn't make sense to have them followed by a 
unary
+//   `+` or `-`.
+if (PrevToken->isOneOf(TT_ConditionalExpr, tok::l_paren, tok::comma,

Especially here, why should a `+` after `delete` be a binary operator?
How much sense it makes doesn't matter, it is valid c++: 
https://gcc.godbolt.org/z/c1x1nn3Ej



Comment at: clang/unittests/Format/FormatTest.cpp:9758
+  verifyFormat("for (x = 0; -10 < x; --x) {\n}");
+  verifyFormat("sizeof -x");
+  verifyFormat("sizeof +x");

A format test is fine, a token annotator test would be better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121754

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

[PATCH] D121907: [clang-format] Use an enum for context types.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

So out of interest, what is the reason? my assumption is that you wanted to add 
more for Verilog and you felt adding the extra bools was the wrong design and 
its better an an enum

  bool InCpp11AttributeSpecifier = false;
  bool InCSharpAttributeSpecifier = false;

Does the fact that some aren't exclusive make you think its ok to split it into 
enums and bools is ok?  (no real opinion just wondered what you and others 
think?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121907

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

[PATCH] D121890: [clang-format] Copy help options to the doc directory.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D121890#3389171 , @sstwcw wrote:

> The issue is #54418.
>
> Actually I don't know who I should add as reviewers.  The patches for 
> clang-format don't always seem to have the same reviewers.  Who are the main 
> developers and how do you decide who you add each time?
>
> I give up making the doc generated.  It would require either integrating into 
> the build process or writing a parser script like for the style options.  
> Besides, that file is not entirely generated.

ok neither is ClangFormatStyleOptions.rts, but also the 
doc/tools/dump_format_style.py is also something thats not part of the build, 
but just a tool we can run to ensure the Format.h and 
ClangFormatStryleOptions.rst stay in lock step.

So actually I think that's a good task for someone to do, but its not necessary 
to do it here, but actually I think I'd prefer to see that than having to keep 
this the same by hand. I clearly missed that step. (so I appreciate you adding 
it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121890

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

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/include/clang/Basic/Cuda.h:105-107
+constexpr CudaArch DefaultCudaArch = CudaArch::SM_35;
+constexpr CudaArch DefaultHIPArch = CudaArch::GFX803;
+

tra wrote:
> Nit: those could be just enum values.
> ```
>   ...
>   LAST,
>   DefaultCudaArch = SM_35,
>   DefaultHIPArch = GFX803,
> };
> ```
Will do.



Comment at: clang/lib/Driver/Driver.cpp:4219
+for (unsigned I = 0, E = TCAndArchs.size(); I != E; ++I)
   DeviceActions.push_back(C.MakeAction(*InputArg, InputType));
 

tra wrote:
> This loop can be folded into the loop above.
> 
> Alternatively, for simple loops like this one could use `llvm::for_each`.
It could, I think the intention is clearer (i.e. make an input for every 
toolchain and architecture) having them separate. But I can merge them if 
that's better.



Comment at: clang/lib/Driver/Driver.cpp:4244
   A = C.MakeAction(HDep, DDep);
+  ++TCAndArch;
+} else if (isa(A) && Kind == Action::OFK_Cuda) {

tra wrote:
> Could you elaborate why TCAndArch is incremented only here? Don't other cases 
> need to advance it, too? At the very least we need some comments here and for 
> the loop in general, describing what's going on.
It shouldn't be, I forgot to move this out of the conditional once I added more 
things, I'll explain the usage as well.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6998
+  // The CUDA toolchain should have a bound arch appended to the filename.
+  StringRef Arch = File.rsplit(".").first.rsplit('-').second;
+  CmdArgs.push_back(Args.MakeArgString(

tra wrote:
> Extracting arch name from the file name looks... questionable. Where does 
> that file name come from? Can't we get this information directly?
Yeah, it's not ideal but it was the easiest way to do it. The alternative is to 
find a way to traverse the job action list and find the nodes with a bound 
architecture set and hope they're in the same order. I can try to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

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

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

[Yuanfang Chen via Phabricator via cfe-commits]

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

gentle ping..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115844

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

[PATCH] D121283: [Clang] Support multiple attributes in a single pragma

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan updated this revision to Diff 416279.
egorzhdan added a comment.

- Fix off-by-one error in fix-it generation logic: the last SubjectMatchRule

(`SubjectMatchRule_variable_not_is_parameter`) was not handled properly

- Add a test for this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121283

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/AttrSubjectMatchRules.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/test/AST/pragma-multiple-attributes-declspec.cpp
  clang/test/AST/pragma-multiple-attributes.cpp
  clang/test/FixIt/fixit-pragma-attribute.c
  clang/test/FixIt/fixit-pragma-attribute.cpp
  clang/test/Parser/pragma-attribute-declspec.cpp
  clang/test/Parser/pragma-attribute.cpp

Index: clang/test/Parser/pragma-attribute.cpp
===
--- clang/test/Parser/pragma-attribute.cpp
+++ clang/test/Parser/pragma-attribute.cpp
@@ -154,9 +154,6 @@
 #pragma clang attribute push ([[gnu::abi_tag]], apply_to=any(function))
 #pragma clang attribute pop
 
-#pragma clang attribute push ([[clang::disable_tail_calls, noreturn]], apply_to = function) // expected-error {{more than one attribute specified in '#pragma clang attribute push'}}
-#pragma clang attribute push ([[clang::disable_tail_calls, noreturn]]) // expected-error {{more than one attribute specified in '#pragma clang attribute push'}}
-
 #pragma clang attribute push ([[gnu::abi_tag]], apply_to=namespace)
 #pragma clang attribute pop
 
@@ -210,3 +207,13 @@
 #pragma clang attribute push([[clang::uninitialized]], apply_to=any) // expected-error {{expected '('}}
 #pragma clang attribute push([[clang::uninitialized]], apply_to = any()) // expected-error {{expected an identifier that corresponds to an attribute subject rule}}
 // NB: neither of these need to be popped; they were never successfully pushed.
+
+#pragma clang attribute push ([[clang::disable_tail_calls, noreturn]], apply_to = function)
+#pragma clang attribute pop
+
+#pragma clang attribute push (__attribute__((disable_tail_calls, annotate("test"))), apply_to = function)
+#pragma clang attribute pop
+
+#pragma clang attribute push (__attribute__((disable_tail_calls,)), apply_to = function) // expected-error {{expected identifier that represents an attribute name}}
+
+#pragma clang attribute push ([[clang::disable_tail_calls, noreturn]]) // expected-error {{expected ','}}
Index: clang/test/Parser/pragma-attribute-declspec.cpp
===
--- clang/test/Parser/pragma-attribute-declspec.cpp
+++ clang/test/Parser/pragma-attribute-declspec.cpp
@@ -6,7 +6,8 @@
 
 #pragma clang attribute pop
 
-#pragma clang attribute push(__declspec(dllexport, dllimport), apply_to = function) // expected-error {{more than one attribute specified in '#pragma clang attribute push'}}
+#pragma clang attribute push(__declspec(dllexport, dllimport), apply_to = function)
+#pragma clang attribute pop
 
 #pragma clang attribute push(__declspec(align), apply_to = variable) // expected-error {{attribute 'align' is not supported by '#pragma clang attribute'}}
 
Index: clang/test/FixIt/fixit-pragma-attribute.cpp
===
--- clang/test/FixIt/fixit-pragma-attribute.cpp
+++ clang/test/FixIt/fixit-pragma-attribute.cpp
@@ -39,7 +39,7 @@
 #pragma clang attribute pop
 
 #pragma clang attribute push (__attribute__((abi_tag("a"
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:60}:", apply_to = any(record(unless(is_union)), variable, function, namespace)"
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:60}:", apply_to = any(function, namespace, record(unless(is_union)), variable)"
 #pragma clang attribute push (__attribute__((abi_tag("a"))) apply_to=function)
 // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:60}:", "
 #pragma clang attribute push (__attribute__((abi_tag("a"))) = function)
@@ -48,36 +48,39 @@
 // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:60}:", apply_to = "
 
 #pragma clang attribute push (__attribute__((abi_tag("a"))) 22)
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:63}:", apply_to = any(record(unless(is_union)), variable, function, namespace)"
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:63}:", apply_to = any(function, namespace, record(unless(is_union)), variable)"
 #pragma clang attribute push (__attribute__((abi_tag("a"))) function)
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:69}:", apply_to = any(record(unless(is_union)), variable, function, namespace)"
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:60-[[@LINE-1]]:69}:", apply_to = any(function, namespace, record(unless(is_union)), variable)"
 #pragma clang attribute push (__attribute__((abi_tag("a"))) (function))
-// CHECK: 

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D121837#3390042 , @thakis wrote:

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

I relaxed the check line. Should resolve the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121837

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

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

I think I've got all the changes incorporated, but I'm getting a test failure 
so I haven't uploaded a new diff.

Honestly, I don't understand the test failure output.  On Windows, the test 
failure output is all mangled and
difficult to interpret correctly as to what exactly is the problem, so I'm 
still trying to figure it out.

Improving the readability of the test failures is one of the things I would 
like to address in a future change.
I suspect it's only a problem on Windows as it seems most of the clang-tidy 
devs are using unix?


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

https://reviews.llvm.org/D117522

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

[clang] 052a6c7 - [OpenMP][FIX] Relax test check lines

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2022-03-17T14:01:47-05:00
New Revision: 052a6c744af542bfd9d7faad5de72caf9f3eb6a7

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

LOG: [OpenMP][FIX] Relax test check lines

Added: 


Modified: 
clang/test/OpenMP/amdgcn_target_global_constructor.cpp

Removed: 




diff  --git a/clang/test/OpenMP/amdgcn_target_global_constructor.cpp 
b/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
index 72d6f177ef7b7..12d538e00ff76 100644
--- a/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
+++ b/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
@@ -93,8 +93,8 @@ S A;
 // CHECK: attributes #3 = { convergent }
 // CHECK: attributes #4 = { convergent nounwind }
 //.
-// CHECK: !0 = !{i32 0, i32 42, i32 18149483, !"__omp_offloading_{{.*}}_ctor", 
i32 19, i32 1}
-// CHECK: !1 = !{i32 0, i32 42, i32 18149483, !"__omp_offloading_{{.*}}_dtor", 
i32 19, i32 2}
+// CHECK: !0 = !{i32 0, {{.*}}, !"__omp_offloading_{{.*}}_ctor", i32 19, i32 1}
+// CHECK: !1 = !{i32 0, {{.*}}, !"__omp_offloading_{{.*}}_dtor", i32 19, i32 2}
 // CHECK: !2 = !{i32 1, !"A", i32 0, i32 0}
 // CHECK: !3 = !{void ()* @__omp_offloading_{{.*}}_ctor, !"kernel", i32 1}
 // CHECK: !4 = !{void ()* @__omp_offloading_{{.*}}_dtor, !"kernel", i32 1}



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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

If people thing it would be preferable to add a new phase and change phase 
scheduling a little I am happy to do that too!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

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

[clang] bbf0d19 - Currently the control of the eval-method is mixed with fast-math.

[Zahira Ammarguellat via cfe-commits]

Author: Zahira Ammarguellat
Date: 2022-03-17T11:48:03-07:00
New Revision: bbf0d1932a3c1be970ed8a580e51edf571b80fd5

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

LOG: Currently the control of the eval-method is mixed with fast-math.
FLT_EVAL_METHOD tells the user the precision at which, temporary results
are evaluated but when fast-math is enabled, the numeric values are not
guaranteed to match the source semantics, so the eval-method is
meaningless.
For example, the expression `x + y + z` has as source semantics `(x + y)
+ z`. FLT_EVAL_METHOD is telling the user at which precision `(x + y)`
is evaluated. With fast-math enable the compiler can choose to
evaluate the expression as `(y + z) + x`.
The correct behavior is to set the FLT_EVAL_METHOD to `-1` to tell the
user that the precision of the intermediate values is unknow. This
patch is doing that.

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

Added: 
clang/test/CodeGen/eval-method-fast-math.cpp

Modified: 
clang/include/clang/Lex/Preprocessor.h
clang/lib/Lex/PPMacroExpansion.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaAttr.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/Preprocessor.h 
b/clang/include/clang/Lex/Preprocessor.h
index 4eb96f1cac4b7..36bf8ed64c2bc 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -192,6 +192,11 @@ class Preprocessor {
   LangOptions::FPEvalMethodKind CurrentFPEvalMethod =
   LangOptions::FPEvalMethodKind::FEM_UnsetOnCommandLine;
 
+  // Keeps the value of the last evaluation method before a
+  // `pragma float_control (precise,off) is applied.
+  LangOptions::FPEvalMethodKind LastFPEvalMethod =
+  LangOptions::FPEvalMethodKind::FEM_UnsetOnCommandLine;
+
   // The most recent pragma location where the floating point evaluation
   // method was modified. This is used to determine whether the
   // 'pragma clang fp eval_method' was used whithin the current scope.
@@ -2078,6 +2083,14 @@ class Preprocessor {
 return LastFPEvalPragmaLocation;
   }
 
+  LangOptions::FPEvalMethodKind getLastFPEvalMethod() const {
+return LastFPEvalMethod;
+  }
+
+  void setLastFPEvalMethod(LangOptions::FPEvalMethodKind Val) {
+LastFPEvalMethod = Val;
+  }
+
   void setCurrentFPEvalMethod(SourceLocation PragmaLoc,
   LangOptions::FPEvalMethodKind Val) {
 assert(Val != LangOptions::FEM_UnsetOnCommandLine &&

diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp 
b/clang/lib/Lex/PPMacroExpansion.cpp
index 82fc57c8f2e88..c2b7c194a2ba3 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1577,14 +1577,35 @@ void Preprocessor::ExpandBuiltinMacro(Token ) {
 Tok.setKind(tok::string_literal);
   } else if (II == Ident__FLT_EVAL_METHOD__) {
 // __FLT_EVAL_METHOD__ is set to the default value.
-OS << getTUFPEvalMethod();
-// __FLT_EVAL_METHOD__ expands to a simple numeric value.
-Tok.setKind(tok::numeric_constant);
-if (getLastFPEvalPragmaLocation().isValid()) {
-  // The program is ill-formed. The value of __FLT_EVAL_METHOD__ is altered
-  // by the pragma.
-  Diag(Tok, diag::err_illegal_use_of_flt_eval_macro);
-  Diag(getLastFPEvalPragmaLocation(), diag::note_pragma_entered_here);
+if (getTUFPEvalMethod() ==
+LangOptions::FPEvalMethodKind::FEM_Indeterminable) {
+  // This is possible if `AllowFPReassoc` or `AllowReciprocal` is enabled.
+  // These modes can be triggered via the command line option `-ffast-math`
+  // or via a `pragam float_control`.
+  // __FLT_EVAL_METHOD__ expands to -1.
+  // The `minus` operator is the next token we read from the stream.
+  auto Toks = std::make_unique(1);
+  OS << "-";
+  Tok.setKind(tok::minus);
+  // Push the token `1` to the stream.
+  Token NumberToken;
+  NumberToken.startToken();
+  NumberToken.setKind(tok::numeric_constant);
+  NumberToken.setLiteralData("1");
+  NumberToken.setLength(1);
+  Toks[0] = NumberToken;
+  EnterTokenStream(std::move(Toks), 1, /*DisableMacroExpansion*/ false,
+   /*IsReinject*/ false);
+} else {
+  OS << getTUFPEvalMethod();
+  // __FLT_EVAL_METHOD__ expands to a simple numeric value.
+  Tok.setKind(tok::numeric_constant);
+  if (getLastFPEvalPragmaLocation().isValid()) {
+// The program is ill-formed. The value of __FLT_EVAL_METHOD__ is
+// altered by the pragma.
+Diag(Tok, diag::err_illegal_use_of_flt_eval_macro);
+Diag(getLastFPEvalPragmaLocation(), diag::note_pragma_entered_here);
+  }
 }
   } else if (II == Ident__COUNTER__) {
 // __COUNTER__ expands to a simple numeric value.

diff 

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

[Zahira Ammarguellat via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbf0d1932a3c: Currently the control of the eval-method is 
mixed with fast-math. (authored by zahiraam).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121122

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/eval-method-fast-math.cpp

Index: clang/test/CodeGen/eval-method-fast-math.cpp
===
--- /dev/null
+++ clang/test/CodeGen/eval-method-fast-math.cpp
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -triple x86_64-linux-gnu -emit-llvm -o - %s  \
+// RUN: | FileCheck %s -check-prefixes=CHECK
+
+// RUN: %clang_cc1 -triple i386--linux -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-EXT
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -mreassociate -freciprocal-math -ffp-contract=fast \
+// RUN: -ffast-math -triple x86_64-linux-gnu \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+// RUN: %clang_cc1 -triple i386--linux -mreassociate -freciprocal-math \
+// RUN: -ffp-contract=fast -ffast-math -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+float a = 1.0f, b = 2.0f, c = 3.0f;
+#pragma float_control(precise, off)
+float res2 = a + b + c;
+int val3 = __FLT_EVAL_METHOD__;
+#pragma float_control(precise, on)
+float res3 = a + b + c;
+int val4 = __FLT_EVAL_METHOD__;
+
+// CHECK: @val3 = global i32 -1
+// CHECK: @val4 = global i32 0
+
+// CHECK-EXT: @val3 = global i32 -1
+// CHECK-EXT: @val4 = global i32 2
+
+// CHECK-FAST: @val3 = global i32 -1
+// CHECK-FAST: @val4 = global i32 -1
+
+float res;
+int add(float a, float b, float c) {
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+int add_precise(float a, float b, float c) {
+#pragma float_control(precise, on)
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+#pragma float_control(push)
+#pragma float_control(precise, on)
+int add_precise_1(float a, float b, float c) {
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+#pragma float_control(pop)
+
+int add_not_precise(float a, float b, float c) {
+  // Fast-math is enabled with this pragma.
+#pragma float_control(precise, off)
+  // CHECK: fadd fast float
+  // CHECK: load float, float*
+  // CHECK: fadd fast float
+  // CHECK: float {{.*}}, float*
+  // CHECK: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+#pragma float_control(push)
+// Fast-math is enabled with this pragma.
+#pragma float_control(precise, off)
+int add_not_precise_1(float a, float b, float c) {
+  // CHECK: fadd fast float
+  // CHECK: load float, float*
+  // CHECK: fadd fast float
+  // CHECK: float {{.*}}, float*
+  // CHECK: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+#pragma float_control(pop)
+
+int getFPEvalMethod() {
+  // CHECK: ret i32 0
+  return __FLT_EVAL_METHOD__;
+}
+
+float res1;
+int whatever(float a, float b, float c) {
+#pragma float_control(precise, off)
+  // CHECK: load float, float*
+  // CHECK: fadd fast float
+  // CHECK: store float {{.*}}, float*
+  // CHECK: store i32 -1
+  // CHECK: store i32 0
+  // CHECK: ret i32 -1
+  res1 = a + b + c;
+  int val1 = __FLT_EVAL_METHOD__;
+  {
+#pragma float_control(precise, on)
+int val2 = __FLT_EVAL_METHOD__;
+  }
+  return __FLT_EVAL_METHOD__;
+}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -508,6 +508,13 @@
   case PFC_Precise:
 NewFPFeatures.setFPPreciseEnabled(true);
 FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
+if (PP.getCurrentFPEvalMethod() ==
+LangOptions::FPEvalMethodKind::FEM_Indeterminable &&
+PP.getLastFPEvalPragmaLocation().isValid())
+  // A preceding `pragma float_control(precise,off)` has changed
+  // the value of the evaluation method.
+  // Set it back to its old value.
+  PP.setCurrentFPEvalMethod(SourceLocation(), PP.getLastFPEvalMethod());
 break;
   case PFC_NoPrecise:
 if (CurFPFeatures.getFPExceptionMode() == LangOptions::FPE_Strict)
@@ -517,6 +524,10 @@
 else
   NewFPFeatures.setFPPreciseEnabled(false);
 FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
+PP.setLastFPEvalMethod(PP.getCurrentFPEvalMethod());

[PATCH] D121873: [clang][extract-api] Add enum support

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/SymbolGraph/ExtractAPIConsumer.cpp:197
 
+  void recordEnumConstants(EnumRecord *EnumRecord,
+   const EnumDecl::enumerator_range Constants) {

Should this be static or in an anonymous namespace?



Comment at: clang/lib/SymbolGraph/Serialization.cpp:228
   const LangOptions ) {
+  auto AddLangPrefix = [](StringRef S) -> std::string {
+return (getLanguageName(LangOpts) + "." + S).str();

Nice!



Comment at: clang/lib/SymbolGraph/Serialization.cpp:253
+Kind["identifier"] = AddLangPrefix("enum.case");
+Kind["displayName"] = "Enum Case";
+break;

"Enumeration Case"



Comment at: clang/lib/SymbolGraph/Serialization.cpp:257
+Kind["identifier"] = AddLangPrefix("enum");
+Kind["displayName"] = "Enum";
+break;

"Enumeration"



Comment at: clang/lib/SymbolGraph/Serialization.cpp:321
+  case RelationshipKind::MemberOf:
+Relationship["kind"] = "memberOf";
+break;

Since we are going to add more cases to this switch, would you mind doing 
something along the lines of:
```
const char *KindString = "";
switch(Kind) {
  default:
 llvm_unreachable("Unhandled relationship kind in Symbol Graph!");
 break;
  case RelationshipKind::MemberOf: 
KindString = "memberOf";
break;
}

Relationship["kind"] = KindString
```
or a method in the serializer for getting the string representation of the 
relationship kind?



Comment at: clang/lib/SymbolGraph/Serialization.cpp:343
+  if (!Enum)
+return;
+

Quick design question: Do we want to be silently failing in these situations 
(especially since this shouldn't be happening)? Let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121873

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

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests: http://45.33.8.238/linux/71358/step_7.txt

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121837

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

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Basic/Cuda.h:105-107
+constexpr CudaArch DefaultCudaArch = CudaArch::SM_35;
+constexpr CudaArch DefaultHIPArch = CudaArch::GFX803;
+

Nit: those could be just enum values.
```
  ...
  LAST,
  DefaultCudaArch = SM_35,
  DefaultHIPArch = GFX803,
};
```



Comment at: clang/lib/Driver/Driver.cpp:4219
+for (unsigned I = 0, E = TCAndArchs.size(); I != E; ++I)
   DeviceActions.push_back(C.MakeAction(*InputArg, InputType));
 

This loop can be folded into the loop above.

Alternatively, for simple loops like this one could use `llvm::for_each`.



Comment at: clang/lib/Driver/Driver.cpp:4244
   A = C.MakeAction(HDep, DDep);
+  ++TCAndArch;
+} else if (isa(A) && Kind == Action::OFK_Cuda) {

Could you elaborate why TCAndArch is incremented only here? Don't other cases 
need to advance it, too? At the very least we need some comments here and for 
the loop in general, describing what's going on.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6998
+  // The CUDA toolchain should have a bound arch appended to the filename.
+  StringRef Arch = File.rsplit(".").first.rsplit('-').second;
+  CmdArgs.push_back(Args.MakeArgString(

Extracting arch name from the file name looks... questionable. Where does that 
file name come from? Can't we get this information directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

Nice! Thank you for adding support for multiple headers. Is this a step towards 
the json input file list?




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:467
 
+def err_drv_extract_api_wrong_kind : Error<
+  "header file '%0' input '%1' does not match the type of prior input "

Could you please add a test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/include/clang/Driver/Phases.h:17
   /// compilation process which interact with user options.
-  enum ID {
-Preprocess,

Is this an unrelated formatting change?



Comment at: clang/lib/Driver/Phases.cpp:19
   case Precompile: return "precompiler";
-  case Compile: return "compiler";
   case Backend: return "backend";

Unrelated format change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

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

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 416261.
dang added a comment.

Get rid of spurious clang-format changes due to reverted modifications


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Action.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/extract-api-multiheader.h
  clang/test/Driver/extract-api.c
  clang/test/Driver/extract-api.h
  clang/test/SymbolGraph/global_record.c

Index: clang/test/SymbolGraph/global_record.c
===
--- clang/test/SymbolGraph/global_record.c
+++ clang/test/SymbolGraph/global_record.c
@@ -3,7 +3,7 @@
 // RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
 // RUN: %t/reference.output.json
 // RUN: %clang -extract-api -target arm64-apple-macosx \
-// RUN: %t/input.c -o %t/output.json | FileCheck -allow-empty %s
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
@@ -13,7 +13,7 @@
 // CHECK-NOT: error:
 // CHECK-NOT: warning:
 
-//--- input.c
+//--- input.h
 int num;
 
 /**
@@ -80,7 +80,7 @@
   "location": {
 "character": 5,
 "line": 1,
-"uri": "file://INPUT_DIR/input.c"
+"uri": "file://INPUT_DIR/input.h"
   },
   "names": {
 "subHeading": [
@@ -272,7 +272,7 @@
   "location": {
 "character": 6,
 "line": 9,
-"uri": "file://INPUT_DIR/input.c"
+"uri": "file://INPUT_DIR/input.h"
   },
   "names": {
 "subHeading": [
Index: clang/test/Driver/extract-api.h
===
--- clang/test/Driver/extract-api.h
+++ clang/test/Driver/extract-api.h
@@ -3,8 +3,8 @@
 // RUN: FileCheck -check-prefix EXTRACT-API-PHASES -input-file %t %s
 
 // EXTRACT-API-PHASES: 0: input,
-// EXTRACT-API-PHASES: , c
-// EXTRACT-API-PHASES: 1: preprocessor, {0}, cpp-output
-// EXTRACT-API-PHASES: 2: compiler, {1}, api-information
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 1: preprocessor, {0}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 2: api-extractor, {1}, api-information
 // EXTRACT-API-PHASES-NOT: 3:
 // EXTRACT-API-PHASES: END
Index: clang/test/Driver/extract-api-multiheader.h
===
--- /dev/null
+++ clang/test/Driver/extract-api-multiheader.h
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang -target x86_64-unknown-unknown -ccc-print-phases -extract-api %t/first-header.h %t/second-header.h 2> %t1
+// RUN: echo 'END' >> %t1
+// RUN: FileCheck -check-prefix EXTRACT-API-PHASES -input-file %t1 %s
+
+// EXTRACT-API-PHASES: 0: input
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 1: preprocessor, {0}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 2: input
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 3: preprocessor, {2}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 4: api-extractor, {1, 3}, api-information
+// EXTRACT-API-PHASES-NOT: 5:
+// EXTRACT-API-PHASES: END
+
+//--- first-header.h
+
+void dummy_function(void);
+
+//--- second-header.h
+
+void other_dummy_function(void);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -28,6 +28,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
+#include "clang/Driver/Action.h"
 #include "clang/Driver/Distro.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/InputInfo.h"
@@ -4387,8 +4388,9 @@
   // CUDA/HIP compilation may have multiple inputs (source file + results of
   // device-side compilations). OpenMP device jobs also take the host IR as a
   // second input. Module precompilation accepts a list of header files to
-  // include as part of the module. All other jobs are expected to have exactly
-  // one input.
+  // include as part of the module. API extraction accepts a list of header
+  // files whose API information is emitted in the output. All other jobs are
+  // expected to have exactly one input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
   bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
@@ -4396,6 +4398,7 @@
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
   bool IsOpenMPHost = JA.isHostOffloading(Action::OFK_OpenMP);
   bool IsHeaderModulePrecompile = isa(JA);
+  bool 

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added reviewers: ributzka, zixuw, QuietMisdreavus, arphaman.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

clang -extract-api should accept multiple headers and forward them to a
single CC1 instance. This change introduces a new ExtractAPIJobAction.
Currently API Extraction is done during the Precompile phase as this is
the current phase that matches the requirements the most. Adding a new
phase would need to change some logic in how phases are scheduled. If
the headers scheduled for API extraction are of different types the
driver emits a diagnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121936

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/Phases.h
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Action.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Phases.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/extract-api-multiheader.h
  clang/test/Driver/extract-api.c
  clang/test/Driver/extract-api.h
  clang/test/SymbolGraph/global_record.c

Index: clang/test/SymbolGraph/global_record.c
===
--- clang/test/SymbolGraph/global_record.c
+++ clang/test/SymbolGraph/global_record.c
@@ -3,7 +3,7 @@
 // RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
 // RUN: %t/reference.output.json
 // RUN: %clang -extract-api -target arm64-apple-macosx \
-// RUN: %t/input.c -o %t/output.json | FileCheck -allow-empty %s
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
@@ -13,7 +13,7 @@
 // CHECK-NOT: error:
 // CHECK-NOT: warning:
 
-//--- input.c
+//--- input.h
 int num;
 
 /**
@@ -80,7 +80,7 @@
   "location": {
 "character": 5,
 "line": 1,
-"uri": "file://INPUT_DIR/input.c"
+"uri": "file://INPUT_DIR/input.h"
   },
   "names": {
 "subHeading": [
@@ -272,7 +272,7 @@
   "location": {
 "character": 6,
 "line": 9,
-"uri": "file://INPUT_DIR/input.c"
+"uri": "file://INPUT_DIR/input.h"
   },
   "names": {
 "subHeading": [
Index: clang/test/Driver/extract-api.h
===
--- clang/test/Driver/extract-api.h
+++ clang/test/Driver/extract-api.h
@@ -3,8 +3,8 @@
 // RUN: FileCheck -check-prefix EXTRACT-API-PHASES -input-file %t %s
 
 // EXTRACT-API-PHASES: 0: input,
-// EXTRACT-API-PHASES: , c
-// EXTRACT-API-PHASES: 1: preprocessor, {0}, cpp-output
-// EXTRACT-API-PHASES: 2: compiler, {1}, api-information
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 1: preprocessor, {0}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 2: api-extractor, {1}, api-information
 // EXTRACT-API-PHASES-NOT: 3:
 // EXTRACT-API-PHASES: END
Index: clang/test/Driver/extract-api-multiheader.h
===
--- /dev/null
+++ clang/test/Driver/extract-api-multiheader.h
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang -target x86_64-unknown-unknown -ccc-print-phases -extract-api %t/first-header.h %t/second-header.h 2> %t1
+// RUN: echo 'END' >> %t1
+// RUN: FileCheck -check-prefix EXTRACT-API-PHASES -input-file %t1 %s
+
+// EXTRACT-API-PHASES: 0: input
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 1: preprocessor, {0}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 2: input
+// EXTRACT-API-PHASES-SAME: , c-header
+// EXTRACT-API-PHASES-NEXT: 3: preprocessor, {2}, c-header-cpp-output
+// EXTRACT-API-PHASES-NEXT: 4: api-extractor, {1, 3}, api-information
+// EXTRACT-API-PHASES-NOT: 5:
+// EXTRACT-API-PHASES: END
+
+//--- first-header.h
+
+void dummy_function(void);
+
+//--- second-header.h
+
+void other_dummy_function(void);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -28,6 +28,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
+#include "clang/Driver/Action.h"
 #include "clang/Driver/Distro.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/InputInfo.h"
@@ -4387,8 +4388,9 @@
   // CUDA/HIP compilation may have multiple inputs (source file + results of
   // device-side compilations). OpenMP device jobs also take the host IR as a
   // second input. Module precompilation accepts a list of header files to
-  // include as part of the module. All other jobs are expected to have exactly
-  // one input.
+  // include as 

[PATCH] D121733: Clean pathnames in FileManager.

[Paul Pluzhnikov via Phabricator via cfe-commits]

ppluzhnikov updated this revision to Diff 416255.
ppluzhnikov added a comment.
Herald added a project: clang-tools-extra.

Fix Win x64 failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

Files:
  clang-tools-extra/test/clang-apply-replacements/order-dependent.cpp
  clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
  clang-tools-extra/test/clang-tidy/checkers/abseil-no-namespace.cpp
  clang/bindings/python/tests/cindex/test_translation_unit.py
  clang/lib/Basic/FileManager.cpp
  clang/test/Frontend/dependency-gen.c
  clang/test/Index/skip-parsed-bodies/compile_commands.json
  clang/test/Modules/filename.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -1593,21 +1593,22 @@
   return L.getFilePath() < R.getFilePath();
 });
 
-  ASSERT_EQ(Changes[0].getFilePath(), "./input.h");
-  EXPECT_THAT(Changes[0].getInsertedHeaders(), IsEmpty());
-  EXPECT_THAT(Changes[0].getRemovedHeaders(), IsEmpty());
+  const auto _h = Changes[1];
+  ASSERT_EQ(change_h.getFilePath(), "input.h");
+  EXPECT_THAT(change_h.getInsertedHeaders(), IsEmpty());
+  EXPECT_THAT(change_h.getRemovedHeaders(), IsEmpty());
   llvm::Expected UpdatedCode =
-  clang::tooling::applyAllReplacements(Header,
-   Changes[0].getReplacements());
+  clang::tooling::applyAllReplacements(Header, change_h.getReplacements());
   ASSERT_TRUE(static_cast(UpdatedCode))
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
   EXPECT_EQ(format(*UpdatedCode), "");
 
-  ASSERT_EQ(Changes[1].getFilePath(), "input.cc");
-  EXPECT_THAT(Changes[1].getInsertedHeaders(), IsEmpty());
-  EXPECT_THAT(Changes[1].getRemovedHeaders(), IsEmpty());
-  UpdatedCode = clang::tooling::applyAllReplacements(
-  Source, Changes[1].getReplacements());
+  const auto _cc = Changes[0];
+  ASSERT_EQ(change_cc.getFilePath(), "input.cc");
+  EXPECT_THAT(change_cc.getInsertedHeaders(), IsEmpty());
+  EXPECT_THAT(change_cc.getRemovedHeaders(), IsEmpty());
+  UpdatedCode =
+  clang::tooling::applyAllReplacements(Source, change_cc.getReplacements());
   ASSERT_TRUE(static_cast(UpdatedCode))
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
   EXPECT_EQ(format(*UpdatedCode), format("#include \"input.h\"\n"));
@@ -1630,7 +1631,7 @@
   {{"input.h", Header}}));
 
   ASSERT_EQ(Changes.size(), 1U);
-  ASSERT_EQ(Changes[0].getFilePath(), "./input.h");
+  ASSERT_EQ(Changes[0].getFilePath(), "input.h");
   EXPECT_THAT(Changes[0].getInsertedHeaders(), ElementsAre("header.h"));
   EXPECT_THAT(Changes[0].getRemovedHeaders(), IsEmpty());
   llvm::Expected UpdatedCode =
@@ -1679,7 +1680,7 @@
   ResultOf([](const AtomicChange ) { return C.getFilePath(); },
"input.cc"),
   ResultOf([](const AtomicChange ) { return C.getFilePath(); },
-   "./input.h";
+   "input.h";
 }
 
 } // namespace
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -292,7 +292,7 @@
   {"int main() {}",
R"(expanded tokens:
   int main ( ) { }
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 int main ( ) { }
   no mappings.
@@ -301,7 +301,7 @@
   {"\t\n  int\t\n  main\t\n  (\t\n  )\t\n{\t\n  }\t\n",
R"(expanded tokens:
   int main ( ) { }
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 int main ( ) { }
   no mappings.
@@ -313,7 +313,7 @@
   )cpp",
R"(expanded tokens:
   
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 # pragma GCC visibility push ( public ) # pragma GCC visibility pop
   mappings:
@@ -322,7 +322,7 @@
   // Empty files should not crash.
   {R"cpp()cpp", R"(expanded tokens:
   
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 
   no mappings.
@@ -336,7 +336,7 @@
 )cpp",
   R"(expanded tokens:
   a
-file './input.cpp'
+file 'input.cpp'
   spelled tokens:
 a # define MACRO ( ) A # B
   mappings:
@@ -401,7 +401,7 @@
   std::string Expected =
   "expanded tokens:\n"
   "  int a ;\n"
-  "file './input.cpp'\n"
+  "file 'input.cpp'\n"
   "  spelled tokens:\n"
   "# define FOO a # include \"unresolved_file.h\" # undef FOO "
   "# ifdef X # else # endif # ifndef Y # endif # if 1 # elif 2 # else "
@@ -420,7 +420,7 @@
   )cpp",
R"(expanded tokens:
   int const a ;
-file 

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

[Johannes Doerfert 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 rGf02550bdd9b7: Reapply [OpenMP][FIX] Allow device 
constructors for AMD GPU (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D121837?vs=416218=416250#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121837

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

Index: clang/test/OpenMP/amdgcn_target_global_constructor.cpp
===
--- /dev/null
+++ clang/test/OpenMP/amdgcn_target_global_constructor.cpp
@@ -0,0 +1,105 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --check-globals --include-generated-funcs
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple amdgcn-amd-amdhsa -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+void foo(void);
+
+struct S {
+  int a;
+  S() : a(1) {}
+  ~S() { foo(); }
+};
+
+#pragma omp declare target
+S A;
+#pragma omp end declare target
+
+#endif
+//.
+// CHECK: @__omp_rtl_debug_kind = weak_odr hidden addrspace(1) constant i32 0
+// CHECK: @__omp_rtl_assume_teams_oversubscription = weak_odr hidden addrspace(1) constant i32 0
+// CHECK: @__omp_rtl_assume_threads_oversubscription = weak_odr hidden addrspace(1) constant i32 0
+// CHECK: @__omp_rtl_assume_no_thread_state = weak_odr hidden addrspace(1) constant i32 0
+// CHECK: @A = addrspace(1) global %struct.S zeroinitializer, align 4
+// CHECK: @llvm.used = appending addrspace(1) global [2 x i8*] [i8* bitcast (void ()* @__omp_offloading_{{.*}}_ctor to i8*), i8* bitcast (void ()* @__omp_offloading_{{.*}}_dtor to i8*)], section "llvm.metadata"
+//.
+// CHECK-LABEL: define {{[^@]+}}@__omp_offloading_{{.*}}_ctor
+// CHECK-SAME: () #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @_ZN1SC1Ev(%struct.S* noundef nonnull align 4 dereferenceable(4) addrspacecast ([[STRUCT_S:%.*]] addrspace(1)* @A to %struct.S*)) #[[ATTR3:[0-9]+]]
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@_ZN1SC1Ev
+// CHECK-SAME: (%struct.S* noundef nonnull align 4 dereferenceable(4) [[THIS:%.*]]) unnamed_addr #[[ATTR1:[0-9]+]] comdat align 2 {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca %struct.S*, align 8, addrspace(5)
+// CHECK-NEXT:[[THIS_ADDR_ASCAST:%.*]] = addrspacecast %struct.S* addrspace(5)* [[THIS_ADDR]] to %struct.S**
+// CHECK-NEXT:store %struct.S* [[THIS]], %struct.S** [[THIS_ADDR_ASCAST]], align 8
+// CHECK-NEXT:[[THIS1:%.*]] = load %struct.S*, %struct.S** [[THIS_ADDR_ASCAST]], align 8
+// CHECK-NEXT:call void @_ZN1SC2Ev(%struct.S* noundef nonnull align 4 dereferenceable(4) [[THIS1]]) #[[ATTR3]]
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@__omp_offloading_{{.*}}_dtor
+// CHECK-SAME: () #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @_ZN1SD1Ev(%struct.S* noundef nonnull align 4 dereferenceable(4) addrspacecast ([[STRUCT_S:%.*]] addrspace(1)* @A to %struct.S*)) #[[ATTR4:[0-9]+]]
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@_ZN1SD1Ev
+// CHECK-SAME: (%struct.S* noundef nonnull align 4 dereferenceable(4) [[THIS:%.*]]) unnamed_addr #[[ATTR1]] comdat align 2 {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca %struct.S*, align 8, addrspace(5)
+// CHECK-NEXT:[[THIS_ADDR_ASCAST:%.*]] = addrspacecast %struct.S* addrspace(5)* [[THIS_ADDR]] to %struct.S**
+// CHECK-NEXT:store %struct.S* [[THIS]], %struct.S** [[THIS_ADDR_ASCAST]], align 8
+// CHECK-NEXT:[[THIS1:%.*]] = load %struct.S*, %struct.S** [[THIS_ADDR_ASCAST]], align 8
+// CHECK-NEXT:call void @_ZN1SD2Ev(%struct.S* noundef nonnull align 4 dereferenceable(4) [[THIS1]]) #[[ATTR4]]
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@_ZN1SC2Ev
+// CHECK-SAME: (%struct.S* noundef nonnull align 4 dereferenceable(4) [[THIS:%.*]]) unnamed_addr #[[ATTR1]] comdat align 2 {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca %struct.S*, align 8, addrspace(5)
+// CHECK-NEXT:[[THIS_ADDR_ASCAST:%.*]] = addrspacecast %struct.S* addrspace(5)* [[THIS_ADDR]] to %struct.S**
+// CHECK-NEXT:store %struct.S* [[THIS]], %struct.S** [[THIS_ADDR_ASCAST]], align 8
+// CHECK-NEXT:[[THIS1:%.*]] = load %struct.S*, %struct.S** [[THIS_ADDR_ASCAST]], align 8
+// CHECK-NEXT:[[A:%.*]] = getelementptr inbounds [[STRUCT_S:%.*]], %struct.S* 

[clang] f02550b - Reapply "[OpenMP][FIX] Allow device constructors for AMD GPU"

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2022-03-17T12:53:47-05:00
New Revision: f02550bdd9b7e4b442009edc02f9e3f78400ae30

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

LOG: Reapply "[OpenMP][FIX] Allow device constructors for AMD GPU"

This reverts commit a597d6a780b184539f504392168b004bf392a135 and
reapplies 07b176646134.

In AMD GPU device code the globals are in AS(1). Before, we crashed if
the global was a structure. Now we simply cast away the AS before we
generate the code to initialize the global.

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

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

Added: 
clang/test/OpenMP/amdgcn_target_global_constructor.cpp

Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index eabebd9c3ed37..1192d194f5865 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -35,6 +35,7 @@
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/GlobalValue.h"
+#include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/Format.h"
@@ -1940,8 +1941,14 @@ bool 
CGOpenMPRuntime::emitDeclareTargetVarDefinition(const VarDecl *VD,
   CtorCGF.StartFunction(GlobalDecl(), CGM.getContext().VoidTy, Fn, FI,
 FunctionArgList(), Loc, Loc);
   auto AL = ApplyDebugLocation::CreateArtificial(CtorCGF);
+  llvm::Constant *AddrInAS0 = Addr;
+  if (Addr->getAddressSpace() != 0)
+AddrInAS0 = llvm::ConstantExpr::getAddrSpaceCast(
+Addr, llvm::PointerType::getWithSamePointeeType(
+  cast(Addr->getType()), 0));
   CtorCGF.EmitAnyExprToMem(
-  Init, Address::deprecated(Addr, CGM.getContext().getDeclAlign(VD)),
+  Init,
+  Address::deprecated(AddrInAS0, CGM.getContext().getDeclAlign(VD)),
   Init->getType().getQualifiers(),
   /*IsInitializer=*/true);
   CtorCGF.FinishFunction();
@@ -1980,9 +1987,14 @@ bool 
CGOpenMPRuntime::emitDeclareTargetVarDefinition(const VarDecl *VD,
   // Create a scope with an artificial location for the body of this
   // function.
   auto AL = ApplyDebugLocation::CreateArtificial(DtorCGF);
+  llvm::Constant *AddrInAS0 = Addr;
+  if (Addr->getAddressSpace() != 0)
+AddrInAS0 = llvm::ConstantExpr::getAddrSpaceCast(
+Addr, llvm::PointerType::getWithSamePointeeType(
+  cast(Addr->getType()), 0));
   DtorCGF.emitDestroy(
-  Address::deprecated(Addr, CGM.getContext().getDeclAlign(VD)), ASTTy,
-  DtorCGF.getDestroyer(ASTTy.isDestructedType()),
+  Address::deprecated(AddrInAS0, CGM.getContext().getDeclAlign(VD)),
+  ASTTy, DtorCGF.getDestroyer(ASTTy.isDestructedType()),
   DtorCGF.needsEHCleanup(ASTTy.isDestructedType()));
   DtorCGF.FinishFunction();
   Dtor = Fn;

diff  --git a/clang/test/OpenMP/amdgcn_target_global_constructor.cpp 
b/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
new file mode 100644
index 0..72d6f177ef7b7
--- /dev/null
+++ b/clang/test/OpenMP/amdgcn_target_global_constructor.cpp
@@ -0,0 +1,105 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --function-signature --check-globals --include-generated-funcs
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown 
-fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple amdgcn-amd-amdhsa 
-fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+void foo(void);
+
+struct S {
+  int a;
+  S() : a(1) {}
+  ~S() { foo(); }
+};
+
+#pragma omp declare target
+S A;
+#pragma omp end declare target
+
+#endif
+//.
+// CHECK: @__omp_rtl_debug_kind = weak_odr hidden addrspace(1) constant i32 0
+// CHECK: @__omp_rtl_assume_teams_oversubscription = weak_odr hidden 
addrspace(1) constant i32 0
+// CHECK: @__omp_rtl_assume_threads_oversubscription = weak_odr hidden 
addrspace(1) constant i32 0
+// CHECK: @__omp_rtl_assume_no_thread_state = weak_odr hidden addrspace(1) 
constant i32 0
+// CHECK: @A = addrspace(1) global %struct.S zeroinitializer, align 4
+// CHECK: @llvm.used = appending addrspace(1) global [2 x i8*] [i8* bitcast 
(void ()* @__omp_offloading_{{.*}}_ctor to i8*), i8* bitcast (void ()* 
@__omp_offloading_{{.*}}_dtor to i8*)], section "llvm.metadata"
+//.
+// CHECK-LABEL: define 

[PATCH] D121736: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

[Julian Lettner 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 rG22570bac6943: Lower `@llvm.global_dtors` using 
`__cxa_atexit` on MachO (authored by yln).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121736

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/docs/Passes.rst
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/LinkAllPasses.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
  llvm/include/llvm/Transforms/Utils.h
  llvm/include/llvm/Transforms/Utils/LowerGlobalDtors.h
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Target/WebAssembly/CMakeLists.txt
  llvm/lib/Target/WebAssembly/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyLowerGlobalDtors.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/LowerGlobalDtors.cpp
  llvm/lib/Transforms/Utils/Utils.cpp
  llvm/test/CodeGen/ARM/ctors_dtors.ll
  llvm/test/CodeGen/X86/2011-08-29-InitOrder.ll
  llvm/test/Transforms/LowerGlobalDestructors/lower-global-dtors.ll

Index: llvm/test/Transforms/LowerGlobalDestructors/lower-global-dtors.ll
===
--- /dev/null
+++ llvm/test/Transforms/LowerGlobalDestructors/lower-global-dtors.ll
@@ -0,0 +1,156 @@
+; RUN: opt-lower-global-dtors -S < %s | FileCheck %s --implicit-check-not=llvm.global_dtors
+; RUN: opt -passes=lower-global-dtors -S < %s | FileCheck %s --implicit-check-not=llvm.global_dtors
+
+; Test that @llvm.global_dtors is properly lowered into @llvm.global_ctors,
+; grouping dtor calls by priority and associated symbol.
+
+declare void @orig_ctor()
+declare void @orig_dtor0()
+declare void @orig_dtor1a()
+declare void @orig_dtor1b()
+declare void @orig_dtor1c0()
+declare void @orig_dtor1c1a()
+declare void @orig_dtor1c1b()
+declare void @orig_dtor1c2a()
+declare void @orig_dtor1c2b()
+declare void @orig_dtor1c3()
+declare void @orig_dtor1d()
+declare void @orig_dtor65535()
+declare void @orig_dtor65535c0()
+declare void @after_the_null()
+
+@associatedc0 = external global i8
+@associatedc1 = external global i8
+@associatedc2 = global i8 42
+@associatedc3 = global i8 84
+
+@llvm.global_ctors = appending global
+[1 x { i32, void ()*, i8* }]
+[
+  { i32, void ()*, i8* } { i32 200, void ()* @orig_ctor, i8* null }
+]
+
+@llvm.global_dtors = appending global
+[14 x { i32, void ()*, i8* }]
+[
+  { i32, void ()*, i8* } { i32 0, void ()* @orig_dtor0, i8* null },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1a, i8* null },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1b, i8* null },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1c0, i8* @associatedc0 },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1c1a, i8* @associatedc1 },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1c1b, i8* @associatedc1 },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1c2a, i8* @associatedc2 },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1c2b, i8* @associatedc2 },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1c3, i8* @associatedc3 },
+  { i32, void ()*, i8* } { i32 1, void ()* @orig_dtor1d, i8* null },
+  { i32, void ()*, i8* } { i32 65535, void ()* @orig_dtor65535c0, i8* @associatedc0 },
+  { i32, void ()*, i8* } { i32 65535, void ()* @orig_dtor65535, i8* null },
+  { i32, void ()*, i8* } { i32 65535, void ()* null, i8* null },
+  { i32, void ()*, i8* } { i32 65535, void ()* @after_the_null, i8* null }
+]
+
+; CHECK: @associatedc0 = external global i8
+; CHECK: @associatedc1 = external global i8
+; CHECK: @associatedc2 = global i8 42
+; CHECK: @associatedc3 = global i8 84
+; CHECK: @__dso_handle = extern_weak hidden constant i8
+
+; CHECK-LABEL: @llvm.global_ctors = appending global [10 x { i32, void ()*, i8* }] [
+; CHECK-SAME:  { i32, void ()*, i8* } { i32 200, void ()* @orig_ctor, i8* null },
+; CHECK-SAME:  { i32, void ()*, i8* } { i32 0, void ()* @register_call_dtors.0, i8* null },
+; CHECK-SAME:  { i32, void ()*, i8* } { i32 1, void ()* @"register_call_dtors.1$0", i8* null },
+; CHECK-SAME:  { i32, void ()*, i8* } { i32 1, void ()* @"register_call_dtors.1$1.associatedc0", i8* @associatedc0 },
+; CHECK-SAME:  { i32, void ()*, i8* } { i32 1, void ()* @"register_call_dtors.1$2.associatedc1", i8* @associatedc1 },
+; CHECK-SAME:  { i32, void ()*, i8* } { i32 1, void ()* @"register_call_dtors.1$3.associatedc2", i8* @associatedc2 },
+; CHECK-SAME:  { i32, void ()*, i8* } { i32 1, void ()* 

[clang] 22570ba - Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

[Julian Lettner via cfe-commits]

Author: Julian Lettner
Date: 2022-03-17T10:47:13-07:00
New Revision: 22570bac694396514fff18dec926558951643fa6

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

LOG: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

For MachO, lower `@llvm.global_dtors` into `@llvm_global_ctors` with
`__cxa_atexit` calls to avoid emitting the deprecated `__mod_term_func`.

Reuse the existing `WebAssemblyLowerGlobalDtors.cpp` to accomplish this.

Enable fallback to the old behavior via Clang driver flag
(`-fregister-global-dtors-with-atexit`) or llc / code generation flag
(`-lower-global-dtors-via-cxa-atexit`).  This escape hatch will be
removed in the future.

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

Added: 
llvm/include/llvm/Transforms/Utils/LowerGlobalDtors.h
llvm/lib/Transforms/Utils/LowerGlobalDtors.cpp
llvm/test/Transforms/LowerGlobalDestructors/lower-global-dtors.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/docs/Passes.rst
llvm/include/llvm/CodeGen/CommandFlags.h
llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
llvm/include/llvm/InitializePasses.h
llvm/include/llvm/LinkAllPasses.h
llvm/include/llvm/Target/TargetOptions.h
llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
llvm/include/llvm/Transforms/Utils.h
llvm/lib/CodeGen/CodeGen.cpp
llvm/lib/CodeGen/CommandFlags.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/CodeGen/TargetPassConfig.cpp
llvm/lib/Passes/PassBuilder.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Target/WebAssembly/CMakeLists.txt
llvm/lib/Target/WebAssembly/WebAssembly.h
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
llvm/lib/Transforms/Utils/CMakeLists.txt
llvm/lib/Transforms/Utils/Utils.cpp
llvm/test/CodeGen/ARM/ctors_dtors.ll
llvm/test/CodeGen/X86/2011-08-29-InitOrder.ll

Removed: 
llvm/lib/Target/WebAssembly/WebAssemblyLowerGlobalDtors.cpp



diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 490f5b3de1ff3..716a565ee7871 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -546,6 +546,8 @@ static bool initTargetOptions(DiagnosticsEngine ,
   Options.BinutilsVersion =
   llvm::TargetMachine::parseBinutilsVersion(CodeGenOpts.BinutilsVersion);
   Options.UseInitArray = CodeGenOpts.UseInitArray;
+  Options.LowerGlobalDtorsViaCxaAtExit =
+  CodeGenOpts.RegisterGlobalDtorsWithAtExit;
   Options.DisableIntegratedAS = CodeGenOpts.DisableIntegratedAS;
   Options.CompressDebugSections = CodeGenOpts.getCompressDebugSections();
   Options.RelaxELFRelocations = CodeGenOpts.RelaxELFRelocations;

diff  --git a/llvm/docs/Passes.rst b/llvm/docs/Passes.rst
index 92f06496b4ef9..7c0666992e8f5 100644
--- a/llvm/docs/Passes.rst
+++ b/llvm/docs/Passes.rst
@@ -876,6 +876,14 @@ This pass expects :ref:`LICM ` to be run 
before it to hoist
 invariant conditions out of the loop, to make the unswitching opportunity
 obvious.
 
+``-lower-global-dtors``: Lower global destructors
+
+
+This pass lowers global module destructors (``llvm.global_dtors``) by creating
+wrapper functions that are registered as global constructors in
+``llvm.global_ctors`` and which contain a call to ``__cxa_atexit`` to register
+their destructor functions.
+
 ``-loweratomic``: Lower atomic intrinsics to non-atomic form
 
 

diff  --git a/llvm/include/llvm/CodeGen/CommandFlags.h 
b/llvm/include/llvm/CodeGen/CommandFlags.h
index feb5de5415269..9281ed723854c 100644
--- a/llvm/include/llvm/CodeGen/CommandFlags.h
+++ b/llvm/include/llvm/CodeGen/CommandFlags.h
@@ -93,6 +93,8 @@ std::string getTrapFuncName();
 
 bool getUseCtors();
 
+bool getLowerGlobalDtorsViaCxaAtExit();
+
 bool getRelaxELFRelocations();
 
 bool getDataSections();

diff  --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h 
b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
index 2a35987507446..26bda8d5d239d 100644
--- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
+++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
@@ -119,6 +119,9 @@ class TargetLoweringObjectFileMachO : public 
TargetLoweringObjectFile {
 
   void Initialize(MCContext , const TargetMachine ) override;
 
+  MCSection *getStaticDtorSection(unsigned Priority,
+  const MCSymbol *KeySym) const override;
+
   /// Emit the module flags that specify the garbage collection information.
   void emitModuleMetadata(MCStreamer , Module ) const override;
 

diff  --git a/llvm/include/llvm/InitializePasses.h 
b/llvm/include/llvm/InitializePasses.h
index 

[PATCH] D121890: [clang-format] Copy help options to the doc directory.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a subscriber: krasimir.
owenpan added a comment.

In D121890#3389171 , @sstwcw wrote:

> The issue is #54418.
>
> Actually I don't know who I should add as reviewers.  The patches for 
> clang-format don't always seem to have the same reviewers.  Who are the main 
> developers and how do you decide who you add each time?

We have been adding @MyDeveloperDay @curdeius @HazardyKnusperkeks as reviewers 
to your patches. For JavaScript, you should add @krasimir too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121890

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

[PATCH] D121907: [clang-format] Use an enum for context types.

[Owen Pan via Phabricator via cfe-commits]

owenpan added subscribers: HazardyKnusperkeks, MyDeveloperDay, curdeius, 
owenpan.
owenpan added a comment.

If this patch is an NFC, please add `[NFC]` to the title.

Please add @MyDeveloperDay @curdeius @HazardyKnusperkeks to Reviewers.




Comment at: clang/lib/Format/TokenAnnotator.cpp:116
 // template parameter, not an argument.
-Contexts.back().InTemplateArgument =
-Left->Previous && Left->Previous->isNot(tok::kw_template);
+if (Left->Previous && Left->Previous->isNot(tok::kw_template))
+  Contexts.back().ContextType = Context::TemplateArgument;

If this was bug, it should be in a separate patch with test cases added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121907

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

In D119792#3389126 , @erichkeane 
wrote:

> So P2025  has not successfully made it 
> through EWG, so this would have to be under a 'flag'.  Also, I believe this 
> will end up being applied to C++23, so it would have to be under a C++23 
> flag, even when we make it a default behavior.

I felt like this particular patch don't really need to wait until the paper 
make it to C++XX.

**RVO** (unnamed return value optimization), a simpler optimization, has been 
used for a very very long time, before they made it mandatory in C++17 
(effectively just describing the status quo in the Standard).

The paper contains an exhaustive set of NRVO use-cases. **13** out of **20** 
cases are already implemented in Clang, and the current patch makes it **17** 
out of **20**. I could send a patch without mentioning the paper at all, but it 
would be harder to track progress.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119792

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

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: jamieschmeiser, aaron.ballman, 
chandlerc, cebowleratibm, w2yehia.
Herald added a project: All.
hubert.reinterpretcast requested review of this revision.
Herald added a project: clang.

Update `WeakUndeclaredIdentifiers` to hold a collection of weak aliases per 
identifier instead of only one.

This also allows the "used" state to be removed from `WeakInfo` because it is 
really only there as an alternative to removing processed map entries, and we 
can represent that using an empty set now. The serialization code is updated 
for the removal of the field. Additionally, a PCH test is added for the new 
functionality.

The records are grouped by the "target" identifier, which was already being 
used as a key for lookup purposes. We also store only one record per alias 
name; combined, this means that diagnostics are grouped by the "target" and 
limited to one per alias (which should be acceptable).

Fixes PR28611.
Fixes llvm/llvm-project#28985.

Co-authored-by: Rachel Craik 
Co-authored-by: Jamie Schmeiser 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121927

Files:
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Weak.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGen/pragma-weak.c
  clang/test/PCH/pragma-weak-functional.c
  clang/test/PCH/pragma-weak-functional.h

Index: clang/test/PCH/pragma-weak-functional.h
===
--- /dev/null
+++ clang/test/PCH/pragma-weak-functional.h
@@ -0,0 +1,6 @@
+// Header for PCH test pragma-weak-functional.c
+
+#pragma weak undecfunc_alias2 = undecfunc
+#pragma weak undecfunc_alias4 = undecfunc_alias1
+#pragma weak undecfunc_alias3 = undecfunc_alias1
+#pragma weak undecfunc_alias1 = undecfunc
Index: clang/test/PCH/pragma-weak-functional.c
===
--- /dev/null
+++ clang/test/PCH/pragma-weak-functional.c
@@ -0,0 +1,17 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/pragma-weak-functional.h %s -verify -emit-llvm -o - | FileCheck %s
+
+// Test with pch.
+// RUN: %clang_cc1 -x c-header -emit-pch -o %t %S/pragma-weak-functional.h
+// RUN: %clang_cc1 -include-pch %t %s -verify -emit-llvm -o - | FileCheck %s
+
+// CHECK-DAG: @undecfunc_alias1 = weak{{.*}} alias void (), void ()* @undecfunc
+// CHECK-DAG: @undecfunc_alias2 = weak{{.*}} alias void (), void ()* @undecfunc
+// CHECK-DAG: @undecfunc_alias3 = weak{{.*}} alias void (), void ()* @undecfunc
+// CHECK-DAG: @undecfunc_alias4 = weak{{.*}} alias void (), void ()* @undecfunc
+
+/ PR28611: Try multiple aliases of same undeclared symbol or alias
+void undecfunc_alias1(void);
+void undecfunc(void) { }
+// expected-warning@pragma-weak-functional.h:4 {{alias will always resolve to undecfunc}}
+// expected-warning@pragma-weak-functional.h:5 {{alias will always resolve to undecfunc}}
Index: clang/test/CodeGen/pragma-weak.c
===
--- clang/test/CodeGen/pragma-weak.c
+++ clang/test/CodeGen/pragma-weak.c
@@ -17,6 +17,10 @@
 // CHECK-DAG: @mix2 = weak{{.*}} alias void (), void ()* @__mix2
 // CHECK-DAG: @a1 = weak{{.*}} alias void (), void ()* @__a1
 // CHECK-DAG: @xxx = weak{{.*}} alias void (), void ()* @__xxx
+// CHECK-DAG: @undecfunc_alias1 = weak{{.*}} alias void (), void ()* @undecfunc
+// CHECK-DAG: @undecfunc_alias2 = weak{{.*}} alias void (), void ()* @undecfunc
+// CHECK-DAG: @undecfunc_alias3 = weak{{.*}} alias void (), void ()* @undecfunc
+// CHECK-DAG: @undecfunc_alias4 = weak{{.*}} alias void (), void ()* @undecfunc
 
 
 
@@ -136,6 +140,14 @@
 __attribute((pure,noinline,const)) void __xxx(void) { }
 // CHECK: void @__xxx() [[RN:#[0-9]+]]
 
+/ PR28611: Try multiple aliases of same undeclared symbol or alias
+#pragma weak undecfunc_alias1 = undecfunc
+#pragma weak undecfunc_alias3 = undecfunc_alias2 // expected-warning {{alias will always resolve to undecfunc}}
+#pragma weak undecfunc_alias4 = undecfunc_alias2 // expected-warning {{alias will always resolve to undecfunc}}
+#pragma weak undecfunc_alias2 = undecfunc
+void undecfunc_alias2(void);
+void undecfunc(void) { }
+
 / PR10878: Make sure we can call a weak alias
 void SHA512Pad(void *context) {}
 #pragma weak SHA384Pad = SHA512Pad
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4561,13 +4561,14 @@
   // entire table, since later PCH files in a PCH chain are only interested in
   // the results at the end of the chain.
   RecordData WeakUndeclaredIdentifiers;
-  for (auto  : SemaRef.WeakUndeclaredIdentifiers) {
-IdentifierInfo *II = 

[PATCH] D121906: [clang-format] Indent import statements in JavaScript.

[Owen Pan via Phabricator via cfe-commits]

owenpan added subscribers: HazardyKnusperkeks, krasimir, MyDeveloperDay, 
curdeius, owenpan.
owenpan added a comment.

Please add @krasimir @MyDeveloperDay @curdeius @HazardyKnusperkeks to Reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121906

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