[PATCH] D149986: AMDGPU: Force sc0 and sc1 on stores for gfx940 and gfx941

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh added a comment.

I think that if this is a new property of the GFX940/941 targets, and turning 
it off shouldn't be possible, we shouldn't even bother with a feature and just 
set a bool in the ST for those targets




Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:524
+ SIAtomicAddrSpace::NONE)
+  return enableSC0Bit(MI) | enableSC1Bit(MI);
+return false;

kzhuravl wrote:
> jmmartinez wrote:
> > NIT: Is the use of the bitwise or " | " intended? I'd use the logical or " 
> > || " instead.
> It is intentional, we need both SC0 and SC1 bits set. If I switch this to || 
> it will short circuit and not invoke enableSC1Bit.
IMHO then it needs a comment to explain that it's intentional, otherwise some 
innocent maintainer in the future could think it's a typo and change it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149986

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

[PATCH] D146023: [AMDGPU] Remove Code Object V2

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh removed a reviewer: jdoerfert.
Pierre-vh added a comment.
Herald added a reviewer: jdoerfert.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146023

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

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

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

donat.nagy added inline comments.



Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:1-2
+typedef typeof(sizeof(int)) size_t;
+typedef signed long ssize_t;
+typedef struct {

balazske wrote:
> steakhal wrote:
> > balazske wrote:
> > > steakhal wrote:
> > > > `ssize_t`'s size should match the size of `size_t`. In this 
> > > > implementation, it would be true only if `size_t` is `long`.
> > > > 
> > > I could not find a working way of defining the type in that way (there is 
> > > no `__sizte_t`). The current definition should work well in the test 
> > > code, the property of being the same size is supposedly not used in the 
> > > tests. The previous definition was not better than this (and different in 
> > > different places).
> > I think [[ 
> > https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/format-strings-scanf.c#L7-L15
> >  | clang/test/Sema/format-strings-scanf.c ]] uses something like this: 
> > ```lang=C++
> > typedef __SIZE_TYPE__ size_t;
> > #define __SSIZE_TYPE__  
> >\
> >   __typeof__(_Generic((__SIZE_TYPE__)0, 
> >\
> >   unsigned long long int : (long long int)0,
> >\
> >   unsigned long int : (long int)0,  
> >\
> >   unsigned int : (int)0,
> >\
> >   unsigned short : (short)0,
> >\
> >   unsigned char : (signed char)0))
> > typedef __SSIZE_TYPE__ ssize_t;
> > ```
> This may work (probably not on all platforms) but still I think in this 
> context it is only important that we have a type called `ssize_t` and it is 
> signed, it is less important that it is exactly the correct type. Other types 
> in the same header are not exact, and `ssize_t` in other test code (in 
> Analysis tests) is defined not in this way.
I agree that we don't need that `_Generic` magic in this particular test file. 
If we want consistency between the sizes of `size_t` and `ssize_t` then you may 
define them as e.g. just `unsigned long long` and `long long` -- but I think 
even that consideration is overkill.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

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

[PATCH] D149562: [clang-format] Stop comment disrupting indentation of Verilog ports

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D149562#4332188 , @sstwcw wrote:

>> IMO a trailing comment (empty or not) belongs to the code before it.
>
> There is only a parenthesis before it.  It doesn't usually need a comment.  
> It is like in 5a61139.  One doesn't tend to comment a single brace.

I agree if the brace doesn't start a block, but clang-format sometimes got it 
wrong and misannotates a block as a braced list. (See 
https://github.com/llvm/llvm-project/issues/33891 for an example.)

>> A comment about the code below it should start on a new line.
>
> In this special case, the comment would be indented to the same column as the 
> next line, so it should be clear it is for the next line.  Like the case 
> forbraced initializer lists, the first field will be on the same line as the 
> brace if there isn't a comment, so the first comment is also on the same line 
> as the brace when there is one.
>
>   foo foo{// first field
>   a,
>   // second field
>   b};

I'll go along with other reviewers on this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149562

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

[PATCH] D150139: [clang-repl] Enable basic multiline support.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/test/Interpreter/multiline.cpp:12-17
+void f(int x) \ 
+{   \
+  printf("x=\
+  %d", x); \
+}
+f(i);

aaron.ballman wrote:
> Another fun test case:
> ```
> // Requires -ftrigraphs but the following line ends with a backslash 
> (surprise!)
> i=??/
>   12;
> ```
Yes, the implementation of the multiline support here is actually rather 
rudimentary. It intentionally does not include deeper language understanding 
but provides a way for the users typing "well-behaved" code to tell clang-repl 
that more is coming before it could compile it. In theory we could check for 
the `??/` trigraph and do the same but I don't think that would be used people 
use clang-repl on things like IBM 3270 terminals which seem not to have some 
characters and trigraphs could help there ;) 

Our full-fledged solution is described here 
https://discourse.llvm.org/t/rfc-flexible-lexer-buffering-for-handling-incomplete-input-in-interactive-c-c/64180/9

Until that lands we can have this to unblock work on things like OpenMP support.


Repository:
  rC Clang

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

https://reviews.llvm.org/D150139

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

[PATCH] D150181: [XCOFF][DWARF] XCOFF64 should be able to select the dwarf version under intergrated-as mode.

[Esme Yi via Phabricator via cfe-commits]

Esme updated this revision to Diff 521229.
Esme added a comment.
Herald added a subscriber: nemanjai.

Update test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150181

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options-as.c
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/test/CodeGen/PowerPC/aix-dwarf.ll
  llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section-debug.ll
  llvm/test/DebugInfo/XCOFF/dwarf-format.ll

Index: llvm/test/DebugInfo/XCOFF/dwarf-format.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/XCOFF/dwarf-format.ll
@@ -0,0 +1,33 @@
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj %s -o - \
+; RUN:   | llvm-dwarfdump -debug-line - | FileCheck %s --check-prefixes=CHECK
+
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj -dwarf64 %s -o - \
+; RUN:   | llvm-dwarfdump -debug-line - | FileCheck %s --check-prefixes=CHECK64
+
+; CHECK: file format aix5coff64-rs6000
+; CHECK: format: DWARF32
+
+; CHECK64: file format aix5coff64-rs6000
+; CHECK64: format: DWARF64
+
+source_filename = "1.c"
+target datalayout = "E-m:a-p:32:32-Fi32-i64:64-n32"
+
+@foo = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "foo", scope: !2, file: !3, line: 1, type: !5, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 17.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "1.c", directory: "llvm-project")
+!4 = !{!0}
+!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!6 = !{i32 7, !"Dwarf Version", i32 3}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{i32 1, !"wchar_size", i32 2}
+!9 = !{i32 8, !"PIC Level", i32 2}
+!10 = !{i32 7, !"frame-pointer", i32 2}
+!11 = !{!"clang version 17.0.0"}
Index: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section-debug.ll
===
--- llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section-debug.ll
+++ llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section-debug.ll
@@ -3,8 +3,8 @@
 ; Exception auxilliary entries are present in the 64-bit tests because 64-bit && debug enabled are the requirements.
 ; RUN: llc -mtriple=powerpc-ibm-aix-xcoff -filetype=obj -o %t_32.o < %s
 ; RUN: llvm-readobj --syms %t_32.o | FileCheck %s --check-prefix=SYMS32
-; RUN: llc -mtriple=powerpc64-unknown-aix -filetype=obj -o %t_32.o < %s
-; RUN: llvm-readobj --syms %t_32.o | FileCheck %s --check-prefix=SYMS64
+; RUN: llc -mtriple=powerpc64-unknown-aix -filetype=obj -o %t_64.o < %s
+; RUN: llvm-readobj --syms %t_64.o | FileCheck %s --check-prefix=SYMS64
 
 ; If any debug information is included in a module and is XCOFF64, exception auxilliary entries are emitted
 
@@ -93,7 +93,7 @@
 ; SYMS64-NEXT:  NumberOfAuxEntries: 3
 ; SYMS64-NEXT:  Exception Auxiliary Entry {
 ; SYMS64-NEXT:Index: [[#IND+1]]
-; SYMS64-NEXT:OffsetToExceptionTable: 0x398
+; SYMS64-NEXT:OffsetToExceptionTable: 0x38C
 ; SYMS64-NEXT:SizeOfFunction: 0x18
 ; SYMS64-NEXT:SymbolIndexOfNextBeyond: [[#IND+4]]
 ; SYMS64-NEXT:Auxiliary Type: AUX_EXCEPT (0xFF)
@@ -126,7 +126,7 @@
 ; SYMS64-NEXT:  NumberOfAuxEntries: 3
 ; SYMS64-NEXT:  Exception Auxiliary Entry {
 ; SYMS64-NEXT:Index: [[#IND+5]]
-; SYMS64-NEXT:OffsetToExceptionTable: 0x3AC
+; SYMS64-NEXT:OffsetToExceptionTable: 0x3A0
 ; SYMS64-NEXT:SizeOfFunction: 0x68
 ; SYMS64-NEXT:SymbolIndexOfNextBeyond: [[#IND+8]]
 ; SYMS64-NEXT:Auxiliary Type: AUX_EXCEPT (0xFF)
Index: llvm/test/CodeGen/PowerPC/aix-dwarf.ll
===
--- llvm/test/CodeGen/PowerPC/aix-dwarf.ll
+++ llvm/test/CodeGen/PowerPC/aix-dwarf.ll
@@ -65,7 +65,7 @@
 ; SEC32-NEXT:RelocationPointer: 0x1F4
 ; SEC64-NEXT:Size: 0x18
 ; SEC64-NEXT:RawDataOffset: 0x1A8
-; SEC64-NEXT:RelocationPointer: 0x2C8
+; SEC64-NEXT:RelocationPointer: 0x29C
 ; SEC-NEXT:  LineNumberPointer: 0x0
 ; SEC-NEXT:  NumberOfRelocations: 2
 ; SEC-NEXT:  NumberOfLineNumbers: 0
@@ -93,9 +93,9 @@
 ; SEC32-NEXT:Size: 0x57
 ; SEC32-NEXT:RawDataOffset: 0x15C
 ; SEC32-NEXT:RelocationPointer: 0x208
-; SEC64-NEXT:Size: 0x6F
+; SEC64-NEXT:Size: 0x5F
 ; SEC64-NEXT:RawDataOffset: 0x200
-; SEC64-NEXT:RelocationPointer: 0x2E4
+; SEC64-NEXT:RelocationPointer: 0x2B8
 ; SEC-NEXT:  LineNumberPointer: 0x0
 ; SEC-NEXT:  NumberOfRelocations: 4
 ; SEC-NEXT:  NumberOfLineNumbers: 0
@@ -109,9 +109

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

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

steakhal added inline comments.



Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:1-2
+typedef typeof(sizeof(int)) size_t;
+typedef signed long ssize_t;
+typedef struct {

donat.nagy wrote:
> balazske wrote:
> > steakhal wrote:
> > > balazske wrote:
> > > > steakhal wrote:
> > > > > `ssize_t`'s size should match the size of `size_t`. In this 
> > > > > implementation, it would be true only if `size_t` is `long`.
> > > > > 
> > > > I could not find a working way of defining the type in that way (there 
> > > > is no `__sizte_t`). The current definition should work well in the test 
> > > > code, the property of being the same size is supposedly not used in the 
> > > > tests. The previous definition was not better than this (and different 
> > > > in different places).
> > > I think [[ 
> > > https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/format-strings-scanf.c#L7-L15
> > >  | clang/test/Sema/format-strings-scanf.c ]] uses something like this: 
> > > ```lang=C++
> > > typedef __SIZE_TYPE__ size_t;
> > > #define __SSIZE_TYPE__
> > >  \
> > >   __typeof__(_Generic((__SIZE_TYPE__)0,   
> > >  \
> > >   unsigned long long int : (long long int)0,  
> > >  \
> > >   unsigned long int : (long int)0,
> > >  \
> > >   unsigned int : (int)0,  
> > >  \
> > >   unsigned short : (short)0,  
> > >  \
> > >   unsigned char : (signed char)0))
> > > typedef __SSIZE_TYPE__ ssize_t;
> > > ```
> > This may work (probably not on all platforms) but still I think in this 
> > context it is only important that we have a type called `ssize_t` and it is 
> > signed, it is less important that it is exactly the correct type. Other 
> > types in the same header are not exact, and `ssize_t` in other test code 
> > (in Analysis tests) is defined not in this way.
> I agree that we don't need that `_Generic` magic in this particular test 
> file. If we want consistency between the sizes of `size_t` and `ssize_t` then 
> you may define them as e.g. just `unsigned long long` and `long long` -- but 
> I think even that consideration is overkill.
Whatever. My problem is that this is a header. It should be included from 
individual test files. And test files are the place where the author decides if 
they want to pin the test to a specific target to make assumptions about sizes 
of types or signedness for example. So my concern is that the current form of 
theader is not portable, hence it should be includes from tests where the 
target is pinned to x86 linux. However, we dontenforce this in any way ormake 
it portable.
Having an ifdef check and error out if the target is not what we expect would 
be suboptimal as premerge bots might only run on only x86 linux. (On second 
thought there are probably windows bots so with caee it might be not as a bit 
issue). I hope I clarified my concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp:77
+  verifyFormat("@property(a, b, c) int p;", "@property(b, a, c) int p;", 
Style);
+  verifyFormat("@property(a, b, c) int p;", "@property(c, b, a) int p;", 
Style);
+}

might it be a good idea to actually test some/all of the keywords you intend to 
support? just incase some of them are not actually tok::identifiers?


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

https://reviews.llvm.org/D150083

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

[PATCH] D148490: [AIX] use system assembler for assembly files

[ChenZheng via Phabricator via cfe-commits]

shchenz updated this revision to Diff 521233.
shchenz added a comment.

update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148490

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/target-as.s


Index: clang/test/Driver/target-as.s
===
--- clang/test/Driver/target-as.s
+++ clang/test/Driver/target-as.s
@@ -1,6 +1,29 @@
 // Make sure the -march is passed down to cc1as.
 // RUN: %clang -target i386-unknown-freebsd -### -c -integrated-as %s \
-// RUN: -march=geode 2>&1 | FileCheck -check-prefix=TARGET %s
+// RUN: -march=geode 2>&1 | FileCheck -check-prefix=GEODE %s
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit.
+// RUN: %clang %s -### -c -fintegrated-as 2>&1 \
+// RUN: --target=powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS64,CHECK %s
 //
-// TARGET: "-cc1as"
-// TARGET: "-target-cpu" "geode"
+// RUN: %clang %s -### -c -fno-integrated-as 2>&1 \
+// RUN: --target=powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS64,CHECK %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit.
+// RUN: %clang %s -### -c -fintegrated-as 2>&1 \
+// RUN: --target=powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS32,CHECK %s
+//
+// RUN: %clang %s -### -c 2>&1 -fno-integrated-as \
+// RUN: --target=powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS32,CHECK %s
+
+// GEODE: "-cc1as"
+// GEODE: "-target-cpu" "geode"
+
+// CHECK: "{{.*}}as{{(.exe)?}}"
+// AIX-AS32: "-a32"
+// AIX-AS64: "-a64"
+// CHECK: "-many"
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -662,7 +662,8 @@
   if (D.IsFlangMode() && getDriver().ShouldUseFlangCompiler(JA)) return 
getFlang();
   if (getDriver().ShouldUseClangCompiler(JA)) return getClang();
   Action::ActionClass AC = JA.getKind();
-  if (AC == Action::AssembleJobClass && useIntegratedAs())
+  if (AC == Action::AssembleJobClass && useIntegratedAs() &&
+  !getTriple().isOSAIX())
 return getClangAs();
   return getTool(AC);
 }


Index: clang/test/Driver/target-as.s
===
--- clang/test/Driver/target-as.s
+++ clang/test/Driver/target-as.s
@@ -1,6 +1,29 @@
 // Make sure the -march is passed down to cc1as.
 // RUN: %clang -target i386-unknown-freebsd -### -c -integrated-as %s \
-// RUN: -march=geode 2>&1 | FileCheck -check-prefix=TARGET %s
+// RUN: -march=geode 2>&1 | FileCheck -check-prefix=GEODE %s
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit.
+// RUN: %clang %s -### -c -fintegrated-as 2>&1 \
+// RUN: --target=powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS64,CHECK %s
 //
-// TARGET: "-cc1as"
-// TARGET: "-target-cpu" "geode"
+// RUN: %clang %s -### -c -fno-integrated-as 2>&1 \
+// RUN: --target=powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS64,CHECK %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit.
+// RUN: %clang %s -### -c -fintegrated-as 2>&1 \
+// RUN: --target=powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS32,CHECK %s
+//
+// RUN: %clang %s -### -c 2>&1 -fno-integrated-as \
+// RUN: --target=powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefixes=AIX-AS32,CHECK %s
+
+// GEODE: "-cc1as"
+// GEODE: "-target-cpu" "geode"
+
+// CHECK: "{{.*}}as{{(.exe)?}}"
+// AIX-AS32: "-a32"
+// AIX-AS64: "-a64"
+// CHECK: "-many"
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -662,7 +662,8 @@
   if (D.IsFlangMode() && getDriver().ShouldUseFlangCompiler(JA)) return getFlang();
   if (getDriver().ShouldUseClangCompiler(JA)) return getClang();
   Action::ActionClass AC = JA.getKind();
-  if (AC == Action::AssembleJobClass && useIntegratedAs())
+  if (AC == Action::AssembleJobClass && useIntegratedAs() &&
+  !getTriple().isOSAIX())
 return getClangAs();
   return getTool(AC);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148490: [AIX] use system assembler for assembly files

[ChenZheng via Phabricator via cfe-commits]

shchenz added a comment.

> I am not sure you need 6 RUN lines to test this. Whether a target uses 
> integrated assembler has an existing test file and you can reuse that.

I don't have strong prefer which one we should use, a new file or reuse file. 
Since you point out, I change to reuse a legacy one in same dir. Please notice 
that there are some other targets that has their own assembler test file, like 
mips-as.s, systemz-as.s, arm64-as.s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148490

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

[PATCH] D150191: [clang][Diagnostics] Provide a source range for 'use of undeclared identifier' diagnostics

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

tbaeder updated this revision to Diff 521236.

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

https://reviews.llvm.org/D150191

Files:
  clang/include/clang/Sema/Lookup.h
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Misc/reduced-diags-macros-backtrace.cpp

Index: clang/test/Misc/reduced-diags-macros-backtrace.cpp
===
--- clang/test/Misc/reduced-diags-macros-backtrace.cpp
+++ clang/test/Misc/reduced-diags-macros-backtrace.cpp
@@ -11,37 +11,69 @@
 
 int a = LEVEL1(b);
 
+
 // ALL: {{.*}}:12:9: error: use of undeclared identifier 'p'
 // ALL-NEXT: int a = LEVEL1(b);
-// ALL-NEXT: ^
+// ALL-NEXT: ^
 // ALL-NEXT: {{.*}}:10:19: note: expanded from macro 'LEVEL1'
 // ALL-NEXT: #define LEVEL1(x) LEVEL2(x)
-// ALL-NEXT:   ^
+// ALL-NEXT:   ^
 // ALL-NEXT: {{.*}}:9:19: note: expanded from macro 'LEVEL2'
 // ALL-NEXT: #define LEVEL2(x) LEVEL3(x)
-// ALL-NEXT:   ^
+// ALL-NEXT:   ^
 // ALL-NEXT: {{.*}}:8:19: note: expanded from macro 'LEVEL3'
 // ALL-NEXT: #define LEVEL3(x) LEVEL4(x)
-// ALL-NEXT:   ^
+// ALL-NEXT:   ^
 // ALL-NEXT: {{.*}}:7:23: note: expanded from macro 'LEVEL4'
 // ALL-NEXT: #define LEVEL4(x) ADD(p,x)
 // ALL-NEXT:   ^
+// ALL-NEXT: {{.*}}:6:20: note: expanded from macro 'ADD'
+// ALL-NEXT: #define ADD(x,y) G(x) + y
+// ALL-NEXT:^
+// ALL-NEXT: {{.*}}:5:16: note: expanded from macro 'G'
+// ALL-NEXT: #define G(x) F(x) + 2
+// ALL-NEXT:^
+// ALL-NEXT: {{.*}}:4:14: note: expanded from macro 'F'
+// ALL-NEXT: #define F(x) x + 1
+// ALL-NEXT:  ^
 // ALL-NEXT: {{.*}}:12:16: error: use of undeclared identifier 'b'
 // ALL-NEXT: int a = LEVEL1(b);
 // ALL-NEXT:^
-// ALL-NEXT: 2 errors generated.
+// ALL-NEXT: {{.*}}:10:26: note: expanded from macro 'LEVEL1'
+// ALL-NEXT: #define LEVEL1(x) LEVEL2(x)
+// ALL-NEXT:  ^
+// ALL-NEXT: {{.*}}:9:26: note: expanded from macro 'LEVEL2'
+// ALL-NEXT: #define LEVEL2(x) LEVEL3(x)
+// ALL-NEXT:  ^
+// ALL-NEXT: {{.*}}:8:26: note: expanded from macro 'LEVEL3'
+// ALL-NEXT: #define LEVEL3(x) LEVEL4(x)
+// ALL-NEXT:  ^
+// ALL-NEXT: {{.*}}:7:25: note: expanded from macro 'LEVEL4'
+// ALL-NEXT: #define LEVEL4(x) ADD(p,x)
+// ALL-NEXT: ^
+// ALL-NEXT: {{.*}}:6:25: note: expanded from macro 'ADD'
+// ALL-NEXT: #define ADD(x,y) G(x) + y
+// ALL-NEXT: ^
+  // ALL-NEXT: 2 errors generated.
 
 // SKIP: {{.*}}:12:9: error: use of undeclared identifier 'p'
 // SKIP-NEXT: int a = LEVEL1(b);
-// SKIP-NEXT: ^
+// SKIP-NEXT: ^
 // SKIP-NEXT: {{.*}}:10:19: note: expanded from macro 'LEVEL1'
 // SKIP-NEXT: #define LEVEL1(x) LEVEL2(x)
-// SKIP-NEXT:   ^
-// SKIP-NEXT: note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
-// SKIP-NEXT: {{.*}}:7:23: note: expanded from macro 'LEVEL4'
-// SKIP-NEXT: #define LEVEL4(x) ADD(p,x)
-// SKIP-NEXT:   ^
+// SKIP-NEXT:   ^
+// SKIP-NEXT: note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
+// SKIP-NEXT: {{.*}}:4:14: note: expanded from macro 'F'
+// SKIP-NEXT: #define F(x) x + 1
+// SKIP-NEXT:  ^
 // SKIP-NEXT: {{.*}}:12:16: error: use of undeclared identifier 'b'
 // SKIP-NEXT: int a = LEVEL1(b);
 // SKIP-NEXT:^
+// SKIP-NEXT: {{.*}}:10:26: note: expanded from macro 'LEVEL1'
+// SKIP-NEXT: #define LEVEL1(x) LEVEL2(x)
+// SKIP-NEXT:  ^
+// SKIP-NEXT: note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
+// SKIP-NEXT: {{.*}}:6:25: note: expanded from macro 'ADD'
+// SKIP-NEXT: #define ADD(x,y) G(x) + y
+// SKIP-NEXT: ^
 // SKIP-NEXT: 2 errors generated.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2173,7 +2173,7 @@
 
 static void emitEmptyLookupTypoDiagnostic(
 const TypoCorrection &TC, Sema &SemaRef, const CXXScopeSpec &SS,
-DeclarationName Typo, SourceLocation TypoLoc, ArrayRef Args,
+DeclarationName Typo, SourceRange TypoRange, ArrayRef Args,
 unsigned DiagnosticID, unsigned DiagnosticSuggestID) {
   DeclContext *Ctx =
   SS.isEmpty() ? nullptr : SemaRef.computeDeclContext(SS, false);
@@ -2181,10 +2181,10 @@
 // Emit a special diagnostic for failed member lookups.
 // FIXME: computing the declaration context might fail here (?)
 if (Ctx)
-  SemaRef.Diag(TypoLoc, diag::err_no_member) << Typo << Ctx
- << SS.getRange();
+  SemaRef.Diag(TypoRange.getBegin(), diag::err_n

[PATCH] D150209: [clang][Interp] Add more shift error checking

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

tbaeder updated this revision to Diff 521237.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D150209

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/shifts.cpp


Index: clang/test/AST/Interp/shifts.cpp
===
--- clang/test/AST/Interp/shifts.cpp
+++ clang/test/AST/Interp/shifts.cpp
@@ -152,4 +152,21 @@
   constexpr signed int R = (sizeof(unsigned) * 8) + 1;
   constexpr decltype(L) M  = (R > 32 && R < 64) ?  L << R : 0;
   constexpr decltype(L) M2 = (R > 32 && R < 64) ?  L >> R : 0;
+
+
+  constexpr int signedShift() { // cxx17-error {{never produces a constant 
expression}} \
+// ref-cxx17-error {{never produces a constant 
expression}}
+return 1024 << 31; // cxx17-warning {{signed shift result}} \
+   // ref-cxx17-warning {{signed shift result}} \
+   // cxx17-note {{signed left shift discards bits}} \
+   // ref-cxx17-note {{signed left shift discards bits}}
+  }
+
+  constexpr int negativeShift() { // cxx17-error {{never produces a constant 
expression}} \
+  // ref-cxx17-error {{never produces a 
constant expression}}
+return -1 << 2; // cxx17-warning {{shifting a negative signed value is 
undefined}} \
+// ref-cxx17-warning {{shifting a negative signed value is 
undefined}} \
+// cxx17-note {{left shift of negative value -1}} \
+// ref-cxx17-note {{left shift of negative value -1}}
+  }
 };
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -109,8 +109,9 @@
 bool CheckCtorCall(InterpState &S, CodePtr OpPC, const Pointer &This);
 
 /// Checks if the shift operation is legal.
-template 
-bool CheckShift(InterpState &S, CodePtr OpPC, const RT &RHS, unsigned Bits) {
+template 
+bool CheckShift(InterpState &S, CodePtr OpPC, const LT &LHS, const RT &RHS,
+unsigned Bits) {
   if (RHS.isNegative()) {
 const SourceInfo &Loc = S.Current->getSource(OpPC);
 S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
@@ -126,6 +127,20 @@
 S.CCEDiag(E, diag::note_constexpr_large_shift) << Val << Ty << Bits;
 return false;
   }
+
+  if (LHS.isSigned() && !S.getLangOpts().CPlusPlus20) {
+const Expr *E = S.Current->getExpr(OpPC);
+// C++11 [expr.shift]p2: A signed left shift must have a non-negative
+// operand, and must not overflow the corresponding unsigned type.
+if (LHS.isNegative())
+  S.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
+else if (LHS.toUnsigned().countLeadingZeros() < static_cast(RHS))
+  S.CCEDiag(E, diag::note_constexpr_lshift_discards);
+  }
+
+  // C++2a [expr.shift]p2: [P0907R4]:
+  //E1 << E2 is the unique value congruent to
+  //E1 x 2^E2 module 2^N.
   return true;
 }
 
@@ -1612,7 +1627,7 @@
   const auto &LHS = S.Stk.pop();
   const unsigned Bits = LHS.bitWidth();
 
-  if (!CheckShift(S, OpPC, RHS, Bits))
+  if (!CheckShift(S, OpPC, LHS, RHS, Bits))
 return false;
 
   Integral R;
@@ -1629,7 +1644,7 @@
   const auto &LHS = S.Stk.pop();
   const unsigned Bits = LHS.bitWidth();
 
-  if (!CheckShift(S, OpPC, RHS, Bits))
+  if (!CheckShift(S, OpPC, LHS, RHS, Bits))
 return false;
 
   Integral R;
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -127,7 +127,11 @@
 return Compare(V, RHS.V);
   }
 
-  unsigned countLeadingZeros() const { return llvm::countl_zero(V); }
+  unsigned countLeadingZeros() const {
+if constexpr (!Signed)
+  return llvm::countl_zero(V);
+llvm_unreachable("Don't call countLeadingZeros() on signed types.");
+  }
 
   Integral truncate(unsigned TruncBits) const {
 if (TruncBits >= Bits)


Index: clang/test/AST/Interp/shifts.cpp
===
--- clang/test/AST/Interp/shifts.cpp
+++ clang/test/AST/Interp/shifts.cpp
@@ -152,4 +152,21 @@
   constexpr signed int R = (sizeof(unsigned) * 8) + 1;
   constexpr decltype(L) M  = (R > 32 && R < 64) ?  L << R : 0;
   constexpr decltype(L) M2 = (R > 32 && R < 64) ?  L >> R : 0;
+
+
+  constexpr int signedShift() { // cxx17-error {{never produces a constant expression}} \
+// ref-cxx17-error {{never produces a constant expression}}
+return 1024 << 31; // cxx17-warning {{signed shift result}} \
+   // ref-cxx17-warning {{signed shift result}} \
+   // cxx17-note {{signed left shift discards bits}} \
+   // ref-cxx17-note {{si

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143334

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

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

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

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



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:22-23
+
+static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
+   InterpFrame *Frame) {
+  const Pointer &A = getParam(Frame, 0);

aaron.ballman wrote:
> Thought: it would be nice if we could hoist as much of this implementation as 
> we can into a helper function so that we can share most of the guts with 
> `__builtin_memcmp()` as well.
> 
> Also, it would be good to generalize this so it works for 
> `__builtin_wcscmp()` and `__builtin_wmemcmp()` as well.
Definitely, but I think it makes more sense to do that when we implement those 
functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149816

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

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

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

tbaeder updated this revision to Diff 521239.

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

https://reviews.llvm.org/D149816

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
+
+namespace strcmp {
+  constexpr char kFoobar[6] = {'f','o','o','b','a','r'};
+  constexpr char kFoobazfoobar[12] = {'f','o','o','b','a','z','f','o','o','b','a','r'};
+
+  static_assert(__builtin_strcmp("abab", "abab") == 0);
+  static_assert(__builtin_strcmp("abab", "abba") == -1);
+  static_assert(__builtin_strcmp("abab", "abaa") == 1);
+  static_assert(__builtin_strcmp("ababa", "abab") == 1);
+  static_assert(__builtin_strcmp("abab", "ababa") == -1);
+  static_assert(__builtin_strcmp("a\203", "a") == 1);
+  static_assert(__builtin_strcmp("a\203", "a\003") == 1);
+  static_assert(__builtin_strcmp("abab\0banana", "abab") == 0);
+  static_assert(__builtin_strcmp("abab", "abab\0banana") == 0);
+  static_assert(__builtin_strcmp("abab\0banana", "abab\0canada") == 0);
+  static_assert(__builtin_strcmp(0, "abab") == 0); // expected-error {{not an integral constant}} \
+   // expected-note {{dereferenced null}} \
+   // expected-note {{in call to}} \
+   // ref-error {{not an integral constant}} \
+   // ref-note {{dereferenced null}}
+  static_assert(__builtin_strcmp("abab", 0) == 0); // expected-error {{not an integral constant}} \
+   // expected-note {{dereferenced null}} \
+   // expected-note {{in call to}} \
+   // ref-error {{not an integral constant}} \
+   // ref-note {{dereferenced null}}
+
+  static_assert(__builtin_strcmp(kFoobar, kFoobazfoobar) == -1);
+  static_assert(__builtin_strcmp(kFoobar, kFoobazfoobar + 6) == 0); // expected-error {{not an integral constant}} \
+// expected-note {{dereferenced one-past-the-end}} \
+// expected-note {{in call to}} \
+// ref-error {{not an integral constant}} \
+// ref-note {{dereferenced one-past-the-end}}
+}
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -341,7 +341,8 @@
 
   /// Dereferences a primitive element.
   template  T &elem(unsigned I) const {
-return reinterpret_cast(Pointee->rawData())[I];
+assert(I < getNumElems());
+return reinterpret_cast(Pointee->data() + sizeof(InitMap *))[I];
   }
 
   /// Initializes a field.
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -14,15 +14,64 @@
 namespace clang {
 namespace interp {
 
-bool InterpretBuiltin(InterpState &S, CodePtr &PC, unsigned BuiltinID) {
+template  T getParam(InterpFrame *Frame, unsigned Index) {
+  unsigned Offset = Frame->getFunction()->getParamOffset(Index);
+  return Frame->getParam(Offset);
+}
+
+static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
+   InterpFrame *Frame) {
+  const Pointer &A = getParam(Frame, 0);
+  const Pointer &B = getParam(Frame, 1);
+
+  if (!CheckLive(S, OpPC, A, AK_Read) || !CheckLive(S, OpPC, B, AK_Read))
+return false;
+
+  assert(A.getFieldDesc()->isPrimitiveArray());
+  assert(B.getFieldDesc()->isPrimitiveArray());
+
+  unsigned IndexA = A.getIndex();
+  unsigned IndexB = B.getIndex();
+  int32_t Result = 0;
+  for (;; ++IndexA, ++IndexB) {
+const Pointer &PA = A.atIndex(IndexA);
+const Pointer &PB = B.atIndex(IndexB);
+if (!CheckRange(S, OpPC, PA, AK_Read) ||
+!CheckRange(S, OpPC, PB, AK_Read)) {
+  return false;
+}
+uint8_t CA = PA.deref();
+uint8_t CB = PB.deref();
+
+if (CA > CB) {
+  Result = 1;
+  break;
+} else if (CA < CB) {
+  Result = -1;
+  break;
+}
+if (CA =

[PATCH] D144457: [clang][Interp] Handle global composite temporaries

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144457

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

[PATCH] D144943: [clang][Interp] Implement bitcasts (WIP)

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

tbaeder added a comment.

Pig


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

https://reviews.llvm.org/D144943

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

[PATCH] D148614: [clang][Interp] Add frame depth checking

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

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D148614

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

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

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

balazske added inline comments.



Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:1-2
+typedef typeof(sizeof(int)) size_t;
+typedef signed long ssize_t;
+typedef struct {

steakhal wrote:
> donat.nagy wrote:
> > balazske wrote:
> > > steakhal wrote:
> > > > balazske wrote:
> > > > > steakhal wrote:
> > > > > > `ssize_t`'s size should match the size of `size_t`. In this 
> > > > > > implementation, it would be true only if `size_t` is `long`.
> > > > > > 
> > > > > I could not find a working way of defining the type in that way 
> > > > > (there is no `__sizte_t`). The current definition should work well in 
> > > > > the test code, the property of being the same size is supposedly not 
> > > > > used in the tests. The previous definition was not better than this 
> > > > > (and different in different places).
> > > > I think [[ 
> > > > https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/format-strings-scanf.c#L7-L15
> > > >  | clang/test/Sema/format-strings-scanf.c ]] uses something like this: 
> > > > ```lang=C++
> > > > typedef __SIZE_TYPE__ size_t;
> > > > #define __SSIZE_TYPE__  
> > > >\
> > > >   __typeof__(_Generic((__SIZE_TYPE__)0, 
> > > >\
> > > >   unsigned long long int : (long long int)0,
> > > >\
> > > >   unsigned long int : (long int)0,  
> > > >\
> > > >   unsigned int : (int)0,
> > > >\
> > > >   unsigned short : (short)0,
> > > >\
> > > >   unsigned char : (signed char)0))
> > > > typedef __SSIZE_TYPE__ ssize_t;
> > > > ```
> > > This may work (probably not on all platforms) but still I think in this 
> > > context it is only important that we have a type called `ssize_t` and it 
> > > is signed, it is less important that it is exactly the correct type. 
> > > Other types in the same header are not exact, and `ssize_t` in other test 
> > > code (in Analysis tests) is defined not in this way.
> > I agree that we don't need that `_Generic` magic in this particular test 
> > file. If we want consistency between the sizes of `size_t` and `ssize_t` 
> > then you may define them as e.g. just `unsigned long long` and `long long` 
> > -- but I think even that consideration is overkill.
> Whatever. My problem is that this is a header. It should be included from 
> individual test files. And test files are the place where the author decides 
> if they want to pin the test to a specific target to make assumptions about 
> sizes of types or signedness for example. So my concern is that the current 
> form of theader is not portable, hence it should be includes from tests where 
> the target is pinned to x86 linux. However, we dontenforce this in any way 
> ormake it portable.
> Having an ifdef check and error out if the target is not what we expect would 
> be suboptimal as premerge bots might only run on only x86 linux. (On second 
> thought there are probably windows bots so with caee it might be not as a bit 
> issue). I hope I clarified my concerns.
The test did work with the old definition and this patch is only about 
restructuring the code. This patch is meant to be a "test NFC". But if this 
code gets no acceptance I have these possibilities:

  - Define `size_t` as `unsigned long` and `ssize_t` as `signed long`. Probably 
mention in a comment that the definitions are not always realistic.
  - Use the definition with `_Generic`. But then the other definitions in the 
POSIX header should be fixed too, for example `off_t` and `off64_t` are now the 
same type, this looks not exact. Will this work on all buildbots with probably 
different C language standard?
  - Run the tests that use this header only on x86 linux.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

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

[PATCH] D150352: [clang][dataflow] Don't analyze templated declarations.

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

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

Attempting to analyze templated code doesn't have a good cost-benefit ratio. We
have so far done a best-effort attempt at this, but maintaining this support has
an ongoing high maintenance cost because the AST for templates can violate a lot
of the invariants that otherwise hold for the AST of concrete code. As just one
example, in concrete code the operand of a UnaryOperator '*' is always a prvalue
(https://godbolt.org/z/s3e5xxMd1), but in templates this isn't true
(https://godbolt.org/z/6W9xxGvoM).

Further rationale for not analyzing templates:

- The semantics of a template itself are weakly defined; semantics can depend 
strongly on the concrete template arguments. Analyzing the template itself (as 
opposed to an instantiation) therefore has limited value.

- Analyzing templates requires a lot of special-case code that isn't necessary 
for concrete code because dependent types are hard to deal with and the AST 
violates invariants that otherwise hold for concrete code (see above).

- There's precedent in that neither Clang Static Analyzer nor the 
flow-sensitive warnings in Clang (such as uninitialized variables) support 
analyzing templates.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150352

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -68,7 +68,7 @@
   assert(Body != nullptr);
 
   auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(nullptr, *Body, AST->getASTContext()));
+  ControlFlowContext::build(Func, *Body, AST->getASTContext()));
 
   AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
   DataflowAnalysisContext DACtx(std::make_unique());
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -39,10 +39,11 @@
 using BuiltinOptions = DataflowAnalysisContext::Options;
 
 template 
-void runDataflow(llvm::StringRef Code, Matcher Match,
- DataflowAnalysisOptions Options,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- llvm::StringRef TargetFun = "target") {
+llvm::Error
+runDataflowReturnError(llvm::StringRef Code, Matcher Match,
+   DataflowAnalysisOptions Options,
+   LangStandard::Kind Std = LangStandard::lang_cxx17,
+   llvm::StringRef TargetFun = "target") {
   using ast_matchers::hasName;
   llvm::SmallVector ASTBuildArgs = {
   "-fsyntax-only", "-fno-delayed-template-parsing",
@@ -61,13 +62,21 @@
   AI.ASTBuildArgs = ASTBuildArgs;
   if (Options.BuiltinOpts)
 AI.BuiltinOptions = *Options.BuiltinOpts;
+  return checkDataflow(
+  std::move(AI),
+  /*VerifyResults=*/
+  [&Match](
+  const llvm::StringMap> &Results,
+  const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); });
+}
+
+template 
+void runDataflow(llvm::StringRef Code, Matcher Match,
+ DataflowAnalysisOptions Options,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ llvm::StringRef TargetFun = "target") {
   ASSERT_THAT_ERROR(
-  checkDataflow(
-  std::move(AI),
-  /*VerifyResults=*/
-  [&Match](const llvm::StringMap>
-   &Results,
-   const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); }),
+  runDataflowReturnError(Code, Match, Options, Std, TargetFun),
   llvm::Succeeded());
 }
 
@@ -2534,31 +2543,34 @@
   });
 }
 
-TEST(TransferTest, DerefDependentPtr) {
+TEST(TransferTest, CannotAnalyzeFunctionTemplate) {
   std::string Code = R"(
 template 
-void target(T *Foo) {
-  T &Bar = *Foo;
-  /*[[p]]*/
-}
+void target() {}
   )";
-  runDataflow(
-  Code,
-  [](const llvm::StringMap> &Results,
- ASTContext &ASTCtx) {
-ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
-const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
-
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
-
-const ValueDecl *BarDecl = findValueDecl

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

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

steakhal added inline comments.



Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:1-2
+typedef typeof(sizeof(int)) size_t;
+typedef signed long ssize_t;
+typedef struct {

balazske wrote:
> steakhal wrote:
> > donat.nagy wrote:
> > > balazske wrote:
> > > > steakhal wrote:
> > > > > balazske wrote:
> > > > > > steakhal wrote:
> > > > > > > `ssize_t`'s size should match the size of `size_t`. In this 
> > > > > > > implementation, it would be true only if `size_t` is `long`.
> > > > > > > 
> > > > > > I could not find a working way of defining the type in that way 
> > > > > > (there is no `__sizte_t`). The current definition should work well 
> > > > > > in the test code, the property of being the same size is supposedly 
> > > > > > not used in the tests. The previous definition was not better than 
> > > > > > this (and different in different places).
> > > > > I think [[ 
> > > > > https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/format-strings-scanf.c#L7-L15
> > > > >  | clang/test/Sema/format-strings-scanf.c ]] uses something like 
> > > > > this: 
> > > > > ```lang=C++
> > > > > typedef __SIZE_TYPE__ size_t;
> > > > > #define __SSIZE_TYPE__
> > > > >  \
> > > > >   __typeof__(_Generic((__SIZE_TYPE__)0,   
> > > > >  \
> > > > >   unsigned long long int : (long long int)0,  
> > > > >  \
> > > > >   unsigned long int : (long int)0,
> > > > >  \
> > > > >   unsigned int : (int)0,  
> > > > >  \
> > > > >   unsigned short : (short)0,  
> > > > >  \
> > > > >   unsigned char : (signed char)0))
> > > > > typedef __SSIZE_TYPE__ ssize_t;
> > > > > ```
> > > > This may work (probably not on all platforms) but still I think in this 
> > > > context it is only important that we have a type called `ssize_t` and 
> > > > it is signed, it is less important that it is exactly the correct type. 
> > > > Other types in the same header are not exact, and `ssize_t` in other 
> > > > test code (in Analysis tests) is defined not in this way.
> > > I agree that we don't need that `_Generic` magic in this particular test 
> > > file. If we want consistency between the sizes of `size_t` and `ssize_t` 
> > > then you may define them as e.g. just `unsigned long long` and `long 
> > > long` -- but I think even that consideration is overkill.
> > Whatever. My problem is that this is a header. It should be included from 
> > individual test files. And test files are the place where the author 
> > decides if they want to pin the test to a specific target to make 
> > assumptions about sizes of types or signedness for example. So my concern 
> > is that the current form of theader is not portable, hence it should be 
> > includes from tests where the target is pinned to x86 linux. However, we 
> > dontenforce this in any way ormake it portable.
> > Having an ifdef check and error out if the target is not what we expect 
> > would be suboptimal as premerge bots might only run on only x86 linux. (On 
> > second thought there are probably windows bots so with caee it might be not 
> > as a bit issue). I hope I clarified my concerns.
> The test did work with the old definition and this patch is only about 
> restructuring the code. This patch is meant to be a "test NFC". But if this 
> code gets no acceptance I have these possibilities:
> 
>   - Define `size_t` as `unsigned long` and `ssize_t` as `signed long`. 
> Probably mention in a comment that the definitions are not always realistic.
>   - Use the definition with `_Generic`. But then the other definitions in the 
> POSIX header should be fixed too, for example `off_t` and `off64_t` are now 
> the same type, this looks not exact. Will this work on all buildbots with 
> probably different C language standard?
>   - Run the tests that use this header only on x86 linux.
> 
> 
I can agree with your two last points. I'm fine with either of those two.

My problem with the first option is that Im afraid that the ASTContex wont 
recognize the given type alias as a size_t or similar tspes, and checkers such 
as the stdlibrary functuons checker frequently does that sort of type checking 
AFAIK. So on certain target triples it wouldnt match and either surface as 
broken tests or rotten tests down the line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

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

[PATCH] D150354: [OpenMP][MLIR][Flang][bbc][Driver] Add fopenmp-version and generate corresponding MLIR attribute

[Dominik Adamski via Phabricator via cfe-commits]

domada created this revision.
domada added reviewers: kiranchandramohan, kiranktp, dpalermo, NimishMishra, 
skatrak, agozillon, raghavendhra.
domada added projects: Flang, OpenMP, MLIR.
Herald added subscribers: bviyer, sunshaoce, Moerafaat, zero9178, bzcheeseman, 
sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, jdoerfert, guansong, yaxunl.
Herald added a reviewer: sscalpone.
Herald added a reviewer: awarzynski.
Herald added a project: All.
domada requested review of this revision.
Herald added subscribers: cfe-commits, jplehr, sstefan1, stephenneuendorffer, 
nicolasvasilache, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: nicolasvasilache.
Herald added a project: clang.

This patch adds flag -fopenmp-version to the Flang frontend and bbc tool. This 
flag is lowered to MLIR OpenMP flag attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150354

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/LangOptions.def
  flang/include/flang/Tools/CrossToolHelpers.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/omp-driver-offload.f90
  flang/test/Lower/OpenMP/rtl-flags.f90
  flang/tools/bbc/bbc.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -72,7 +72,7 @@
 DefaultValuedParameter<"bool", "false">:$assume_threads_oversubscription,
 DefaultValuedParameter<"bool", "false">:$assume_no_thread_state,
 DefaultValuedParameter<"bool", "false">:$assume_no_nested_parallelism,
-DefaultValuedParameter<"uint32_t", "45">:$openmp_device_version
+DefaultValuedParameter<"uint32_t", "50">:$openmp_device_version
   );
 
   let assemblyFormat = "`<` struct(params) `>`";
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -132,6 +132,11 @@
 // A simplified subset of the OpenMP RTL Flags from Flang, only the primary
 // positive options are available, no negative options e.g. fopen_assume* vs
 // fno_open_assume*
+static llvm::cl::opt setOpenMPVersion(
+"fopenmp-version",
+llvm::cl::desc("OpenMP standard version"),
+llvm::cl::init(50));
+
 static llvm::cl::opt setOpenMPTargetDebug(
 "fopenmp-target-debug",
 llvm::cl::desc("Enable debugging in the OpenMP offloading device RTL"),
@@ -280,7 +285,7 @@
 auto offloadModuleOpts =
 OffloadModuleOpts(setOpenMPTargetDebug, setOpenMPTeamSubscription,
   setOpenMPThreadSubscription, setOpenMPNoThreadState,
-  setOpenMPNoNestedParallelism, enableOpenMPDevice);
+  setOpenMPNoNestedParallelism, enableOpenMPDevice, setOpenMPVersion);
 setOffloadModuleInterfaceAttributes(mlirModule, offloadModuleOpts);
   }
   std::error_code ec;
Index: flang/test/Lower/OpenMP/rtl-flags.f90
===
--- flang/test/Lower/OpenMP/rtl-flags.f90
+++ flang/test/Lower/OpenMP/rtl-flags.f90
@@ -1,5 +1,7 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR
-!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-version=45 %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device -fopenmp-version=45  %s -o - | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR-VERSION
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-version=45 %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR-VERSION
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DBG-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug=111 -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DBG-EQ-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-assume-teams-oversubscription -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=TEAMS-OSUB-DEVICE-FIR
@@ -8,7 +10,9 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-assume-no-nested-parallelism -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=NEST-PAR-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug -fopenmp-assume-teams-oversubscription -fopenmp-assume-no-nested-parallelism -fopenmp-assume-threads-oversubscription -fopenmp-assume-no-thread-state -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=ALL-DEVICE-FIR
 !RUN: bbc -emit-fir -fo

[PATCH] D150354: [OpenMP][MLIR][Flang][bbc][Driver] Add fopenmp-version and generate corresponding MLIR attribute

[Dominik Adamski via Phabricator via cfe-commits]

domada updated this revision to Diff 521251.
domada added a comment.

Patch rebased


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

https://reviews.llvm.org/D150354

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/LangOptions.def
  flang/include/flang/Tools/CrossToolHelpers.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/omp-driver-offload.f90
  flang/test/Lower/OpenMP/rtl-flags.f90
  flang/tools/bbc/bbc.cpp

Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -132,6 +132,11 @@
 // A simplified subset of the OpenMP RTL Flags from Flang, only the primary
 // positive options are available, no negative options e.g. fopen_assume* vs
 // fno_open_assume*
+static llvm::cl::opt setOpenMPVersion(
+"fopenmp-version",
+llvm::cl::desc("OpenMP standard version"),
+llvm::cl::init(50));
+
 static llvm::cl::opt setOpenMPTargetDebug(
 "fopenmp-target-debug",
 llvm::cl::desc("Enable debugging in the OpenMP offloading device RTL"),
@@ -280,7 +285,7 @@
 auto offloadModuleOpts =
 OffloadModuleOpts(setOpenMPTargetDebug, setOpenMPTeamSubscription,
   setOpenMPThreadSubscription, setOpenMPNoThreadState,
-  setOpenMPNoNestedParallelism, enableOpenMPDevice);
+  setOpenMPNoNestedParallelism, enableOpenMPDevice, setOpenMPVersion);
 setOffloadModuleInterfaceAttributes(mlirModule, offloadModuleOpts);
   }
   std::error_code ec;
Index: flang/test/Lower/OpenMP/rtl-flags.f90
===
--- flang/test/Lower/OpenMP/rtl-flags.f90
+++ flang/test/Lower/OpenMP/rtl-flags.f90
@@ -1,5 +1,7 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR
-!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-version=45 %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device -fopenmp-version=45  %s -o - | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR-VERSION
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-version=45 %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR-VERSION
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DBG-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug=111 -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DBG-EQ-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-assume-teams-oversubscription -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=TEAMS-OSUB-DEVICE-FIR
@@ -8,7 +10,9 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-assume-no-nested-parallelism -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=NEST-PAR-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug -fopenmp-assume-teams-oversubscription -fopenmp-assume-no-nested-parallelism -fopenmp-assume-threads-oversubscription -fopenmp-assume-no-thread-state -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=ALL-DEVICE-FIR
 !RUN: bbc -emit-fir -fopenmp -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR
+!RUN: bbc -emit-fir -fopenmp -fopenmp-is-device -fopenmp-version=45 -o - %s | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR-VERSION
 !RUN: bbc -emit-fir -fopenmp -o - %s | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: bbc -emit-fir -fopenmp -fopenmp-version=45 -o - %s | FileCheck %s --check-prefix=DEFAULT-HOST-FIR-VERSION
 !RUN: bbc -emit-fir -fopenmp -fopenmp-target-debug=111 -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=DBG-EQ-DEVICE-FIR
 !RUN: bbc -emit-fir -fopenmp -fopenmp-assume-teams-oversubscription -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=TEAMS-OSUB-DEVICE-FIR
 !RUN: bbc -emit-fir -fopenmp -fopenmp-assume-threads-oversubscription -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=THREAD-OSUB-DEVICE-FIR
@@ -17,7 +21,9 @@
 !RUN: bbc -emit-fir -fopenmp -fopenmp-target-debug=1 -fopenmp-assume-teams-oversubscription -fopenmp-assume-no-nested-parallelism -fopenmp-assume-threads-oversubscription -fopenmp-assume-no-thread-state -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=ALL-DEVICE-FIR
 
 !DEFAULT-DEVICE-FIR: module attributes {{{.*}}, omp.flags = #omp.flags<>, omp.is_device = #omp.isdevice{{.*}}}
-!DEFAULT-HOST-FIR: module attributes {{{.*}},  omp.is_device = #omp.isdevice{{.*}}}
+!DEFAULT-DEVICE-FIR-VERSION: module attributes {{{.*}}, omp.flags = #omp.flags, omp.is_device = #omp.isdevice, omp.version = #omp.version{{.*}}
+!DEFAULT-HOST-FIR: module attributes {{{.*}},  omp.is_device = #omp.isdevice{{.*}}
+!DEFAULT-HOST-FIR-

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

In D148663#4318907 , @kadircet wrote:

> I agree with Ilya's concerns here.
>
> We deliberately don't mess with compile flags pushed over LSP. These are 
> "overrides" to whatever information we have from other sources, turning on 
> interpolation at this override layer implies we'll never fallback to other 
> sources of information (as inference will always pick a target, it doesn't 
> have a "that's too bad" threshold).
> The contract on the LSP based compile flag setting requires clients to be 
> "capable" of managing files somehow, having some mixed support is unlikely to 
> benefit other users and more likely to break things as we're changing 
> behaviour now (instead of fallback to underlying compilation database, we'll 
> have interpolation).

Thank you for the feedback, I see your point. Would you mind if I make this 
behaviour conditionally behind a command line flag?
I think most of the users never mix flags from different sources and, if they 
have CDB pushed from LSP, all flags except for fallback comes from this source 
and underlying compilation database is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148663

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

[John Brawn via Phabricator via cfe-commits]

john.brawn updated this revision to Diff 521263.
john.brawn added a comment.

Rebasing now that the patches this depends on have been committed.


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

https://reviews.llvm.org/D144654

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/builtin_redef.c
  clang/test/Preprocessor/macro-reserved.c
  clang/test/Preprocessor/macro-reserved.cpp

Index: clang/test/Preprocessor/macro-reserved.cpp
===
--- clang/test/Preprocessor/macro-reserved.cpp
+++ clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus
 
 // allowlisted definitions
Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
@@ -13,6 +14,7 @@
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
+#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
Index: clang/test/Lexer/builtin_redef.c
===
--- clang/test/Lexer/builtin_redef.c
+++ clang/test/Lexer/builtin_redef.c
@@ -1,12 +1,24 @@
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
-// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
+// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
 
 // CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __TIME__ 1234
 // CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __DATE__
+// CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __STDC__ 1
+// CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __STDC_HOSTED__
 
 // CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __TIME__ 1234
+// CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __DATE__
+// CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __STDC__ 1
 // CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __STDC_HOSTED__
 
 int n = __TIME__;
 __DATE__
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -319,15 +319,6 @@
 return Diag(MacroNameTok, diag::err_defined_macro_name);
   }
 
-  if (isDefineUndef == MU_Undef) {
-auto *MI = getMacroInfo(II);
-if (MI && MI->isBuiltinMacro()) {
-  // Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4
-  // and C++ [cpp.predefined]p4], but allow it as an extension.
-  Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
-}
-  }
-
   // If defining/undefining reserved identifier or a keyword, we need to issue
   // a warning.
   SourceLocation MacroNameLoc = MacroNameTok.getLocation();
@@ -3012,6 +3003,12 @@
   MI->setTokens(Tokens, BP);
   return MI;
 }
+
+static bool isObjCProtectedMacro(const IdentifierInfo *II) {
+  return II->isStr("__strong") || II->isStr("__weak") ||
+ II->isStr("__unsafe_unretained") || II->isStr("__autoreleasing");
+}
+
 /// HandleDefineDirective - Implements \#define.  This consumes the entire macro
 /// line then lets the caller lex the next real token.
 void Preprocessor::HandleDefineDirective(
@@ -3083,15 +3080,9 @@
 // In Objective-C, ignore attempts to directly redefine the builtin
 // definitions of the ownership qualifiers.  It's still possible to
 // #undef them.
-auto isObjCProtectedMacro = [](const IdentifierInfo *II) -> bool {
-  return II->isStr("__strong") ||
- II->isStr("__weak") ||
- II->isStr("__unsafe_unretained") ||
- II->isStr("__autoreleasing");
-};
-   if (getLangOpts().ObjC &&
-SourceMgr.getFileID(OtherMI->getDefinitionLoc())
-  == getPredefinesFileID() &&
+if (getLangOpts().ObjC &&
+SourceMgr.getFileID(OtherMI->getDefinitio

[PATCH] D149986: AMDGPU: Force sc0 and sc1 on stores for gfx940 and gfx941

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

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

In D149986#4334274 , @Pierre-vh wrote:

> I think that if this is a new property of the GFX940/941 targets, and turning 
> it off shouldn't be possible, we shouldn't even bother with a feature and 
> just set a bool in the ST for those targets

AFAIK, the subtarget only knows the generation and has access to features. The 
subtarget cannot differentiate between gfx940, gfx941, gfx942.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149986

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

[PATCH] D149838: [clang][dataflow] Eliminate `SkipPast::ReferenceThenPointer`.

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

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

Rebase to head


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149838

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -372,8 +372,7 @@
   auto *Object = E->getImplicitObjectArgument();
   assert(Object != nullptr);
 
-  auto *ObjectLoc =
-  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  auto *ObjectLoc = getImplicitObjectLocation(*E, Env);
   assert(ObjectLoc != nullptr);
 
   auto &ConstructorVal = *Env.createValue(Object->getType());
@@ -532,8 +531,7 @@
   auto *Object = E->getArg(0);
   assert(Object != nullptr);
 
-  auto *ObjectLoc =
-  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  auto *ObjectLoc = Env.getStorageLocation(*Object, SkipPast::Reference);
   assert(ObjectLoc != nullptr);
 
   auto &ConstructorVal = *Env.createValue(Object->getType());
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -542,10 +542,7 @@
   }
 }
 
-// The receiver can be either a value or a pointer to a value. Skip past the
-// indirection to handle both cases.
-auto *BaseLoc = cast_or_null(
-Env.getStorageLocation(*S->getBase(), SkipPast::ReferenceThenPointer));
+AggregateStorageLocation *BaseLoc = getBaseObjectLocation(*S, Env);
 if (BaseLoc == nullptr)
   return;
 
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -372,10 +372,26 @@
   return HasValueVal != nullptr && Env.flowConditionImplies(*HasValueVal);
 }
 
+StorageLocation *maybeSkipPointer(StorageLocation *Loc,
+  const Environment &Env) {
+  if (Loc == nullptr)
+return nullptr;
+  if (auto *Val = dyn_cast_or_null(Env.getValue(*Loc)))
+return &Val->getPointeeLoc();
+  return Loc;
+}
+
+Value *getValueBehindPossiblePointer(const Expr &E, const Environment &Env) {
+  Value *Val = Env.getValue(E, SkipPast::Reference);
+  if (auto *PointerVal = dyn_cast_or_null(Val))
+return Env.getValue(PointerVal->getPointeeLoc());
+  return Val;
+}
+
 void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
 LatticeTransferState &State) {
   if (auto *OptionalVal =
-  State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) {
+  getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {
 if (State.Env.getStorageLocation(*UnwrapExpr, SkipPast::None) == nullptr)
   if (auto *Loc = maybeInitializeOptionalValueMember(
   UnwrapExpr->getType(), *OptionalVal, State.Env))
@@ -396,8 +412,8 @@
   const MatchFinder::MatchResult &,
   LatticeTransferState &State) {
   if (auto *HasValueVal = getHasValue(
-  State.Env, State.Env.getValue(*CallExpr->getImplicitObjectArgument(),
-SkipPast::ReferenceThenPointer))) {
+  State.Env, getValueBehindPossiblePointer(
+ *CallExpr->getImplicitObjectArgument(), State.Env))) {
 auto &CallExprLoc = State.Env.createStorageLocation(*CallExpr);
 State.Env.setValue(CallExprLoc, *HasValueVal);
 State.Env.setStorageLocation(*CallExpr, CallExprLoc);
@@ -419,8 +435,7 @@
   ->getImplicitObjectArgument();
 
   auto *HasValueVal = getHasValue(
-  State.Env,
-  State.Env.getValue(*ObjectArgumentExpr, SkipPast::ReferenceThenPointer));
+  State.Env, getValueBehindPossiblePointer(*ObjectArgumentExpr, State.Env));
   if (HasValueVal == nullptr)
 return;
 
@@ -472,8 +487,8 @@
 
 void assignOptionalValue(const Expr &E, Environment &Env,
  BoolValue &HasValueVal) {
-  if (auto *OptionalLoc =
-  Env.getStorageLocation(E, SkipPast::ReferenceThenPointer)) {
+  if (auto *OptionalLoc = maybeSkipPointer(
+  Env.getStorageLocation(E, SkipPast:

[PATCH] D150358: [clang][Interp] Remove args from called functions in more cases

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

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  When calling functions in the checkingPotentialConstantExpression mode,
  we cannot have arguments (including This + RVO pointers) for the
  toplevel callee, but the functions called from within can work just
  fine, or at least we succeed in pushing their arguments on the stack, so
  we must also succeed in removing them again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150358

Files:
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/functions.cpp


Index: clang/test/AST/Interp/functions.cpp
===
--- clang/test/AST/Interp/functions.cpp
+++ clang/test/AST/Interp/functions.cpp
@@ -257,3 +257,11 @@
// ref-note {{in call to 'SS()'}}
 
 }
+
+namespace CallWithArgs {
+  /// This used to call problems during checkPotentialConstantExpression() 
runs.
+  constexpr void g(int a) {}
+  constexpr void f() {
+g(0);
+  }
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -65,12 +65,12 @@
   switch (F->getBuiltinID()) {
   case Builtin::BI__builtin_is_constant_evaluated:
 S.Stk.push(Boolean::from(S.inConstantContext()));
-return Ret(S, OpPC, Dummy);
+return Ret(S, OpPC, Dummy);
   case Builtin::BI__builtin_assume:
-return RetVoid(S, OpPC, Dummy);
+return RetVoid(S, OpPC, Dummy);
   case Builtin::BI__builtin_strcmp:
 if (interp__builtin_strcmp(S, OpPC, Frame))
-  return Ret(S, OpPC, Dummy);
+  return Ret(S, OpPC, Dummy);
 return false;
   default:
 return false;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -192,13 +192,12 @@
 // Returning values
 
//===--===//
 
-template ::T>
+template ::T>
 bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   const T &Ret = S.Stk.pop();
 
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (Builtin || !S.checkingPotentialConstantExpression())
+  if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
 S.Current->popArgs();
 
   if (InterpFrame *Caller = S.Current->Caller) {
@@ -215,10 +214,9 @@
   return true;
 }
 
-template 
 inline bool RetVoid(InterpState &S, CodePtr &PC, APValue &Result) {
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (Builtin || !S.checkingPotentialConstantExpression())
+  if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
 S.Current->popArgs();
 
   if (InterpFrame *Caller = S.Current->Caller) {


Index: clang/test/AST/Interp/functions.cpp
===
--- clang/test/AST/Interp/functions.cpp
+++ clang/test/AST/Interp/functions.cpp
@@ -257,3 +257,11 @@
// ref-note {{in call to 'SS()'}}
 
 }
+
+namespace CallWithArgs {
+  /// This used to call problems during checkPotentialConstantExpression() runs.
+  constexpr void g(int a) {}
+  constexpr void f() {
+g(0);
+  }
+}
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -65,12 +65,12 @@
   switch (F->getBuiltinID()) {
   case Builtin::BI__builtin_is_constant_evaluated:
 S.Stk.push(Boolean::from(S.inConstantContext()));
-return Ret(S, OpPC, Dummy);
+return Ret(S, OpPC, Dummy);
   case Builtin::BI__builtin_assume:
-return RetVoid(S, OpPC, Dummy);
+return RetVoid(S, OpPC, Dummy);
   case Builtin::BI__builtin_strcmp:
 if (interp__builtin_strcmp(S, OpPC, Frame))
-  return Ret(S, OpPC, Dummy);
+  return Ret(S, OpPC, Dummy);
 return false;
   default:
 return false;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -192,13 +192,12 @@
 // Returning values
 //===--===//
 
-template ::T>
+template ::T>
 bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   const T &Ret = S.Stk.pop();
 
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (Builtin || !S.checkingPotentialConstantExpression())
+  if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
 S.Current->popArgs();
 
   if (InterpFrame *Caller = S.Current->Caller) {
@@ -215,10 

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, mostly LG at the high level i think one thing we're missing is the 
ability to suppress analysis for certain headers, similar to what we have in 
clangd config options. can you add a check option for that?




Comment at: clang-tools-extra/clang-tidy/CMakeLists.txt:10
 include_directories(BEFORE ${CMAKE_CURRENT_BINARY_DIR})
+include_directories(BEFORE 
"${CMAKE_CURRENT_SOURCE_DIR}/../include-cleaner/include")
 

can you move this into `clang-tools-extra/clang-tidy/misc/CMakeLists.txt` ? in 
theory it isn't needed by all the checks



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:49
+void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
+  TUDecl = const_cast(Result.Nodes.getNodeAs("top"));
+  SM = Result.SourceManager;

rather than storing state in `::check` and running logic in 
`::onEndOfTranslationUnit`, you can just trigger analysis & finding generation 
here as we already match only on the translation unit decl.



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:57
+  std::vector Missing;
+  walkUsed(TUDecl, RecordedPreprocessor.MacroReferences, &RecordedPI, *SM,
+   [&](const include_cleaner::SymbolReference &Ref,

TUDecl can have children that are not part of the main file and we'll just run 
analysis on these decls for nothing as we discard references spelled outside 
mainfile inside `walkUsed`.

so can you go over the children of TUDecl, and only pick the ones that are 
spelled inside the mainfile and feed them as analysis roots?



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:93
+  // header mappings. But that's not different than rest of the places.
+  if (MainFile->tryGetRealPathName().endswith(PHeader))
+continue;

you can use `ClangTidyCheck::getCurrentMainFile()` instead of 
`Mainfile->tryGetRealPathName()`



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:104
+
+diag(Inc->HashLocation, Description) << FixItHint::CreateRemoval(
+{Inc->HashLocation,

you can have: `diag(Inc->HashLocation, "unused include %0") << Inc->quote() << 
FixItHint::CreateRemoval(..)` instead of the string concat above.
(also convention is to have diagnostic messages that start with lower case 
letters)



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:106-109
+ SM->getComposedLoc(SM->getMainFileID(),
+SM->getDecomposedLoc(Inc->HashLocation).second +
+std::string("#include").length() +
+Inc->quote().length() + 1)});

you can use `tooling::HeaderIncludes` not only for insertions, but also for 
removals.



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:113
+  auto FileStyle = format::getStyle(
+  format::DefaultFormatStyle, MainFile->tryGetRealPathName(),
+  format::DefaultFallbackStyle, SM->getBufferData(SM->getMainFileID()),

again you can use `ClangTidyCheck::getCurrentMainFile()`



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:114
+  format::DefaultFormatStyle, MainFile->tryGetRealPathName(),
+  format::DefaultFallbackStyle, SM->getBufferData(SM->getMainFileID()),
+  &SM->getFileManager().getVirtualFileSystem());

nit: i'd extract `SM->getBufferData(SM->getMainFileID())` into a 
`llvm::StringRef Code` and use it in both places



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:116
+  &SM->getFileManager().getVirtualFileSystem());
+  if (!FileStyle) {
+FileStyle = format::getLLVMStyle();

no need for braces



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:123-124
+
+if (MainFile->getName().empty())
+  continue;
+tooling::HeaderIncludes HeaderIncludes(

again you can use `ClangTidyCheck::getCurrentMainFile()` and drop the check



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:125
+  continue;
+tooling::HeaderIncludes HeaderIncludes(
+MainFile->getName(), SM->getBufferData(SM->getMainFileID()),

constructor of `tooling::HeaderIncludes` parses the code, so let's create it 
once outside the loop?



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:139
+
+diag(Inc.SymRefLocation, Description) << FixItHint::CreateInsertion(
+SM->getComposedLoc(SM->getMainFileID(), Replacement->getOffset()),

you can use the substitution alternative mentioned above here as well



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerChe

[PATCH] D150354: [OpenMP][MLIR][Flang][bbc][Driver] Add fopenmp-version and generate corresponding MLIR attribute

[Dominik Adamski via Phabricator via cfe-commits]

domada updated this revision to Diff 521282.
domada added a comment.

Patch rebased + clang format


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

https://reviews.llvm.org/D150354

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/LangOptions.def
  flang/include/flang/Tools/CrossToolHelpers.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/omp-driver-offload.f90
  flang/test/Lower/OpenMP/rtl-flags.f90
  flang/tools/bbc/bbc.cpp

Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -132,6 +132,11 @@
 // A simplified subset of the OpenMP RTL Flags from Flang, only the primary
 // positive options are available, no negative options e.g. fopen_assume* vs
 // fno_open_assume*
+static llvm::cl::opt
+setOpenMPVersion("fopenmp-version",
+ llvm::cl::desc("OpenMP standard version"),
+ llvm::cl::init(50));
+
 static llvm::cl::opt setOpenMPTargetDebug(
 "fopenmp-target-debug",
 llvm::cl::desc("Enable debugging in the OpenMP offloading device RTL"),
@@ -277,10 +282,10 @@
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   if (enableOpenMP) {
-auto offloadModuleOpts =
-OffloadModuleOpts(setOpenMPTargetDebug, setOpenMPTeamSubscription,
-  setOpenMPThreadSubscription, setOpenMPNoThreadState,
-  setOpenMPNoNestedParallelism, enableOpenMPDevice);
+auto offloadModuleOpts = OffloadModuleOpts(
+setOpenMPTargetDebug, setOpenMPTeamSubscription,
+setOpenMPThreadSubscription, setOpenMPNoThreadState,
+setOpenMPNoNestedParallelism, enableOpenMPDevice, setOpenMPVersion);
 setOffloadModuleInterfaceAttributes(mlirModule, offloadModuleOpts);
   }
   std::error_code ec;
Index: flang/test/Lower/OpenMP/rtl-flags.f90
===
--- flang/test/Lower/OpenMP/rtl-flags.f90
+++ flang/test/Lower/OpenMP/rtl-flags.f90
@@ -1,5 +1,7 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR
-!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-version=45 %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device -fopenmp-version=45  %s -o - | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR-VERSION
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-version=45 %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR-VERSION
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DBG-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug=111 -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DBG-EQ-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-assume-teams-oversubscription -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=TEAMS-OSUB-DEVICE-FIR
@@ -8,7 +10,9 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-assume-no-nested-parallelism -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=NEST-PAR-DEVICE-FIR
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-target-debug -fopenmp-assume-teams-oversubscription -fopenmp-assume-no-nested-parallelism -fopenmp-assume-threads-oversubscription -fopenmp-assume-no-thread-state -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=ALL-DEVICE-FIR
 !RUN: bbc -emit-fir -fopenmp -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR
+!RUN: bbc -emit-fir -fopenmp -fopenmp-is-device -fopenmp-version=45 -o - %s | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR-VERSION
 !RUN: bbc -emit-fir -fopenmp -o - %s | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: bbc -emit-fir -fopenmp -fopenmp-version=45 -o - %s | FileCheck %s --check-prefix=DEFAULT-HOST-FIR-VERSION
 !RUN: bbc -emit-fir -fopenmp -fopenmp-target-debug=111 -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=DBG-EQ-DEVICE-FIR
 !RUN: bbc -emit-fir -fopenmp -fopenmp-assume-teams-oversubscription -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=TEAMS-OSUB-DEVICE-FIR
 !RUN: bbc -emit-fir -fopenmp -fopenmp-assume-threads-oversubscription -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=THREAD-OSUB-DEVICE-FIR
@@ -17,7 +21,9 @@
 !RUN: bbc -emit-fir -fopenmp -fopenmp-target-debug=1 -fopenmp-assume-teams-oversubscription -fopenmp-assume-no-nested-parallelism -fopenmp-assume-threads-oversubscription -fopenmp-assume-no-thread-state -fopenmp-is-device -o - %s | FileCheck %s --check-prefix=ALL-DEVICE-FIR
 
 !DEFAULT-DEVICE-FIR: module attributes {{{.*}}, omp.flags = #omp.flags<>, omp.is_device = #omp.isdevice{{.*}}}
-!DEFAUL

[clang] 9dd2af0 - Fix CRTP partial specialization instantiation crash.

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2023-05-11T06:11:43-07:00
New Revision: 9dd2af0b4caeaa3d19d9f48891e59bc5e0877544

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

LOG: Fix CRTP partial specialization instantiation crash.

Fixes #60778.

When instantiating the body of a class template specialization that was
instantiated from a partial specialization, we were incorrectly
collecting template arguments from the primary template, which resulted
in the template arguments list being inaccurate.  In the example from
the issue, we were trying to substitute the boolean 'false' into the
type on Nested, which caused an assertion.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaTemplate/partial-spec-instantiate.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8b6232a6b9e6f..7086d337304b1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -391,6 +391,10 @@ Bug Fixes in This Version
   at the point where it is required. This fixes:
   (`#62224 `_) and
   (`#62596 `_)
+- Fix an assertion when instantiating the body of a Class Template 
Specialization
+  when it had been instantiated from a partial template specialization with 
diff erent
+  template arguments on the containing class. This fixes:
+  (`#60778 `_).
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 3d5da5f8d8284..32a8ba1a5c46d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -163,6 +163,14 @@ HandleClassTemplateSpec(const 
ClassTemplateSpecializationDecl *ClassTemplSpec,
 assert(ClassTemplSpec->getSpecializedTemplate() && "No class template?");
 if (ClassTemplSpec->getSpecializedTemplate()->isMemberSpecialization())
   return Response::Done();
+
+// If this was instantiated from a partial template specialization, we need
+// to get the next level of declaration context from the partial
+// specialization, as the ClassTemplateSpecializationDecl's
+// DeclContext/LexicalDeclContext will be for the primary template.
+if (auto *InstFromPartialTempl = 
ClassTemplSpec->getSpecializedTemplateOrPartial()
+  .dyn_cast())
+  return 
Response::ChangeDecl(InstFromPartialTempl->getLexicalDeclContext());
   }
   return Response::UseNextDecl(ClassTemplSpec);
 }

diff  --git a/clang/test/SemaTemplate/partial-spec-instantiate.cpp 
b/clang/test/SemaTemplate/partial-spec-instantiate.cpp
index 369ff69aa3756..c457c03baba0f 100644
--- a/clang/test/SemaTemplate/partial-spec-instantiate.cpp
+++ b/clang/test/SemaTemplate/partial-spec-instantiate.cpp
@@ -134,3 +134,23 @@ namespace IgnorePartialSubstitution {
 
   _Static_assert(S::value, "");
 }
+
+namespace GH60778 {
+  template  class ClassTemplate {
+  public:
+  template  class Nested {};
+  };
+
+  template  class Base {};
+
+  template <>
+  template 
+  class ClassTemplate<>::Nested : public Base::Nested > 
{};
+
+  void use() {
+// This should instantiate the body of Nested with the template arguments
+// from the Partial Specialization. This would previously get confused and
+// get the template arguments from the primary template instead.
+ClassTemplate<>::Nested instantiation;
+  }
+}



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

[PATCH] D150285: Fix CRTP partial specialization instantiation crash.

[Erich Keane 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 rG9dd2af0b4cae: Fix CRTP partial specialization instantiation 
crash. (authored by erichkeane).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D150285?vs=521049&id=521283#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150285

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaTemplate/partial-spec-instantiate.cpp


Index: clang/test/SemaTemplate/partial-spec-instantiate.cpp
===
--- clang/test/SemaTemplate/partial-spec-instantiate.cpp
+++ clang/test/SemaTemplate/partial-spec-instantiate.cpp
@@ -134,3 +134,23 @@
 
   _Static_assert(S::value, "");
 }
+
+namespace GH60778 {
+  template  class ClassTemplate {
+  public:
+  template  class Nested {};
+  };
+
+  template  class Base {};
+
+  template <>
+  template 
+  class ClassTemplate<>::Nested : public Base::Nested > 
{};
+
+  void use() {
+// This should instantiate the body of Nested with the template arguments
+// from the Partial Specialization. This would previously get confused and
+// get the template arguments from the primary template instead.
+ClassTemplate<>::Nested instantiation;
+  }
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -163,6 +163,14 @@
 assert(ClassTemplSpec->getSpecializedTemplate() && "No class template?");
 if (ClassTemplSpec->getSpecializedTemplate()->isMemberSpecialization())
   return Response::Done();
+
+// If this was instantiated from a partial template specialization, we need
+// to get the next level of declaration context from the partial
+// specialization, as the ClassTemplateSpecializationDecl's
+// DeclContext/LexicalDeclContext will be for the primary template.
+if (auto *InstFromPartialTempl = 
ClassTemplSpec->getSpecializedTemplateOrPartial()
+  .dyn_cast())
+  return 
Response::ChangeDecl(InstFromPartialTempl->getLexicalDeclContext());
   }
   return Response::UseNextDecl(ClassTemplSpec);
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -391,6 +391,10 @@
   at the point where it is required. This fixes:
   (`#62224 `_) and
   (`#62596 `_)
+- Fix an assertion when instantiating the body of a Class Template 
Specialization
+  when it had been instantiated from a partial template specialization with 
different
+  template arguments on the containing class. This fixes:
+  (`#60778 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaTemplate/partial-spec-instantiate.cpp
===
--- clang/test/SemaTemplate/partial-spec-instantiate.cpp
+++ clang/test/SemaTemplate/partial-spec-instantiate.cpp
@@ -134,3 +134,23 @@
 
   _Static_assert(S::value, "");
 }
+
+namespace GH60778 {
+  template  class ClassTemplate {
+  public:
+  template  class Nested {};
+  };
+
+  template  class Base {};
+
+  template <>
+  template 
+  class ClassTemplate<>::Nested : public Base::Nested > {};
+
+  void use() {
+// This should instantiate the body of Nested with the template arguments
+// from the Partial Specialization. This would previously get confused and
+// get the template arguments from the primary template instead.
+ClassTemplate<>::Nested instantiation;
+  }
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -163,6 +163,14 @@
 assert(ClassTemplSpec->getSpecializedTemplate() && "No class template?");
 if (ClassTemplSpec->getSpecializedTemplate()->isMemberSpecialization())
   return Response::Done();
+
+// If this was instantiated from a partial template specialization, we need
+// to get the next level of declaration context from the partial
+// specialization, as the ClassTemplateSpecializationDecl's
+// DeclContext/LexicalDeclContext will be for the primary template.
+if (auto *InstFromPartialTempl = ClassTemplSpec->getSpecializedTemplateOrPartial()
+  .dyn_cast())
+  return Response::ChangeDecl(InstFromPartialTempl->getLexicalDeclContext());
   }
   return Response::UseNextDecl(ClassTemplSpec);
 }
In

[PATCH] D149838: [clang][dataflow] Eliminate `SkipPast::ReferenceThenPointer`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:840
+return nullptr;
+  } else {
+return cast(Loc);

nit: no need for `else` since `if` branch always returns


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149838

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

[clang] 4bc75f0 - Fixed NATVIS debug visualizers for Clang

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-05-11T09:28:57-04:00
New Revision: 4bc75f046018bcc81ede90d548f95da8ed14f1a9

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

LOG: Fixed NATVIS debug visualizers for Clang

This fixes the visualizers for:
Type
DeclContext
QualType
TypedefNameDecl
NestedNameSpecifier
FunctionDecl

and adds visualizers for:
VariableArrayType
ElaboratedType
ParenType
BitIntType

Added: 


Modified: 
clang/utils/ClangVisualizers/clang.natvis

Removed: 




diff  --git a/clang/utils/ClangVisualizers/clang.natvis 
b/clang/utils/ClangVisualizers/clang.natvis
index 3e6148cd5e05c..ff949ce490d19 100644
--- a/clang/utils/ClangVisualizers/clang.natvis
+++ b/clang/utils/ClangVisualizers/clang.natvis
@@ -25,11 +25,16 @@ For later versions of Visual Studio, no setup is required-->
 
 {*(clang::BuiltinType *)this}
 {*(clang::PointerType *)this}
+{*(clang::ParenType *)this}
+{(clang::BitIntType *)this}
 {*(clang::LValueReferenceType *)this}
 {*(clang::RValueReferenceType *)this}
 {(clang::ConstantArrayType *)this,na}
 {(clang::ConstantArrayType 
*)this,view(left)na}
 {(clang::ConstantArrayType 
*)this,view(right)na}
+{(clang::VariableArrayType *)this,na}
+{(clang::VariableArrayType 
*)this,view(left)na}
+{(clang::VariableArrayType 
*)this,view(right)na}
 {(clang::IncompleteArrayType *)this,na}
 {(clang::IncompleteArrayType 
*)this,view(left)na}
 {(clang::IncompleteArrayType 
*)this,view(right)na}
@@ -39,6 +44,9 @@ For later versions of Visual Studio, no setup is required-->
 {(clang::DecayedType *)this,na}
 {(clang::DecayedType *)this,view(left)na}
 {(clang::DecayedType *)this,view(right)na}
+{(clang::ElaboratedType *)this,na}
+{(clang::ElaboratedType *)this,view(left)na}
+{(clang::ElaboratedType 
*)this,view(right)na}
 {*(clang::TemplateTypeParmType *)this}
 {*(clang::TemplateTypeParmType 
*)this,view(cpp)}
 {*(clang::SubstTemplateTypeParmType *)this}
@@ -59,25 +67,17 @@ For later versions of Visual Studio, no setup is required-->
 {*this,view(cpp)}
 
 No visualizer yet for 
{(clang::Type::TypeClass)TypeBits.TC,en}Type 
-Dependent{" ",sb}
-
-InstantiationDependent{" 
",sb}
-
-VariablyModified{" ",sb}
-
-ContainsUnexpandedParameterPack{"
 ",sb}
-
+Dependence{" ",en}
+
 CachedLinkage: 
{(clang::Linkage)TypeBits.CachedLinkage,en} CachedLocalOrUnnamed
 CachedLinkage: 
{(clang::Linkage)TypeBits.CachedLinkage,en}{" ",sb}
 
 FromAST
 
-
+
   No TypeBits set beyond TypeClass
 
-
-{*this, view(Dependent)}{*this, view(InstantiationDependent)}{*this, 
view(VariablyModified)}
-{*this, view(ContainsUnexpandedParameterPack)}{*this, view(Cache)}{*this, 
view(FromAST)}
+{*this, view(Dependence)}{*this, 
view(Cache)}{*this, view(FromAST)}
 {*this,view(cmn)}  {{{*this,view(poly)}}}
 
   (clang::Type::TypeClass)TypeBits.TC
@@ -85,12 +85,16 @@ For later versions of Visual Studio, no setup is required-->
   CanonicalType
   *(clang::BuiltinType 
*)this
   *(clang::PointerType 
*)this
+  *(clang::ParenType*)this
+  *(clang::BitIntType*)this
   *(clang::LValueReferenceType
 *)this
   *(clang::RValueReferenceType
 *)this
   (clang::ConstantArrayType
 *)this
+  (clang::VariableArrayType
 *)this
   (clang::IncompleteArrayType
 *)this
   *(clang::AttributedType
 *)this
   (clang::DecayedType 
*)this
+  (clang::ElaboratedType
 *)this
   (clang::TemplateTypeParmType
 *)this
   (clang::SubstTemplateTypeParmType
 *)this
   (clang::RecordType 
*)this
@@ -125,6 +129,15 @@ For later versions of Visual Studio, no setup is 
required-->
   (clang::ArrayType *)this
 
   
+  
+{ElementType,view(cpp)}
+[*]
+{ElementType,view(cpp)}[*]
+
+  (clang::Expr *)SizeExpr
+  (clang::ArrayType *)this
+
+  
   
 {Decl,view(name)nd}
 {Decl}
@@ -140,6 +153,21 @@ For later versions of Visual Studio, no setup is 
required-->
   *(clang::Type *)this, view(cmn)
 
   
+  
+{Inner, view(cpp)}
+
+  Inner
+  *(clang::Type *)this, view(cmn)
+
+  
+  
+signed 
_BitInt({NumBits})
+unsigned 
_BitInt({NumBits})(
+
+  NumBits
+  (clang::Type *)this, view(cmn)
+
+  
   
@@ -171,7 +199,7 @@ For later versions of Visual Studio, no setup is required-->
 
   
 FirstDecl
-(clang::Decl *)(NextInContextAndBits.Value & 
~3)
+(clang::Decl *)(*(intptr_t 
*)NextInContextAndBits.Value.Data & ~3)
 *this
   
 
@@ -213,15 +241,15 @@ For later versions of Visual Studio, no setup is 
required-->
   
   
   
-{(clang::T

[PATCH] D150364: [clang][Interp] Add 'Unsupported' opcode and use it for throw stmts

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

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

As discussed the other day. This fixes the diagnostics for the test case in 
`records.cpp`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150364

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/records.cpp
  clang/test/AST/Interp/unsupported.cpp

Index: clang/test/AST/Interp/unsupported.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/unsupported.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -fcxx-exceptions -verify=ref %s
+
+
+
+namespace Throw {
+
+  constexpr int ConditionalThrow(bool t) {
+if (t)
+  throw 4; // expected-note {{subexpression not valid in a constant expression}} \
+   // ref-note {{subexpression not valid in a constant expression}}
+
+return 0;
+  }
+
+  static_assert(ConditionalThrow(false) == 0, "");
+  static_assert(ConditionalThrow(true) == 0, ""); // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'ConditionalThrow(true)'}} \
+  // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to 'ConditionalThrow(true)'}}
+
+  constexpr int Throw() { // expected-error {{never produces a constant expression}} \
+  // ref-error {{never produces a constant expression}}
+throw 5; // expected-note {{subexpression not valid in a constant expression}} \
+ // ref-note {{subexpression not valid in a constant expression}}
+return 0;
+  }
+}
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -328,7 +328,8 @@
   struct S {
 constexpr S() {}
 constexpr ~S() noexcept(false) { throw 12; } // expected-error {{cannot use 'throw'}} \
- // expected-note {{declared here}} \
+ // expected-error {{never produces a constant expression}} \
+ // expected-note 2{{subexpression not valid}} \
  // ref-error {{cannot use 'throw'}} \
  // ref-error {{never produces a constant expression}} \
  // ref-note 2{{subexpression not valid}}
@@ -336,7 +337,9 @@
 
   constexpr int f() {
 S{}; // ref-note {{in call to '&S{}->~S()'}}
-return 12; // expected-note {{undefined function '~S'}}
+
+/// FIXME: Wrong source location below.
+return 12; // expected-note {{in call to '&S{}->~S()'}}
   }
   static_assert(f() == 12); // expected-error {{not an integral constant expression}} \
 // expected-note {{in call to 'f()'}} \
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -627,3 +627,6 @@
   let Types = [AllTypeClass];
   let HasGroup = 1;
 }
+
+// [] -> []
+def Unsupported : Opcode {}
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1823,6 +1823,14 @@
   return true;
 }
 
+/// Just emit a diagnostic. The expression that caused emission of this
+/// op is not valid in a constant context.
+inline bool Unsupported(InterpState &S, CodePtr OpPC) {
+  const SourceLocation &Loc = S.Current->getLocation(OpPC);
+  S.FFDiag(Loc, diag::note_invalid_subexpr_in_const_expr);
+  return false;
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -97,6 +97,7 @@
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
   bool VisitPredefinedExpr(const PredefinedExpr *E);
+  bool VisitCXXThrowExpr(const CXXThrowExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
==

[PATCH] D150364: [clang][Interp] Add 'Unsupported' opcode and use it for throw stmts

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I've unfortunately not caught up enough to comment on the approach, but 'inline 
assembly' has the same restriction, so you might find `MsAsmStmt` and 
`GCCAsmStmt` to be equally as easy 'wins'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150364

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

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150013

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

[PATCH] D127910: [Clang][AArch64][SME] Add vector load/store (ld1/st1) intrinsics

[Bryan Chan via Phabricator via cfe-commits]

bryanpkc added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:726
 
-if (Feature == "+sme") {
-  HasSME = true;

sdesmalen wrote:
> Why did you remove this?
The `Feature == "+sme"` case is also handled below on line 782. Perhaps we 
should delete this block and add `HasFullFP16` below.



Comment at: clang/test/CodeGen/aarch64-sme-intrinsics/acle_sme_ld1.c:16
+//
+__attribute__((arm_streaming)) void test_svld1_hor_za8(uint32_t slice_base, 
svbool_t pg, const void *ptr) {
+  svld1_hor_za8(0, slice_base, 0, pg, ptr);

sdesmalen wrote:
> Hi @bryanpkc, would you be happy to remove the dependence on the attributes 
> patch for now, so that we can move forward to review/land this patch series?
> 
> I'm thinking of doing something like:
> 
>   // RUN: %clang_cc1 -DDISABLE_SME_ATTRIBUTES 
>   ...
>   
>   #ifdef DISABLE_SME_ATTRIBUTES
>   #define ARM_STREAMING_ATTR
>   #else
>   #define ARM_STREAMING_ATTR __attribute__((arm_streaming))
>   #endif
>   
>   ...
>   
>   ARM_STREAMING_ATTR void test_svld1_hor_za16(uint32_t slice_base, svbool_t 
> pg, const void *ptr) {
> svld1_hor_za16(0, slice_base, 0, pg, ptr);
> svld1_hor_za16(1, slice_base, 7, pg, ptr);
>   }
> 
> With that the tests all pass, and when the attribute patches have landed we 
> can just remove the `-DDISABLE_SME_ATTRIBUTES` from the RUN lines.
Sounds good, I will give that a shot. If we can unblock this patch, I will also 
rebase and update the rest of the patch series. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127910

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

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Mészáros Gergely via Phabricator via cfe-commits]

Maetveis added a comment.

LGTM, but I don't feel like I have the experience to "formally" approve. I left 
a nice-to have suggestion too, but feel free to ignore, if you feel its out of 
scope.




Comment at: clang/lib/Driver/Driver.cpp:5514
BaseInput);
+handleTimeTrace(C, Args, JA, BaseInput, Result);
   }

One thing D133662 had that this change doesn't is that `--ftime-trace` was 
reported as unused when there was no job that invoked clang (e.g. if using the 
driver to link object files only).

I am not sure its worth the effort, but it would be nice. IIRC in D133662 I did 
this by having a method (`supportsTimeTrace`) on Tools to query support.

I can also do this if you feel its out of scope here, and if there's no 
objection to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282

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

[PATCH] D150321: [clang] Document extensions from later standards

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! Just a few questions, but in general, this is 
looking good.




Comment at: clang/docs/LanguageExtensions.rst:1386
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11

This brings up a few questions for me: how should we handle C? For example, 
designated initializers also exist in C99 and are backported to C89? And how 
should we handle C features extended into C++ (or vice versa)?

Should we have multiple tables? Should we try to put both languages into one 
table with different columns?

(I'm not asking you to sign up to figure out the C extensions, just trying to 
get an idea for whether we want to change the layout of this table to account 
for that sort of thing.)



Comment at: clang/docs/LanguageExtensions.rst:1387
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03

What are your thoughts on extensions enabled by a feature flag? e.g., 
https://godbolt.org/z/ex9K5dzv6


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

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

[PATCH] D147266: [AArch64] Sink operands to allow for bitselect instructions

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Thanks for working on this. I noticed there was another instance of vbsl being 
reported recently in https://github.com/llvm/llvm-project/issues/62642. 
Hopefully it can be addresses via extra optimizations too.

Can you add a testcase for the issues in 
https://github.com/llvm/llvm-project/issues/49305? And look into the existing 
tests.




Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14343
+// passed to this function. Starting pattern matching with any other
+// instruction (such as Or) would lead to malformed IR
+

That sounds like it might be a bug that happens if it tries to sink too many 
operands? From what I remember the order they are put into Ops might matter. 
And if it is sinking to the Or it might need to add both the And as well as the 
Not.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14346
+// Check if this AND instruction is part of bigger tree rooted at Or.
+if (Subtarget->hasNEON() && I->getNumUses() == 1) {
+  Use &U = *(I->use_begin());

I->hasOneUse()



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14348
+  Use &U = *(I->use_begin());
+  Instruction *OI = cast(U.getUser());
+  Value *And0_Op0 = nullptr, *And0_Op1 = nullptr, *And1_Op0 = nullptr,

I->user_back();



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14364
+for (unsigned AndOpIndex = 0; AndOpIndex < 2; ++AndOpIndex) {
+  if (const Instruction *XI =
+  dyn_cast(Ands[AndIndex][AndOpIndex]);

Can this be simplified with a m_Not matcher? In general instructions will be 
canonicalized so that constants are on the RHS.

If we are sinking the Not, I feel this should want to test that the pattern 
makes up a bsl, and if it does then sink the operand of I. i.e. something like 
checking that OI is `m_c_Or(m_c_And(m_Value(A),m_Value(B)),m_specific(I))`, and 
then checking that `I` is `m_c_And(m_Not(m_Specific(A)),m_Value(D))` or the 
other way around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147266

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

[PATCH] D150321: [clang] Document extensions from later standards

[Hristo Hristov via Phabricator via cfe-commits]

H-G-Hristov added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:1398
+``static operator()``   __cpp_static_call_operator   C++23 
C++03
+Strucuted bindings  __cpp_structured_bindingsC++17 
C++03
+template template arguments __cpp_template_template_args C++17 
C++03




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

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

[PATCH] D150321: [clang] Document extensions from later standards

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:1386
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11

aaron.ballman wrote:
> This brings up a few questions for me: how should we handle C? For example, 
> designated initializers also exist in C99 and are backported to C89? And how 
> should we handle C features extended into C++ (or vice versa)?
> 
> Should we have multiple tables? Should we try to put both languages into one 
> table with different columns?
> 
> (I'm not asking you to sign up to figure out the C extensions, just trying to 
> get an idea for whether we want to change the layout of this table to account 
> for that sort of thing.)
I think it would make sense to put C features that are back-ported into 
previous standards also into this table (are there FTMs for C?). C into C++ or 
vice versa should probably live somewhere else, since in these cases it would 
also be interesting how things interact with other language features. Maybe 
this is not as much of a problem for C++->C(?), but definitely for C->C++. e.g. 
`__restrict` on member functions or VLAs with non-trivial classes. Essentially, 
there is a lot more to say than "this also exists in previous standards".



Comment at: clang/docs/LanguageExtensions.rst:1387
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03

aaron.ballman wrote:
> What are your thoughts on extensions enabled by a feature flag? e.g., 
> https://godbolt.org/z/ex9K5dzv6
I actually had no idea they existed (might be worth documenting :P)! 
Specifically for this case: Is there any reason this is behind a feature flag? 
I guess it's a non-conforming extension? If not, I'd say just make it an 
enabled-by-default extension (it would be a really nice cleanup opportunity for 
libc++). Anyways, we could add another column "flags required" for these cases. 
Do you know how many of these kinds of flags there are?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

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

[PATCH] D149986: AMDGPU: Force sc0 and sc1 on stores for gfx940 and gfx941

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:524
+ SIAtomicAddrSpace::NONE)
+  return enableSC0Bit(MI) | enableSC1Bit(MI);
+return false;

Pierre-vh wrote:
> kzhuravl wrote:
> > jmmartinez wrote:
> > > NIT: Is the use of the bitwise or " | " intended? I'd use the logical or 
> > > " || " instead.
> > It is intentional, we need both SC0 and SC1 bits set. If I switch this to 
> > || it will short circuit and not invoke enableSC1Bit.
> IMHO then it needs a comment to explain that it's intentional, otherwise some 
> innocent maintainer in the future could think it's a typo and change it
Added comment.


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

https://reviews.llvm.org/D149986

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

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

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

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

Change of some type definitions, using `restrict` again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

Files:
  clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
  clang/test/Analysis/Inputs/std-c-library-functions.h
  clang/test/Analysis/std-c-library-functions-POSIX.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions.c

Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -50,6 +50,9 @@
 // CHECK-NEXT: Loaded summary for: int isspace(int)
 // CHECK-NEXT: Loaded summary for: int isupper(int)
 // CHECK-NEXT: Loaded summary for: int isxdigit(int)
+// CHECK-NEXT: Loaded summary for: int toupper(int)
+// CHECK-NEXT: Loaded summary for: int tolower(int)
+// CHECK-NEXT: Loaded summary for: int toascii(int)
 // CHECK-NEXT: Loaded summary for: int getc(FILE *)
 // CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
 // CHECK-NEXT: Loaded summary for: int getchar(void)
@@ -59,16 +62,14 @@
 // CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
 // CHECK-NEXT: Loaded summary for: ssize_t getline(char **restrict, size_t *restrict, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: char *getenv(const char *)
 
+#include "Inputs/std-c-library-functions.h"
 
 void clang_analyzer_eval(int);
 
 int glob;
 
-typedef struct FILE FILE;
-#define EOF -1
-
-int getc(FILE *);
 void test_getc(FILE *fp) {
   int x;
   while ((x = getc(fp)) != EOF) {
@@ -77,17 +78,11 @@
   }
 }
 
-int fgetc(FILE *);
 void test_fgets(FILE *fp) {
   clang_analyzer_eval(fgetc(fp) < 256); // expected-warning{{TRUE}}
   clang_analyzer_eval(fgetc(fp) >= 0); // expected-warning{{UNKNOWN}}
 }
 
-
-typedef typeof(sizeof(int)) size_t;
-typedef signed long ssize_t;
-ssize_t read(int, void *, size_t);
-ssize_t write(int, const void *, size_t);
 void test_read_write(int fd, char *buf) {
   glob = 1;
   ssize_t x = write(fd, buf, 10);
@@ -106,8 +101,6 @@
   }
 }
 
-size_t fread(void *restrict, size_t, size_t, FILE *restrict);
-size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
 void test_fread_fwrite(FILE *fp, int *buf) {
 
   size_t x = fwrite(buf, sizeof(int), 10, fp);
@@ -128,8 +121,6 @@
   (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
 }
 
-ssize_t getline(char **restrict, size_t *restrict, FILE *restrict);
-ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict);
 void test_getline(FILE *fp) {
   char *line = 0;
   size_t n = 0;
@@ -139,7 +130,6 @@
   }
 }
 
-int isascii(int);
 void test_isascii(int x) {
   clang_analyzer_eval(isascii(123)); // expected-warning{{TRUE}}
   clang_analyzer_eval(isascii(-1)); // expected-warning{{FALSE}}
@@ -157,7 +147,6 @@
   clang_analyzer_eval(glob); // expected-warning{{TRUE}}
 }
 
-int islower(int);
 void test_islower(int x) {
   clang_analyzer_eval(islower('x')); // expected-warning{{TRUE}}
   clang_analyzer_eval(islower('X')); // expected-warning{{FALSE}}
@@ -165,7 +154,6 @@
 clang_analyzer_eval(x < 'a'); // expected-warning{{FALSE}}
 }
 
-int getchar(void);
 void test_getchar(void) {
   int x = getchar();
   if (x == EOF)
@@ -174,27 +162,23 @@
   clang_analyzer_eval(x < 256); // expected-warning{{TRUE}}
 }
 
-int isalpha(int);
 void test_isalpha(void) {
   clang_analyzer_eval(isalpha(']')); // expected-warning{{FALSE}}
   clang_analyzer_eval(isalpha('Q')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isalpha(128)); // expected-warning{{UNKNOWN}}
 }
 
-int isalnum(int);
 void test_alnum(void) {
   clang_analyzer_eval(isalnum('1')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isalnum(')')); // expected-warning{{FALSE}}
 }
 
-int isblank(int);
 void test_isblank(void) {
   clang_analyzer_eval(isblank('\t')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isblank(' ')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isblank('\n')); // expected-warning{{FALSE}}
 }
 
-int ispunct(int);
 void test_ispunct(int x) {
   clang_analyzer_eval(ispunct(' ')); // expected-warning{{FALSE}}
   clang_analyzer_eval(ispunct(-1)); // expected-warning{{FALSE}}
@@ -204,21 +188,17 @@
 clang_analyzer_eval(x < 127); // expected-warning{{TRUE}}
 }
 
-int isupper(int);
 void test_isupper(int x) {
   if (isupper(x))
 clang_analyzer_eval(x < 'A'); // expected-warning{{FALSE}}
 }
 
-int isgraph(int);
-int isprint(int);
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
 clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
 }
 
-int isdigit(int);
 void test_mixed_

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav updated this revision to Diff 521330.
chaitanyav added a comment.

Rebase with upstream


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Analysis/uninit-vals.c
  clang/test/Sema/integer-overflow.c
  clang/test/Sema/parentheses.c
  clang/test/Sema/parentheses.cpp
  clang/test/SemaCXX/array-bounds.cpp
  clang/test/SemaCXX/integer-overflow.cpp
  libcxx/include/__chrono/duration.h
  libcxx/include/strstream
  libcxx/test/libcxx/algorithms/nth_element_stability.pass.cpp
  libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
  libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
  libcxxabi/src/cxa_personality.cpp

Index: libcxxabi/src/cxa_personality.cpp
===
--- libcxxabi/src/cxa_personality.cpp
+++ libcxxabi/src/cxa_personality.cpp
@@ -718,9 +718,7 @@
 if (actionEntry == 0)
 {
 // Found a cleanup
-results.reason = actions & _UA_SEARCH_PHASE
- ? _URC_CONTINUE_UNWIND
- : _URC_HANDLER_FOUND;
+results.reason = (actions & _UA_SEARCH_PHASE) ? _URC_CONTINUE_UNWIND : _URC_HANDLER_FOUND;
 return;
 }
 // Convert 1-based byte offset into
@@ -832,9 +830,8 @@
 // End of action list. If this is phase 2 and we have found
 // a cleanup (ttypeIndex=0), return _URC_HANDLER_FOUND;
 // otherwise return _URC_CONTINUE_UNWIND.
-results.reason = hasCleanup && actions & _UA_CLEANUP_PHASE
- ? _URC_HANDLER_FOUND
- : _URC_CONTINUE_UNWIND;
+results.reason =
+(hasCleanup && (actions & _UA_CLEANUP_PHASE)) ? _URC_HANDLER_FOUND : _URC_CONTINUE_UNWIND;
 return;
 }
 // Go to next action
@@ -1243,10 +1240,9 @@
 {
 const __shim_type_info* excpType =
 static_cast(new_exception_header->exceptionType);
-adjustedPtr =
-__getExceptionClass(&new_exception_header->unwindHeader) == kOurDependentExceptionClass ?
-((__cxa_dependent_exception*)new_exception_header)->primaryException :
-new_exception_header + 1;
+adjustedPtr = (__getExceptionClass(&new_exception_header->unwindHeader) == kOurDependentExceptionClass)
+  ? ((__cxa_dependent_exception*)new_exception_header)->primaryException
+  : new_exception_header + 1;
 if (!exception_spec_can_catch(ttypeIndex, classInfo, ttypeEncoding,
   excpType, adjustedPtr,
   unwind_exception, base))
Index: libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   std::less comp;
   std::__sort_dispatch(v.begin(), v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto deterministic_v = deterministic();
   std::sort(v.begin(), v.end());
@@ -62,7 +62,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto snapshot_v = v;
   auto snapshot_custom_v = v;
Index: libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto comp = std::less();
   std::__partial_sort_impl(v.begin(), v.begin() + kSize / 2, v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto deterministic_v = deterministic

[PATCH] D150321: [clang] Document extensions from later standards

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:1386
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11

philnik wrote:
> aaron.ballman wrote:
> > This brings up a few questions for me: how should we handle C? For example, 
> > designated initializers also exist in C99 and are backported to C89? And 
> > how should we handle C features extended into C++ (or vice versa)?
> > 
> > Should we have multiple tables? Should we try to put both languages into 
> > one table with different columns?
> > 
> > (I'm not asking you to sign up to figure out the C extensions, just trying 
> > to get an idea for whether we want to change the layout of this table to 
> > account for that sort of thing.)
> I think it would make sense to put C features that are back-ported into 
> previous standards also into this table (are there FTMs for C?). C into C++ 
> or vice versa should probably live somewhere else, since in these cases it 
> would also be interesting how things interact with other language features. 
> Maybe this is not as much of a problem for C++->C(?), but definitely for 
> C->C++. e.g. `__restrict` on member functions or VLAs with non-trivial 
> classes. Essentially, there is a lot more to say than "this also exists in 
> previous standards".
> I think it would make sense to put C features that are back-ported into 
> previous standards also into this table (are there FTMs for C?).

Okay, I think that makes sense to me. Clang has feature test macros for some C 
functionality (e.g., `c_generic_selections`, more full list at: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Features.def#L241)

> C into C++ or vice versa should probably live somewhere else, since in these 
> cases it would also be interesting how things interact with other language 
> features.

Sold!



Comment at: clang/docs/LanguageExtensions.rst:1387
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03

philnik wrote:
> aaron.ballman wrote:
> > What are your thoughts on extensions enabled by a feature flag? e.g., 
> > https://godbolt.org/z/ex9K5dzv6
> I actually had no idea they existed (might be worth documenting :P)! 
> Specifically for this case: Is there any reason this is behind a feature 
> flag? I guess it's a non-conforming extension? If not, I'd say just make it 
> an enabled-by-default extension (it would be a really nice cleanup 
> opportunity for libc++). Anyways, we could add another column "flags 
> required" for these cases. Do you know how many of these kinds of flags there 
> are?
> I actually had no idea they existed (might be worth documenting :P)! 

Right? 
https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fdouble-square-bracket-attributes
 exists but is hardly ideal.

> Specifically for this case: Is there any reason this is behind a feature 
> flag? I guess it's a non-conforming extension? If not, I'd say just make it 
> an enabled-by-default extension (it would be a really nice cleanup 
> opportunity for libc++).

It's a bit of an odd situation. I added the feature flag because I wanted to 
expose this functionality in C before WG14 adopted the functionality and it 
wasn't entirely clear whether I would run into ambiguities with Objective-C's 
message sending syntax. C++ has this language rule: 
http://eel.is/c++draft/dcl.dcl#dcl.attr.grammar-7  but Objective-C allows for 
things like `[[foo bar] baz];`. We had already solved this for C++, but it 
wasn't entirely clear whether we'd hit other situations in older language modes 
or different situations.

These days, it may be possible we could enable this extension by default in all 
language modes.

> Anyways, we could add another column "flags required" for these cases. Do you 
> know how many of these kinds of flags there are?

I can't think of many... Features.def lists the language options required for 
various features and extensions, and it doesn't look like many have specific 
flags, though there are some that needs a more general flag like 
`-fms-extensions`. We do have things like `-fno-rtti` disabling RTTI, but I 
don't think we want to list that sort of thing here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

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

[PATCH] D150364: [clang][Interp] Add 'Unsupported' opcode and use it for throw stmts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

"Unsupported" is a bit of a loaded term -- it could mean "this operation is not 
supported, YET" or it could mean "this operation is not and will not be 
supported, EVER". Perhaps something more like "InvalidInConstantExpr" would be 
more descriptive?




Comment at: clang/test/AST/Interp/unsupported.cpp:3-5
+
+
+

Can get rid of some whitespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150364

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

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

[Teresa Johnson via Phabricator via cfe-commits]

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



Comment at: llvm/test/ThinLTO/X86/public-type-test.ll:31
 ; HIDDEN-NOT: call {{.*}}@llvm.public.type.test
 ; HIDDEN: call {{.*}}@llvm.type.test
 

aeubanks wrote:
> should this be checked twice for both public type tests?
Yes, fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150326

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

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 521340.
tejohnson marked an inline comment as done.
tejohnson added a comment.

Address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150326

Files:
  clang/test/CodeGenCXX/thinlto_public_type_test_distributed.ll
  lld/test/ELF/lto/update_public_type_test.ll
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
  llvm/test/ThinLTO/X86/public-type-test.ll

Index: llvm/test/ThinLTO/X86/public-type-test.ll
===
--- llvm/test/ThinLTO/X86/public-type-test.ll
+++ llvm/test/ThinLTO/X86/public-type-test.ll
@@ -1,16 +1,40 @@
-; Test to ensure that the legacy LTO API lowers @llvm.public.type.test.
+; Test to ensure that the LTO API (legacy and new) lowers @llvm.public.type.test.
 
-; RUN: opt -module-summary %s -o %t.bc
-; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t.bc --thinlto-save-temps=%t2
-; RUN: llvm-dis -o - %t20.2.internalized.bc | FileCheck %s --check-prefix=PUBLIC
-; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t.bc --thinlto-save-temps=%t2 --whole-program-visibility
-; RUN: llvm-dis -o - %t20.2.internalized.bc | FileCheck %s --check-prefix=HIDDEN
+; RUN: split-file %s %t
+
+; RUN: opt -module-summary %t/main.ll -o %t/main.bc
+; RUN: opt -module-summary %t/foo.ll -o %t/foo.bc
+; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t/main.bc %t/foo.bc --thinlto-save-temps=%t2.
+; RUN: llvm-dis -o - %t2.0.3.imported.bc | FileCheck %s --check-prefix=PUBLIC
+; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t/main.bc %t/foo.bc --thinlto-save-temps=%t2. --whole-program-visibility
+; RUN: llvm-dis -o - %t2.0.3.imported.bc | FileCheck %s --check-prefix=HIDDEN
+
+; RUN: llvm-lto2 run %t/main.bc %t/foo.bc -save-temps -pass-remarks=. \
+; RUN:   -whole-program-visibility \
+; RUN:   -o %t3 \
+; RUN:   -r=%t/main.bc,_main,px \
+; RUN:   -r=%t/main.bc,_bar,px \
+; RUN:   -r=%t/main.bc,_foo, \
+; RUN:   -r=%t/foo.bc,_foo,px
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=HIDDEN
+; RUN: llvm-lto2 run %t/main.bc %t/foo.bc -save-temps -pass-remarks=. \
+; RUN:   -o %t3 \
+; RUN:   -r=%t/main.bc,_main,px \
+; RUN:   -r=%t/main.bc,_bar,px \
+; RUN:   -r=%t/main.bc,_foo, \
+; RUN:   -r=%t/foo.bc,_foo,px
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=PUBLIC
 
 ; PUBLIC-NOT: call {{.*}}@llvm.public.type.test
 ; PUBLIC-NOT: call {{.*}}@llvm.type.test
+;; We should have converted the type tests from both main and the imported
+;; copy of foo to non-public.
+; HIDDEN-NOT: call {{.*}}@llvm.public.type.test
+; HIDDEN: call {{.*}}@llvm.type.test
 ; HIDDEN-NOT: call {{.*}}@llvm.public.type.test
 ; HIDDEN: call {{.*}}@llvm.type.test
 
+;--- main.ll
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9"
 
@@ -18,8 +42,31 @@
 entry:
   %p = call i1 @llvm.public.type.test(ptr %vtable, metadata !"_ZTS1A")
   call void @llvm.assume(i1 %p)
+  call void @bar(ptr %vtable)
   ret i32 0
 }
 
+define void @bar(ptr %vtable) {
+entry:
+  call void @foo(ptr %vtable)
+  ret void
+}
+
+declare void @foo(ptr %vtable)
+
+declare void @llvm.assume(i1)
+declare i1 @llvm.public.type.test(ptr, metadata)
+
+;--- foo.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.9"
+
+define void @foo(ptr %vtable) {
+entry:
+  %p = call i1 @llvm.public.type.test(ptr %vtable, metadata !"_ZTS1A")
+  call void @llvm.assume(i1 %p)
+  ret void
+}
+
 declare void @llvm.assume(i1)
 declare i1 @llvm.public.type.test(ptr, metadata)
Index: llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
===
--- llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
+++ llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
@@ -18,7 +18,7 @@
 ; RUN:   -r=%t2.o,_ZTV1B,px \
 ; RUN:   -r=%t2.o,_ZTV1C,px \
 ; RUN:   -r=%t2.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK
-; RUN: llvm-dis %t3.1.0.preopt.bc -o - | FileCheck %s --check-prefix=CHECK-TT
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=CHECK-TT
 ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; Hybrid WPD
@@ -43,7 +43,7 @@
 ; RUN:   -r=%t.o,_ZTV1B,px \
 ; RUN:   -r=%t.o,_ZTV1C,px \
 ; RUN:   -r=%t.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK --dump-input=fail
-; RUN: llvm-dis %t3.1.0.preopt.bc -o - | FileCheck %s --check-prefix=CHECK-TT
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=CHECK-TT
 ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; Regular LTO WPD
@@ -83,7 +83,7 @@
 ; RUN:   -r=%t2.o,_ZTV1B,px \
 ; RUN:   -r=%t2.o,_ZTV1C,px \
 ; RUN:   -r=%t2.o,_

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

[Teresa Johnson 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 rGa40b0c3e77a2: [WPD] Update llvm.public.type.test after 
importing functions (authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150326

Files:
  clang/test/CodeGenCXX/thinlto_public_type_test_distributed.ll
  lld/test/ELF/lto/update_public_type_test.ll
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
  llvm/test/ThinLTO/X86/public-type-test.ll

Index: llvm/test/ThinLTO/X86/public-type-test.ll
===
--- llvm/test/ThinLTO/X86/public-type-test.ll
+++ llvm/test/ThinLTO/X86/public-type-test.ll
@@ -1,16 +1,40 @@
-; Test to ensure that the legacy LTO API lowers @llvm.public.type.test.
+; Test to ensure that the LTO API (legacy and new) lowers @llvm.public.type.test.
 
-; RUN: opt -module-summary %s -o %t.bc
-; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t.bc --thinlto-save-temps=%t2
-; RUN: llvm-dis -o - %t20.2.internalized.bc | FileCheck %s --check-prefix=PUBLIC
-; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t.bc --thinlto-save-temps=%t2 --whole-program-visibility
-; RUN: llvm-dis -o - %t20.2.internalized.bc | FileCheck %s --check-prefix=HIDDEN
+; RUN: split-file %s %t
+
+; RUN: opt -module-summary %t/main.ll -o %t/main.bc
+; RUN: opt -module-summary %t/foo.ll -o %t/foo.bc
+; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t/main.bc %t/foo.bc --thinlto-save-temps=%t2.
+; RUN: llvm-dis -o - %t2.0.3.imported.bc | FileCheck %s --check-prefix=PUBLIC
+; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t/main.bc %t/foo.bc --thinlto-save-temps=%t2. --whole-program-visibility
+; RUN: llvm-dis -o - %t2.0.3.imported.bc | FileCheck %s --check-prefix=HIDDEN
+
+; RUN: llvm-lto2 run %t/main.bc %t/foo.bc -save-temps -pass-remarks=. \
+; RUN:   -whole-program-visibility \
+; RUN:   -o %t3 \
+; RUN:   -r=%t/main.bc,_main,px \
+; RUN:   -r=%t/main.bc,_bar,px \
+; RUN:   -r=%t/main.bc,_foo, \
+; RUN:   -r=%t/foo.bc,_foo,px
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=HIDDEN
+; RUN: llvm-lto2 run %t/main.bc %t/foo.bc -save-temps -pass-remarks=. \
+; RUN:   -o %t3 \
+; RUN:   -r=%t/main.bc,_main,px \
+; RUN:   -r=%t/main.bc,_bar,px \
+; RUN:   -r=%t/main.bc,_foo, \
+; RUN:   -r=%t/foo.bc,_foo,px
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=PUBLIC
 
 ; PUBLIC-NOT: call {{.*}}@llvm.public.type.test
 ; PUBLIC-NOT: call {{.*}}@llvm.type.test
+;; We should have converted the type tests from both main and the imported
+;; copy of foo to non-public.
+; HIDDEN-NOT: call {{.*}}@llvm.public.type.test
+; HIDDEN: call {{.*}}@llvm.type.test
 ; HIDDEN-NOT: call {{.*}}@llvm.public.type.test
 ; HIDDEN: call {{.*}}@llvm.type.test
 
+;--- main.ll
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9"
 
@@ -18,8 +42,31 @@
 entry:
   %p = call i1 @llvm.public.type.test(ptr %vtable, metadata !"_ZTS1A")
   call void @llvm.assume(i1 %p)
+  call void @bar(ptr %vtable)
   ret i32 0
 }
 
+define void @bar(ptr %vtable) {
+entry:
+  call void @foo(ptr %vtable)
+  ret void
+}
+
+declare void @foo(ptr %vtable)
+
+declare void @llvm.assume(i1)
+declare i1 @llvm.public.type.test(ptr, metadata)
+
+;--- foo.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.9"
+
+define void @foo(ptr %vtable) {
+entry:
+  %p = call i1 @llvm.public.type.test(ptr %vtable, metadata !"_ZTS1A")
+  call void @llvm.assume(i1 %p)
+  ret void
+}
+
 declare void @llvm.assume(i1)
 declare i1 @llvm.public.type.test(ptr, metadata)
Index: llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
===
--- llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
+++ llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
@@ -18,7 +18,7 @@
 ; RUN:   -r=%t2.o,_ZTV1B,px \
 ; RUN:   -r=%t2.o,_ZTV1C,px \
 ; RUN:   -r=%t2.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK
-; RUN: llvm-dis %t3.1.0.preopt.bc -o - | FileCheck %s --check-prefix=CHECK-TT
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=CHECK-TT
 ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; Hybrid WPD
@@ -43,7 +43,7 @@
 ; RUN:   -r=%t.o,_ZTV1B,px \
 ; RUN:   -r=%t.o,_ZTV1C,px \
 ; RUN:   -r=%t.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK --dump-input=fail
-; RUN: llvm-dis %t3.1.0.preopt.bc -o - | FileCheck %s --check-prefix=CHECK-TT
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=CHECK-TT
 ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; Reg

[PATCH] D150364: [clang][Interp] Add 'Unsupported' opcode and use it for throw stmts

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

tbaeder added a comment.

In D150364#4335221 , @aaron.ballman 
wrote:

> "Unsupported" is a bit of a loaded term -- it could mean "this operation is 
> not supported, YET" or it could mean "this operation is not and will not be 
> supported, EVER". Perhaps something more like "InvalidInConstantExpr" would 
> be more descriptive?

I guess it would be more descriptive, but it could still mean that it is "not 
yet valid in a constant expression", so I guess I don't see the upside of using 
a longer opcode name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150364

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

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:22-23
+
+static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
+   InterpFrame *Frame) {
+  const Pointer &A = getParam(Frame, 0);

tbaeder wrote:
> aaron.ballman wrote:
> > Thought: it would be nice if we could hoist as much of this implementation 
> > as we can into a helper function so that we can share most of the guts with 
> > `__builtin_memcmp()` as well.
> > 
> > Also, it would be good to generalize this so it works for 
> > `__builtin_wcscmp()` and `__builtin_wmemcmp()` as well.
> Definitely, but I think it makes more sense to do that when we implement 
> those functions.
I guess I was mostly wondering why we don't generalize that now given that we 
know the need exists. In fact, it seems like you could be handling 
`__builtin_strncmp` and others at the same time with one generalized 
implementation.


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

https://reviews.llvm.org/D149816

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

[PATCH] D150364: [clang][Interp] Add 'Unsupported' opcode and use it for throw stmts

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

tbaeder updated this revision to Diff 521346.

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

https://reviews.llvm.org/D150364

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/records.cpp
  clang/test/AST/Interp/unsupported.cpp

Index: clang/test/AST/Interp/unsupported.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/unsupported.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=ref %s
+
+namespace Throw {
+
+  constexpr int ConditionalThrow(bool t) {
+if (t)
+  throw 4; // expected-note {{subexpression not valid in a constant expression}} \
+   // ref-note {{subexpression not valid in a constant expression}}
+
+return 0;
+  }
+
+  static_assert(ConditionalThrow(false) == 0, "");
+  static_assert(ConditionalThrow(true) == 0, ""); // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'ConditionalThrow(true)'}} \
+  // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to 'ConditionalThrow(true)'}}
+
+  constexpr int Throw() { // expected-error {{never produces a constant expression}} \
+  // ref-error {{never produces a constant expression}}
+throw 5; // expected-note {{subexpression not valid in a constant expression}} \
+ // ref-note {{subexpression not valid in a constant expression}}
+return 0;
+  }
+}
+
+namespace Asm {
+  constexpr int ConditionalAsm(bool t) {
+if (t)
+  asm(""); // expected-note {{subexpression not valid in a constant expression}} \
+   // ref-note {{subexpression not valid in a constant expression}}
+
+return 0;
+  }
+  static_assert(ConditionalAsm(false) == 0, "");
+  static_assert(ConditionalAsm(true) == 0, ""); // expected-error {{not an integral constant expression}} \
+// expected-note {{in call to 'ConditionalAsm(true)'}} \
+// ref-error {{not an integral constant expression}} \
+// ref-note {{in call to 'ConditionalAsm(true)'}}
+
+
+  constexpr int Asm() { // expected-error {{never produces a constant expression}} \
+// ref-error {{never produces a constant expression}}
+__asm volatile(""); // expected-note {{subexpression not valid in a constant expression}} \
+// ref-note {{subexpression not valid in a constant expression}}
+return 0;
+  }
+}
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -328,7 +328,8 @@
   struct S {
 constexpr S() {}
 constexpr ~S() noexcept(false) { throw 12; } // expected-error {{cannot use 'throw'}} \
- // expected-note {{declared here}} \
+ // expected-error {{never produces a constant expression}} \
+ // expected-note 2{{subexpression not valid}} \
  // ref-error {{cannot use 'throw'}} \
  // ref-error {{never produces a constant expression}} \
  // ref-note 2{{subexpression not valid}}
@@ -336,7 +337,9 @@
 
   constexpr int f() {
 S{}; // ref-note {{in call to '&S{}->~S()'}}
-return 12; // expected-note {{undefined function '~S'}}
+
+/// FIXME: Wrong source location below.
+return 12; // expected-note {{in call to '&S{}->~S()'}}
   }
   static_assert(f() == 12); // expected-error {{not an integral constant expression}} \
 // expected-note {{in call to 'f()'}} \
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -627,3 +627,6 @@
   let Types = [AllTypeClass];
   let HasGroup = 1;
 }
+
+// [] -> []
+def Unsupported : Opcode {}
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1823,6 +1823,14 @@
   return true;
 }
 
+/// Just emit a diagnostic. The expression that caused emission of this
+/// op is not valid in a const

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added subscribers: lenary, foad.
tahonermann added a comment.

> I was following the LLVM contribution guidelines to use git clang-format, but 
> I understand the importance of maintaining existing code styles that may be 
> altered by git-clang format.

The guidelines are slightly in conflict in that regard so, yeah, its a 
judgement call.

I added two more suggested edits targeting some comments.

This looks good to me, but I think we should make sure AMD and ARM folks are 
aware of the change. @foad, @lenary, any concerns?




Comment at: clang/include/clang/Basic/TargetInfo.h:650
 
-  /// Determine whether the _BFloat16 type is supported on this target.
-  virtual bool hasBFloat16Type() const { return HasBFloat16; }
+  /// Determine whether the _BF16 type is supported on this target.
+  virtual bool hasBF16Type() const { return HasBF16; }





Comment at: clang/include/clang/Basic/arm_neon_incl.td:240
 // F: change to floating category.
-// B: change to BFloat16
+// B: change to BF16
 // P: change to polynomial category.




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

https://reviews.llvm.org/D150291

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

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:22-23
+
+static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
+   InterpFrame *Frame) {
+  const Pointer &A = getParam(Frame, 0);

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > Thought: it would be nice if we could hoist as much of this 
> > > implementation as we can into a helper function so that we can share most 
> > > of the guts with `__builtin_memcmp()` as well.
> > > 
> > > Also, it would be good to generalize this so it works for 
> > > `__builtin_wcscmp()` and `__builtin_wmemcmp()` as well.
> > Definitely, but I think it makes more sense to do that when we implement 
> > those functions.
> I guess I was mostly wondering why we don't generalize that now given that we 
> know the need exists. In fact, it seems like you could be handling 
> `__builtin_strncmp` and others at the same time with one generalized 
> implementation.
Because smaller patches get reviewed faster :)


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

https://reviews.llvm.org/D149816

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

[PATCH] D150364: [clang][Interp] Add 'Unsupported' opcode and use it for throw stmts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D150364#4335261 , @tbaeder wrote:

> In D150364#4335221 , @aaron.ballman 
> wrote:
>
>> "Unsupported" is a bit of a loaded term -- it could mean "this operation is 
>> not supported, YET" or it could mean "this operation is not and will not be 
>> supported, EVER". Perhaps something more like "InvalidInConstantExpr" would 
>> be more descriptive?
>
> I guess it would be more descriptive, but it could still mean that it is "not 
> yet valid in a constant expression", so I guess I don't see the upside of 
> using a longer opcode name.

I don't feel strongly; it's easy enough to rename if we think it's causing 
confusion. FWIW, my first thought was "Oh, we're planning to support throw 
expressions in constant expressions? Please don't tell WG21." I'm used to 
seeing "invalid" for things that are never valid and "unsupported" for things 
that aren't supported but might be someday. However, I also see we use 
"unsupported" in the same sense you're using it here in some of our 
diagnostics, so I'm fine with whatever you want to go with.




Comment at: clang/test/AST/Interp/records.cpp:341
+
+/// FIXME: Wrong source location below.
+return 12; // expected-note {{in call to '&S{}->~S()'}}

Oh interesting -- does the old constexpr interpreter think the destructor is 
called at the end of the block as opposed to at the end of the full expression 
with the temporary?


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

https://reviews.llvm.org/D150364

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

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:22-23
+
+static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
+   InterpFrame *Frame) {
+  const Pointer &A = getParam(Frame, 0);

tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > Thought: it would be nice if we could hoist as much of this 
> > > > implementation as we can into a helper function so that we can share 
> > > > most of the guts with `__builtin_memcmp()` as well.
> > > > 
> > > > Also, it would be good to generalize this so it works for 
> > > > `__builtin_wcscmp()` and `__builtin_wmemcmp()` as well.
> > > Definitely, but I think it makes more sense to do that when we implement 
> > > those functions.
> > I guess I was mostly wondering why we don't generalize that now given that 
> > we know the need exists. In fact, it seems like you could be handling 
> > `__builtin_strncmp` and others at the same time with one generalized 
> > implementation.
> Because smaller patches get reviewed faster :)
LoL fair.


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

https://reviews.llvm.org/D149816

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

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a subscriber: clang-vendors.
tahonermann added a comment.

Heads up @clang-vendors. This patch changes the names of many internal 
identifiers for enumerators, class data members, and functions (including 
virtual functions) that are related to support for the `__bf16` type. The 
changes are roughly to replace "bfloat16" with "bf16" to make the association 
obvious and to avoid confusion when support for the C++ `std::bfloat16_t` type 
lands (and to make room for a potential matching `_BFloat16` type in C in the 
future). There are no changes to ABI or code generation; mangling continues to 
use the same names even where those have "bfloat16" in them. These changes will 
likely cause compilation failures in downstream projects that will require 
(simple) identifier updates to resolve.


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

https://reviews.llvm.org/D150291

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

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs updated this revision to Diff 521353.
codemzs marked 2 inline comments as done.
codemzs set the repository for this revision to rG LLVM Github Monorepo.
codemzs added a comment.

Update comments as per feedback from @tahonermann


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150291

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/arm_neon_incl.td
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/tools/libclang/CXType.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4909,7 +4909,7 @@
 case clang::BuiltinType::Float128:
 case clang::BuiltinType::Double:
 case clang::BuiltinType::LongDouble:
-case clang::BuiltinType::BFloat16:
+case clang::BuiltinType::BF16:
 case clang::BuiltinType::Ibm128:
   return lldb::eEncodingIEEE754;
 
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -618,7 +618,7 @@
 TKIND(Pipe);
 TKIND(Attributed);
 TKIND(BTFTagAttributed);
-TKIND(BFloat16);
+TKIND(BF16);
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) TKIND(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -7024,8 +7024,8 @@
 case PREDEF_TYPE_INT128_ID:
   T = Context.Int128Ty;
   break;
-case PREDEF_TYPE_BFLOAT16_ID:
-  T = Context.BFloat16Ty;
+case PREDEF_TYPE_BF16_ID:
+  T = Context.BF16Ty;
   break;
 case PREDEF_TYPE_HALF_ID:
   T = Context.HalfTy;
Index: clang/lib/Serialization/ASTCommon.cpp
===
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -270,8 +270,8 @@
   case BuiltinType::OMPIterator:
 ID = PREDEF_TYPE_OMP_ITERATOR;
 break;
-  case BuiltinType::BFloat16:
-ID = PREDEF_TYPE_BFLOAT16_ID;
+  case BuiltinType::BF16:
+ID = PREDEF_TYPE_BF16_ID;
 break;
   }
 
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,12 +1518,12 @@
 Result = Context.Float16Ty;
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
-  case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type() &&
+  case DeclSpec::TST_BF16:
+if (!S.Context.getTargetInfo().hasBF16Type() &&
 !(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice) &&
 !S.getLangOpts().SYCLIsDevice)
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << "__bf16";
-Result = Context.BFloat16Ty;
+Result = Context.BF16Ty;
 break;
   case DeclSpec::TST_float:   Result = Context.FloatTy; break;
   case DeclSpec::TST_double:
@@ -8133,7 +8133,7 @@
  BTy->getKind() == BuiltinType::ULongLong ||
  BTy->getKind() == BuiltinType::Float ||

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added inline comments.



Comment at: clang/lib/AST/Type.cpp:2187
+ BT->getKind() <= BuiltinType::Ibm128 &&
+ BT->getKind() != BuiltinType::BF16;
   if (const auto *ET = dyn_cast(CanonicalType))

Looks like another clang-format quirk.




Comment at: clang/lib/CodeGen/TargetInfo.cpp:5568
 
   bool allowBFloatArgsAndRet() const override {
+return getTarget().hasBF16Type();

Should the name of the method be updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150291

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

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Tom Honermann via Phabricator via cfe-commits]

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

Thanks for all the updates @codemzs! I'm going to go ahead and accept. But 
please wait a few days for recently subscribed folks to have a chance to 
comment before landing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150291

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

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

The summary as it is will be hard to read in git log. Please split it into 
multiple lines 72~80 chars each.
https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150291

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

[PATCH] D150318: [clang][deps] NFC: Pass around the whole scanning service

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:69
+  /// The preprocessing mode used for scanning.
+  ScanningMode Mode;
+  /// The output format.

benlangmuir wrote:
> Why drop `const`?
I don't think it adds much, since the members are private and only ever 
accessed in functions already marked `const`. I'm fine with keeping the `const` 
here if you think there's value in it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150318

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

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

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

In D150291#4335360 , @barannikov88 
wrote:

> The summary as it is will be hard to read in git log. Please split it into 
> multiple lines 72~80 chars each.
> https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

I've provided the git log message that I see on my end below. I ensured that it 
is split into multiple lines, with each line not exceeding the character limit. 
Please let me know if this isn't inline with the LLVM contribution standards.

F27408097: image.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150291

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

[PATCH] D150318: [clang][deps] NFC: Pass around the whole scanning service

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:69
+  /// The preprocessing mode used for scanning.
+  ScanningMode Mode;
+  /// The output format.

jansvoboda11 wrote:
> benlangmuir wrote:
> > Why drop `const`?
> I don't think it adds much, since the members are private and only ever 
> accessed in functions already marked `const`. I'm fine with keeping the 
> `const` here if you think there's value in it.
It's fine, just wanted to check I didn't misunderstand this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150318

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

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

In D150291#4335414 , @codemzs wrote:

> I've provided the git log message that I see on my end below. I ensured that 
> it is split into multiple lines, with each line not exceeding the character 
> limit. Please let me know if this isn't inline with the LLVM contribution 
> standards.
>
> F27408097: image.png 

Looks great, thanks.
One note: if you are going to land the changes via arcanist, you will need to 
update the
message in the web form. (arc ignores git commit message and takes it from the 
web.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150291

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

[PATCH] D144943: [clang][Interp] Implement bitcasts (WIP)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D144943#4334500 , @tbaeder wrote:

> Pig

🐷




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:74
+  // FIXME: Diagnostics.
+  if (ToType->isArrayType() || ToType->isPointerType())
+return false;

Member pointer types? Unions? volatile-qualified types? There's quite a few 
restrictions here for constexpr contexts.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:87
+  if (!ToT) {
+// Conversion to an array or record type.
+return this->emitBitCastPtr(E);

We returned earlier if `ToType` is an array, so this comment is a bit 
suspicious.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:101
+  // Conversion to a primitive type. FromType can be another
+  // primitive type, or a record/array.
+  //

All the same restrictions for `ToType` apply to `FromType`: 
http://eel.is/c++draft/bit.cast#3



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:92
+  // FIXME: Diagnostics.
+  if (*ToT == PT_Ptr)
+return false;

tbaeder wrote:
> One of the problems here is that right now, //all// diagnostics are emitted 
> during interpretation time.
Didn't we early return in this case as well?



Comment at: clang/lib/AST/Interp/Integral.h:179
+
+memcpy(&V, Buff, sizeof(ReprT));
+return Integral(V);

Consistency with below.



Comment at: clang/lib/AST/Interp/Interp.h:1366
+  size_t BuffSize = APFloat::semanticsSizeInBits(*Sem) / 8;
+  std::byte Buff[BuffSize];
+

tbaeder wrote:
> This is a variable sized array. That needs to go of course, but is the best 
> way to heap allocate? Or can we actually use `alloca` in clang code?
We usually go with:
```
std::vector Buff(BuffSize);
if (!DoBitCast(S, FromPtr, Buff.data(), BuffSize))
  return false;
```



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:35
+  std::byte *Buff;
+  unsigned Offset;
+  size_t BuffSize;

We should probably be consistent about using `size_t` in this class so there's 
no chance of size conversion between types.



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:33
 
+struct Foo {
+  std::byte *Buff;

tbaeder wrote:
> This needs to be renamed obviously, but I was wondering if this already 
> exists somewhere in the llvm/clang codebase...
I'm not aware of one, but perhaps this exists in LLVM somewhere?



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:81
+/// Rotate things around for big endian targets.
+static void fiddleMemory(std::byte *M, size_t N) {
+  for (size_t I = 0; I != (N / 2); ++I)

tbaeder wrote:
> Same here, I feel like this should already be available?
See 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/SwapByteOrder.h


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

https://reviews.llvm.org/D144943

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

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs updated this revision to Diff 521363.
codemzs marked an inline comment as done.
codemzs added a comment.

Addressing feedback from @barannikov88


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

https://reviews.llvm.org/D150291

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/arm_neon_incl.td
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/tools/libclang/CXType.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4909,7 +4909,7 @@
 case clang::BuiltinType::Float128:
 case clang::BuiltinType::Double:
 case clang::BuiltinType::LongDouble:
-case clang::BuiltinType::BFloat16:
+case clang::BuiltinType::BF16:
 case clang::BuiltinType::Ibm128:
   return lldb::eEncodingIEEE754;
 
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -618,7 +618,7 @@
 TKIND(Pipe);
 TKIND(Attributed);
 TKIND(BTFTagAttributed);
-TKIND(BFloat16);
+TKIND(BF16);
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) TKIND(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -7024,8 +7024,8 @@
 case PREDEF_TYPE_INT128_ID:
   T = Context.Int128Ty;
   break;
-case PREDEF_TYPE_BFLOAT16_ID:
-  T = Context.BFloat16Ty;
+case PREDEF_TYPE_BF16_ID:
+  T = Context.BF16Ty;
   break;
 case PREDEF_TYPE_HALF_ID:
   T = Context.HalfTy;
Index: clang/lib/Serialization/ASTCommon.cpp
===
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -270,8 +270,8 @@
   case BuiltinType::OMPIterator:
 ID = PREDEF_TYPE_OMP_ITERATOR;
 break;
-  case BuiltinType::BFloat16:
-ID = PREDEF_TYPE_BFLOAT16_ID;
+  case BuiltinType::BF16:
+ID = PREDEF_TYPE_BF16_ID;
 break;
   }
 
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,12 +1518,12 @@
 Result = Context.Float16Ty;
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
-  case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type() &&
+  case DeclSpec::TST_BF16:
+if (!S.Context.getTargetInfo().hasBF16Type() &&
 !(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice) &&
 !S.getLangOpts().SYCLIsDevice)
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << "__bf16";
-Result = Context.BFloat16Ty;
+Result = Context.BF16Ty;
 break;
   case DeclSpec::TST_float:   Result = Context.FloatTy; break;
   case DeclSpec::TST_double:
@@ -8133,7 +8133,7 @@
  BTy->getKind() == BuiltinType::ULongLong ||
  BTy->getKind() == BuiltinType::Float ||
  BTy->getKind() == BuiltinType::Half ||
- BTy->getKind() == BuiltinType::BF

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs added a comment.

In D150291#4335352 , @tahonermann 
wrote:

> Thanks for all the updates @codemzs! I'm going to go ahead and accept. But 
> please wait a few days for recently subscribed folks to have a chance to 
> comment before landing this.

Thank you, @tahonermann, for the review. I will hold off on landing this until 
next week to allow sufficient time for others who are subscribed to review as 
well.


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

https://reviews.llvm.org/D150291

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D147844#4333407 , @ldionne wrote:

> Code changes in libc++ and libc++abi LGTM. I am neutral on whether the 
> diagnostic is worth adding, but don't consider libc++ and libc++abi as 
> blockers for this patch.

Thank you @ldionne! My plan is to accept & land this if we hear no objections 
by next Thur (May 18).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D150209: [clang][Interp] Add more shift error checking

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/shifts.cpp:165-171
+  constexpr int negativeShift() { // cxx17-error {{never produces a constant 
expression}} \
+  // ref-cxx17-error {{never produces a 
constant expression}}
+return -1 << 2; // cxx17-warning {{shifting a negative signed value is 
undefined}} \
+// ref-cxx17-warning {{shifting a negative signed value is 
undefined}} \
+// cxx17-note {{left shift of negative value -1}} \
+// ref-cxx17-note {{left shift of negative value -1}}
+  }

I'd like a test along the lines of:
```
constexpr int foo(int a) {
  return -a << 2;
}
static_assert(foo(10)); // Should fail
constexpr int a = -2;
static_assert(foo(a)); // Should be fine
static_assert(foo(-a)); // Should fail
```


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

https://reviews.llvm.org/D150209

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

[PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a added a subscriber: efriedma.
bolshakov-a added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4397
+// argument.
+// As proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/111.
+auto *SNTTPE = cast(E);

erichkeane wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > We should get this nailed down. It was proposed in Nov 2020 and the issue 
> > > is still open. CC @rjmccall 
> > This definitely needs to happen.  @rjmccall or @eli.friedman ^^ Any idea 
> > what the actual mangling should be?
> This is still an open, and we need @rjmccall @eli.friedman or @asl to help 
> out here.
Ping @efriedma, @rjmccall, @asl.


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

https://reviews.llvm.org/D140996

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

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Fangrui Song via Phabricator via cfe-commits]

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

In D150282#4334927 , @Maetveis wrote:

> LGTM, but I don't feel like I have the experience to "formally" approve. I 
> left a nice-to have suggestion too, but feel free to ignore, if you feel its 
> out of scope.

Thanks. You can certainly give your review opinion as you've studied and 
discussed the feature a lot! 
(https://llvm.org/docs/Phabricator.html#:~:text=participated%20might)
If we land a change like this patch, D133662  
can be turned to address the offloading side issue.

It doesn't work without or with this patch; there is only one single JSON 
output file.
I think supporting offloading will make some extensive changes to 
`GetNamedOutputPath`, so I avoid doing that in this patch.
I am not an offloading user and don't understand its behavior well enough.




Comment at: clang/lib/Driver/Driver.cpp:5514
BaseInput);
+handleTimeTrace(C, Args, JA, BaseInput, Result);
   }

Maetveis wrote:
> One thing D133662 had that this change doesn't is that `--ftime-trace` was 
> reported as unused when there was no job that invoked clang (e.g. if using 
> the driver to link object files only).
> 
> I am not sure its worth the effort, but it would be nice. IIRC in D133662 I 
> did this by having a method (`supportsTimeTrace`) on Tools to query support.
> 
> I can also do this if you feel its out of scope here, and if there's no 
> objection to it.
Thanks! This is a worthy change. I'll add a test separately.
I think we can reuse `canEmitIR` instead of adding `supportsTimeTrace`.

Another frontend flang-new defines `canEmitIR` to true as well. It's a 
work-in-progress and can support `-ftime-trace` later.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282

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

[PATCH] D150321: [clang] Document extensions from later standards

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 521375.
philnik marked 5 inline comments as done.
philnik added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here 
`_.
 
+Language extensions back-ported to previous standards
+=
+
+===  
= = ==
+Feature Feature Test Macro   
Introduced In Backported To Required Flags
+===  
= = ==
+variadic templates  __cpp_variadic_templates C++11 
C++03
+Alias templates __cpp_alias_templatesC++11 
C++03
+Non-static data member initializers __cpp_nsdmi  C++11 
C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 
C++03
+RValue references   __cpp_rvalue_references  C++11 
C++03
+Attributes  __cpp_attributes C++11 
C++03 -fdouble-square-bracket-attributes
+variable templates  __cpp_variable_templates C++14 
C++03
+Binary literals __cpp_binary_literalsC++14 
C++03
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+``if constexpr``__cpp_if_constexpr   C++17 
C++11
+fold expressions__cpp_fold_expressions   C++17 
C++03
+Lambda capture of \*this by value   __cpp_capture_star_this  C++17 
C++11
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03
+Hexadecimal floating literals   __cpp_hex_float  C++17 
C++03
+``inline`` variables__cpp_inline_variables   C++17 
C++03
+Attributes on namespaces__cpp_namespace_attributes   C++17 
C++11
+Structured bindings __cpp_structured_bindingsC++17 
C++03
+template template arguments __cpp_template_template_args C++17 
C++03
+``static operator[]``   __cpp_multidimensional_subscript C++20 
C++03
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Conditional ``explicit``__cpp_conditional_explicit   C++20 
C++03
+``using enum``  __cpp_using_enum C++20 
C++03
+``if consteval``__cpp_if_consteval   C++23 
C++20
+``static operator()``   __cpp_static_call_operator   C++23 
C++03
+---  
- - --
+Designated initializers  C99   
C89
+===  
= = ==
+
 Type Trait Primitives
 =
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here `_.
 
+Language extensions back-ported to previous standards
+=
+
+===  = = ==
+Feature Feature Test Macro   Introduced In Backported To Required Flags
+===  = = ==
+variadic templates  __cpp_variadic_templates C++11 C++03
+Alias templates __cpp_alias_templatesC++11 C++03
+Non-static data member initializers __cpp_nsdmi  C++11 C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 C++03
+RValue refere

[PATCH] D150191: [clang][Diagnostics] Provide a source range for 'use of undeclared identifier' diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:2187
 else
-  SemaRef.Diag(TypoLoc, DiagnosticID) << Typo;
+  SemaRef.Diag(TypoRange.getEnd(), DiagnosticID) << Typo;
 return;

tbaeder wrote:
> I'm not passing the `TypoRange` here now, which regresses the test case I 
> posted. Apparently the handling of `-fmacro-backtrace-limit` depends on the 
> range passed here? That seems weird.
Is it failing within `checkRangesForMacroArgExpansion()` in 
DiagnosticRenderer.cpp? It looks like this change effectively undoes the work 
from ecd36ee80b7a6ac73c84da19f8a75c4c025a7625



Comment at: clang/test/Misc/reduced-diags-macros-backtrace.cpp:30-38
+// ALL-NEXT: {{.*}}:6:20: note: expanded from macro 'ADD'
+// ALL-NEXT: #define ADD(x,y) G(x) + y
+// ALL-NEXT:^
+// ALL-NEXT: {{.*}}:5:16: note: expanded from macro 'G'
+// ALL-NEXT: #define G(x) F(x) + 2
+// ALL-NEXT:^
+// ALL-NEXT: {{.*}}:4:14: note: expanded from macro 'F'

These notes make it harder to read the diagnostic instead of easier, I think 
they should remain suppressed.



Comment at: clang/test/Misc/reduced-diags-macros-backtrace.cpp:42-56
+// ALL-NEXT: {{.*}}:10:26: note: expanded from macro 'LEVEL1'
+// ALL-NEXT: #define LEVEL1(x) LEVEL2(x)
+// ALL-NEXT:  ^
+// ALL-NEXT: {{.*}}:9:26: note: expanded from macro 'LEVEL2'
+// ALL-NEXT: #define LEVEL2(x) LEVEL3(x)
+// ALL-NEXT:  ^
+// ALL-NEXT: {{.*}}:8:26: note: expanded from macro 'LEVEL3'

Same here.


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

https://reviews.llvm.org/D150191

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

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 521379.
MaskRay marked an inline comment as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Add a warning test. Report -Wunused-command-line-argument warning for 
-ftime-trace-granularity=0 if unused as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282

Files:
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ftime-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -213,9 +213,7 @@
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
 Argv, Diags, Argv0);
 
-  if (Clang->getFrontendOpts().TimeTrace ||
-  !Clang->getFrontendOpts().TimeTracePath.empty()) {
-Clang->getFrontendOpts().TimeTrace = 1;
+  if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
 llvm::timeTraceProfilerInitialize(
 Clang->getFrontendOpts().TimeTraceGranularity, Argv0);
   }
@@ -257,16 +255,6 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
-
 // It is possible that the compiler instance doesn't own a file manager here
 // if we're compiling a module unit. Since the file manager are owned by AST
 // when we're compiling a module unit. So the file manager may be invalid
@@ -280,7 +268,8 @@
   Clang->getInvocation(), Clang->getDiagnostics()));
 
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+Clang->getFrontendOpts().TimeTracePath, /*Binary=*/false,
+/*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/ftime-trace.cpp
===
--- clang/test/Driver/ftime-trace.cpp
+++ clang/test/Driver/ftime-trace.cpp
@@ -31,6 +31,31 @@
 // CHECK:  "name": "process_name"
 // CHECK:  "name": "thread_name"
 
+// RUN: mkdir d e f && cp %s d/a.cpp && touch d/b.c
+
+// RUN: %clang -### -c -ftime-trace -ftime-trace-granularity=0 d/a.cpp -o e/a.o 2>&1 | FileCheck %s --check-prefix=COMPILE1
+// COMPILE1: -cc1{{.*}} "-ftime-trace=e/a.json"
+
+// RUN: %clang -### -c -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=COMPILE2
+// COMPILE2: -cc1{{.*}} "-ftime-trace=f/a.json"
+// COMPILE2: -cc1{{.*}} "-ftime-trace=f/b.json"
+
+// RUN: %clang -### -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -o e/x 2>&1 | FileCheck %s --check-prefix=LINK1
+// LINK1: -cc1{{.*}} "-ftime-trace=e/x-a.json"
+// LINK1: -cc1{{.*}} "-ftime-trace=e/x-b.json"
+
+// RUN: %clang -### -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -o e/x -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=LINK2
+// LINK2: -cc1{{.*}} "-ftime-trace=f/a.json"
+// LINK2: -cc1{{.*}} "-ftime-trace=f/b.json"
+
+// RUN: %clang -### -ftime-trace=e -ftime-trace-granularity=0 d/a.cpp d/b.c -o f/x -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=LINK3
+// LINK3: -cc1{{.*}} "-ftime-trace=e/a-{{[^.]*}}.json"
+// LINK3: -cc1{{.*}} "-ftime-trace=e/b-{{[^.]*}}.json"
+
+// RUN: %clang -### -ftime-trace=e -ftime-trace-granularity=1 -xassembler d/a.cpp 2>&1 | FileCheck %s --check-prefix=UNUSED
+// UNUSED: warning: argument unused during compilation: '-ftime-trace=e'
+// UNUSED: warning: argument unused during compilation: '-ftime-trace-granularity=1'
+
 template 
 struct Struct {
   T Num;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6344,13 +6344,15 @@
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report_EQ);
-  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
-  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
-  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_EQ);
 

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

Does GCC have the same `-ftime-trace=` option? It seems like it doesn't, as 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92396 is still open?

If so, I am happy with unifying more of the dump/aux handling, and I imagine 
when/if GCC adds the option it will behave similarly.




Comment at: clang/include/clang/Driver/Compilation.h:279
+  void addTimeTraceFile(const char *Name, const JobAction *JA) {
+TimeTraceFiles[JA] = Name;
+  }

If this is overriding previous paths should it be called `setTimeTraceFile`?



Comment at: clang/lib/Driver/Driver.cpp:5253
+Path = DumpDir->getValue();
+Path += llvm::sys::path::filename(BaseInput);
+  } else {

Why `+=` instead of `append`? Do we just know the value of `dumpdir` is 
terminated with the path separator?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282

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

[PATCH] D149642: [RISCV] Support vreinterpret intrinsics between vector boolean type and m1 vector integer type

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:2042
+
+  SmallVector Operands;
+  if (ResultType->isIntOrIntVectorTy(1)) {

This SmallVector is unused


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149642

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

[PATCH] D150139: [clang-repl] Enable basic multiline support.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/test/Interpreter/multiline.cpp:12-17
+void f(int x) \ 
+{   \
+  printf("x=\
+  %d", x); \
+}
+f(i);

v.g.vassilev wrote:
> aaron.ballman wrote:
> > Another fun test case:
> > ```
> > // Requires -ftrigraphs but the following line ends with a backslash 
> > (surprise!)
> > i=??/
> >   12;
> > ```
> Yes, the implementation of the multiline support here is actually rather 
> rudimentary. It intentionally does not include deeper language understanding 
> but provides a way for the users typing "well-behaved" code to tell 
> clang-repl that more is coming before it could compile it. In theory we could 
> check for the `??/` trigraph and do the same but I don't think that would be 
> used people use clang-repl on things like IBM 3270 terminals which seem not 
> to have some characters and trigraphs could help there ;) 
> 
> Our full-fledged solution is described here 
> https://discourse.llvm.org/t/rfc-flexible-lexer-buffering-for-handling-incomplete-input-in-interactive-c-c/64180/9
> 
> Until that lands we can have this to unblock work on things like OpenMP 
> support.
Okay, that's reasonable enough for the initial commit. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D150139

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D147844#4329497 , @aaron.ballman 
wrote:

> In general, I think this is incremental progress on the diagnostic behavior. 
> However, it's clear that there is room for interpretation on what is or is 
> not a false positive diagnostic for this,

I hope we can agree on what a false positive is here - when the warning fires 
but the code is what the developer intended (ie: the existing code with the 
existing language semantics produce the desired result, the "fix" is to add 
parentheses that explicitly encode the language's existing rules/behavior 
anyway).

Not that we don't have warnings that do this - that encourage parens to 
reinforce what the language already does to be more explicit/intentional about 
it, and in some cases it's not that uncommon that the user will be adding 
parens that reinforce the precedence rules anyway.

Like, I think all the fixes in libc++, llvm, etc, are false positives? (none of 
them found bugs/unintended behavior) Are there any examples of bugs being found 
by this warning in a codebase? (& how many false positives in such a codebase 
did it also flag?)

> so we should pay close attention to user feedback during the 17.x release 
> cycle. If it seems we're getting significant push back, we may need to come 
> back and rethink.
>
> Specifically, I think we've improved the situation for code like this:
>
>   // `p` is a pointer, `x` is an `int`, and `b` is a `bool`
>   p + x ? 1 : 2; // previously didn't warn, now warns
>   p + b ? 1 : 2; // always warned
>
> Does anyone feel we should not move forward with accepting the patch in its 
> current form?

*goes to look*

Mixed feelings - you're right, incremental improvement/adding missed cases to 
an existing warning (especially if that warning's already stylistic in nature) 
is a lower bar/doesn't necessarily need the false positive assessment. But it 
looks like this case might've been intentionally suppressed in the initial 
warning implementation? (anyone linked to the original warning 
implementation/review/design to check if this was intentional?)

But, yeah, seems marginal enough I don't have strong feelings either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

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

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

LGTM, premerge checks are green AFAICT. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

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

[PATCH] D149986: AMDGPU: Force sc0 and sc1 on stores for gfx940 and gfx941

[Scott Linder via Phabricator via cfe-commits]

scott.linder added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:517-529
+  bool tryForceStoreSC0SC1(const SIMemOpInfo &MOI,
+   MachineBasicBlock::iterator &MI) const override {
+if (ST.hasForceStoreSC0SC1() &&
+(MOI.getInstrAddrSpace() & (SIAtomicAddrSpace::SCRATCH |
+SIAtomicAddrSpace::GLOBAL |
+SIAtomicAddrSpace::OTHER)) !=
+ SIAtomicAddrSpace::NONE) {

It may be a bit more verbose, but I would suggest just having a `bool Changed` 
variable, like other (admittedly more complicated) functions in the unit do. I 
don't think any future maintainer will mis-interpret this, even without the 
comment


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

https://reviews.llvm.org/D149986

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

[PATCH] D150321: [clang] Document extensions from later standards

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, this is awesome -- thank you for the improved docs!




Comment at: clang/docs/LanguageExtensions.rst:1373
 
+Language extensions back-ported to previous standards
+=

Seems to be the style we're using for this level of heading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

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

[PATCH] D145343: [AMDGPU] Emit predefined macro `__AMDGCN_CUMODE__`

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 521396.
yaxunl retitled this revision from "[AMDGPU] Emit predefined macro 
`__AMDGCN_CUMODE`" to "[AMDGPU] Emit predefined macro `__AMDGCN_CUMODE__`".
yaxunl edited the summary of this revision.
yaxunl added a comment.

rename the macro to be consistent with most existing predefined macros for 
amdgcn target


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

https://reviews.llvm.org/D145343

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/Hexagon.h
  clang/test/CodeGenHIP/hip-cumode.hip
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/hip-macros.hip
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/TargetParser/TargetParser.cpp

Index: llvm/lib/TargetParser/TargetParser.cpp
===
--- llvm/lib/TargetParser/TargetParser.cpp
+++ llvm/lib/TargetParser/TargetParser.cpp
@@ -107,21 +107,21 @@
   {{"gfx940"},{"gfx940"},  GK_GFX940,  FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_XNACK|FEATURE_SRAMECC},
   {{"gfx941"},{"gfx941"},  GK_GFX941,  FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_XNACK|FEATURE_SRAMECC},
   {{"gfx942"},{"gfx942"},  GK_GFX942,  FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_XNACK|FEATURE_SRAMECC},
-  {{"gfx1010"},   {"gfx1010"}, GK_GFX1010, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK},
-  {{"gfx1011"},   {"gfx1011"}, GK_GFX1011, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK},
-  {{"gfx1012"},   {"gfx1012"}, GK_GFX1012, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK},
-  {{"gfx1013"},   {"gfx1013"}, GK_GFX1013, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK},
-  {{"gfx1030"},   {"gfx1030"}, GK_GFX1030, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1031"},   {"gfx1031"}, GK_GFX1031, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1032"},   {"gfx1032"}, GK_GFX1032, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1033"},   {"gfx1033"}, GK_GFX1033, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1034"},   {"gfx1034"}, GK_GFX1034, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1035"},   {"gfx1035"}, GK_GFX1035, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1036"},   {"gfx1036"}, GK_GFX1036, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1100"},   {"gfx1100"}, GK_GFX1100, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1101"},   {"gfx1101"}, GK_GFX1101, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1102"},   {"gfx1102"}, GK_GFX1102, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
-  {{"gfx1103"},   {"gfx1103"}, GK_GFX1103, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
+  {{"gfx1010"},   {"gfx1010"}, GK_GFX1010, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK|FEATURE_WGP},
+  {{"gfx1011"},   {"gfx1011"}, GK_GFX1011, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK|FEATURE_WGP},
+  {{"gfx1012"},   {"gfx1012"}, GK_GFX1012, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK|FEATURE_WGP},
+  {{"gfx1013"},   {"gfx1013"}, GK_GFX1013, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_XNACK|FEATURE_WGP},
+  {{"gfx1030"},   {"gfx1030"}, GK_GFX1030, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1031"},   {"gfx1031"}, GK_GFX1031, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1032"},   {"gfx1032"}, GK_GFX1032, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1033"},   {"gfx1033"}, GK_GFX1033, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1034"},   {"gfx1034"}, GK_GFX1034, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1035"},   {"gfx1035"}, GK_GFX1035, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1036"},   {"gfx1036"}, GK_GFX1036, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1100"},   {"gfx1100"}, GK_GFX1100, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1101"},   {"gfx1101"}, GK_GFX1101, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32|FEATURE_WGP},
+  {{"gfx1102"},   {"gfx1102"}, G

[clang] b09fad7 - [clang] Document extensions from later standards

[Nikolas Klauser via cfe-commits]

Author: Nikolas Klauser
Date: 2023-05-11T11:54:46-07:00
New Revision: b09fad7f8e9ce5b88fb467be012ea379efa3659d

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

LOG: [clang] Document extensions from later standards

Reviewed By: aaron.ballman

Spies: H-G-Hristov, cfe-commits

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index ddd366b637e59..64ed3ae6ab907 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@ For example, compiling code with ``-fmodules`` enables 
the use of Modules.
 
 More information could be found `here 
`_.
 
+Language Extensions Back-ported to Previous Standards
+=
+
+===  
= = ==
+Feature Feature Test Macro   
Introduced In Backported To Required Flags
+===  
= = ==
+variadic templates  __cpp_variadic_templates C++11 
C++03
+Alias templates __cpp_alias_templatesC++11 
C++03
+Non-static data member initializers __cpp_nsdmi  C++11 
C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 
C++03
+RValue references   __cpp_rvalue_references  C++11 
C++03
+Attributes  __cpp_attributes C++11 
C++03 -fdouble-square-bracket-attributes
+variable templates  __cpp_variable_templates C++14 
C++03
+Binary literals __cpp_binary_literalsC++14 
C++03
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+``if constexpr``__cpp_if_constexpr   C++17 
C++11
+fold expressions__cpp_fold_expressions   C++17 
C++03
+Lambda capture of \*this by value   __cpp_capture_star_this  C++17 
C++11
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03
+Hexadecimal floating literals   __cpp_hex_float  C++17 
C++03
+``inline`` variables__cpp_inline_variables   C++17 
C++03
+Attributes on namespaces__cpp_namespace_attributes   C++17 
C++11
+Structured bindings __cpp_structured_bindingsC++17 
C++03
+template template arguments __cpp_template_template_args C++17 
C++03
+``static operator[]``   __cpp_multidimensional_subscript C++20 
C++03
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Conditional ``explicit``__cpp_conditional_explicit   C++20 
C++03
+``using enum``  __cpp_using_enum C++20 
C++03
+``if consteval``__cpp_if_consteval   C++23 
C++20
+``static operator()``   __cpp_static_call_operator   C++23 
C++03
+---  
- - --
+Designated initializers  C99   
C89
+===  
= = ==
+
 Type Trait Primitives
 =
 



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

[PATCH] D150321: [clang] Document extensions from later standards

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
philnik marked an inline comment as done.
Closed by commit rGb09fad7f8e9c: [clang] Document extensions from later 
standards (authored by philnik).

Changed prior to commit:
  https://reviews.llvm.org/D150321?vs=521375&id=521402#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here 
`_.
 
+Language Extensions Back-ported to Previous Standards
+=
+
+===  
= = ==
+Feature Feature Test Macro   
Introduced In Backported To Required Flags
+===  
= = ==
+variadic templates  __cpp_variadic_templates C++11 
C++03
+Alias templates __cpp_alias_templatesC++11 
C++03
+Non-static data member initializers __cpp_nsdmi  C++11 
C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 
C++03
+RValue references   __cpp_rvalue_references  C++11 
C++03
+Attributes  __cpp_attributes C++11 
C++03 -fdouble-square-bracket-attributes
+variable templates  __cpp_variable_templates C++14 
C++03
+Binary literals __cpp_binary_literalsC++14 
C++03
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+``if constexpr``__cpp_if_constexpr   C++17 
C++11
+fold expressions__cpp_fold_expressions   C++17 
C++03
+Lambda capture of \*this by value   __cpp_capture_star_this  C++17 
C++11
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03
+Hexadecimal floating literals   __cpp_hex_float  C++17 
C++03
+``inline`` variables__cpp_inline_variables   C++17 
C++03
+Attributes on namespaces__cpp_namespace_attributes   C++17 
C++11
+Structured bindings __cpp_structured_bindingsC++17 
C++03
+template template arguments __cpp_template_template_args C++17 
C++03
+``static operator[]``   __cpp_multidimensional_subscript C++20 
C++03
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Conditional ``explicit``__cpp_conditional_explicit   C++20 
C++03
+``using enum``  __cpp_using_enum C++20 
C++03
+``if consteval``__cpp_if_consteval   C++23 
C++20
+``static operator()``   __cpp_static_call_operator   C++23 
C++03
+---  
- - --
+Designated initializers  C99   
C89
+===  
= = ==
+
 Type Trait Primitives
 =
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here `_.
 
+Language Extensions Back-ported to Previous Standards
+=
+
+===  = = ==
+Feature Feature Test Macro   Introduced In Backported To Required Flags
+===  = = ==
+variadic templates  __cpp_variadic_templates C++11 C++03
+Alias templates __cpp_alias_temp

[PATCH] D148381: [WIP][Clang] Add element_count attribute

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 521406.
void added a comment.

Use strings for the attribute argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -61,6 +61,7 @@
 // CHECK-NEXT: DiagnoseAsBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: ElementCount (SubjectMatchRule_field)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnforceTCBLeaf (SubjectMatchRule_function, SubjectMatchRule_objc_method)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8238,6 +8238,29 @@
   D->addAttr(ZeroCallUsedRegsAttr::Create(S.Context, Kind, AL));
 }
 
+static void handleElementCountAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // TODO: Probably needs more processing here. See Sema::AddAlignValueAttr.
+  SmallVector Names;
+  SmallVector Ranges;
+
+  for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
+StringRef Field;
+SourceLocation FieldLoc;
+
+if (!S.checkStringLiteralArgumentAttr(AL, I, Field, &FieldLoc))
+  return;
+
+Names.push_back(Field);
+Ranges.push_back(FieldLoc);
+  }
+
+  ElementCountAttr *ECA = ::new (S.Context) ElementCountAttr(S.Context, AL,
+ Names.data(),
+ Names.size());
+  ECA->addCountFieldSourceRange(Ranges);
+  D->addAttr(ECA);
+}
+
 static void handleFunctionReturnThunksAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
   StringRef KindStr;
@@ -9136,6 +9159,9 @@
   case ParsedAttr::AT_FunctionReturnThunks:
 handleFunctionReturnThunksAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_ElementCount:
+handleElementCountAttr(S, D, AL);
+break;
 
   // Microsoft attributes:
   case ParsedAttr::AT_LayoutVersion:
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -17692,6 +17692,44 @@
  "Broken injected-class-name");
 }
 
+static const FieldDecl *FindFieldWithElementCountAttr(const RecordDecl *RD) {
+  for (const Decl *D : RD->decls()) {
+if (const auto *FD = dyn_cast(D))
+  if (FD->hasAttr())
+return FD;
+
+if (const auto *SubRD = dyn_cast(D))
+  if (const FieldDecl *FD = FindFieldWithElementCountAttr(SubRD))
+return FD;
+  }
+
+  return nullptr;
+}
+
+static StringRef
+CheckElementCountAttr(const RecordDecl *RD, const FieldDecl *FD,
+  SourceRange &Loc) {
+  const ElementCountAttr *ECA = FD->getAttr();
+  unsigned Idx = 0;
+
+  for (StringRef Field : ECA->elementCountFields()) {
+Loc = ECA->getCountFieldSourceRange(Idx++);
+
+auto DeclIter = llvm::find_if(
+RD->fields(), [&](const FieldDecl *FD){
+  return Field == FD->getName();
+});
+
+if (DeclIter == RD->field_end())
+  return Field;
+
+if (auto *SubRD = DeclIter->getType()->getAsRecordDecl())
+  RD = SubRD;
+  }
+
+  return StringRef();
+}
+
 void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
 SourceRange BraceRange) {
   AdjustDeclIfTemplate(TagD);
@@ -17749,6 +17787,19 @@
  [](const FieldDecl *FD) { return FD->isBitField(); }))
   Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
   }
+
+  // Check the "element_count" attribute to ensure that the count field exists
+  // in the struct.
+  if (const RecordDecl *RD = dyn_cast(Tag)) {
+if (const FieldDecl *FD = FindFieldWithElementCountAttr(RD)) {
+  SourceRange SR;
+  StringRef Unknown = CheckElementCountAttr(RD, FD, SR);
+
+  if (!Unknown.empty())
+Diag(SR.getBegin(), diag::warn_element_count_placeholder)
+<< Unknown << SR;
+}
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: 

[PATCH] D146342: [-Wunsafe-buffer-usage] Move the whole analysis to the end of a translation unit

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 updated this revision to Diff 521409.
ziqingluo-90 added a comment.

Address comments:   refactor the callable-definition visitor to take a lambda 
Callback, who calls various analyses that need whole-TU information.If one 
needs to add a new such analysis later,  just add a call in the Callback 
function.


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

https://reviews.llvm.org/D146342

Files:
  clang/include/clang/Sema/AnalysisBasedWarnings.h
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/Sema.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -220,11 +220,11 @@
 void testTemplate(int * p) {
   int *a[10];
   foo(f(p, &p, a, a)[1]); // expected-warning{{unsafe buffer access}}
-  // expected-note@-1{{in instantiation of function template specialization 'f' requested here}}
+  // FIXME: expected note@-1{{in instantiation of function template specialization 'f' requested here}}
 
   const int **q = const_cast(&p);
 
-  testPointerArithmetic(p, q, p); //expected-note{{in instantiation of}}
+  testPointerArithmetic(p, q, p); //FIXME: expected note{{in instantiation of}}
 }
 
 void testPointerToMember() {
@@ -315,7 +315,7 @@
   foo(ar[5]);   // expected-note{{used in buffer access here}}
 }
 
-template void fArr(int t[]); // expected-note {{in instantiation of}}
+template void fArr(int t[]); // FIXME: expected note {{in instantiation of}}
 
 int testReturn(int t[]) {
   // expected-warning@-1{{'t' is an unsafe pointer used for buffer access}}
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1426,6 +1426,8 @@
 }
   }
 
+  AnalysisWarnings.IssueWarnings(Context.getTranslationUnitDecl());
+
   // Check we've noticed that we're no longer parsing the initializer for every
   // variable. If we miss cases, then at best we have a performance issue and
   // at worst a rejects-valid bug.
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -13,6 +13,7 @@
 //===--===//
 
 #include "clang/Sema/AnalysisBasedWarnings.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
@@ -25,6 +26,8 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
 #include "clang/Analysis/Analyses/CalledOnceCheck.h"
 #include "clang/Analysis/Analyses/Consumed.h"
@@ -35,6 +38,7 @@
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/CFGStmtMap.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Preprocessor.h"
@@ -43,6 +47,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -2290,6 +2295,88 @@
 S.Diag(D.Loc, D.PD);
 }
 
+// An AST Visitor that calls a callback function on each callable DEFINITION
+// that is NOT in a dependent context:
+class CallableVisitor : public RecursiveASTVisitor {
+private:
+  llvm::function_ref Callback;
+
+public:
+  CallableVisitor(llvm::function_ref Callback)
+  : Callback(Callback) {}
+
+  bool VisitFunctionDecl(FunctionDecl *Node) {
+if (cast(Node)->isDependentContext())
+  return true; // Not to analyze dependent decl
+// `FunctionDecl->hasBody()` returns true if the function has a body
+// somewhere defined.  But we want to know if this `Node` has a body
+// child.  So we use `doesThisDeclarationHaveABody`:
+if (Node->doesThisDeclarationHaveABody())
+  Callback(Node);
+return true;
+  }
+
+  bool VisitBlockDecl(BlockDecl *Node) {
+if (cast(Node)->isDependentContext())
+  return true; // Not to analyze dependent decl
+Callback(Node);
+return true;
+  }
+
+  bool VisitObjCMethodDecl(ObjCMethodDecl *Node) {
+if (cast(Node)->isDependentContext())
+  return true; // Not to analyze dependent decl
+if (Node->hasBody())
+  Callback(Node);
+return true;
+  }
+
+  bool VisitLambdaExpr(LambdaExpr *Node) {
+return VisitFunctionDecl(Node->getCallOperator());
+  }
+
+  bool shouldVisitTem

[PATCH] D150258: [clang][parser] Fix namespace dropping after malformed declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! FWIW, I verified that the precommit CI failures 
are unrelated to this patch.

Can you add a release note to `clang/docs/ReleaseNotes.rst` about the fix?




Comment at: clang/lib/Parse/ParseDecl.cpp:2179-2180
 
-if (isDeclarationSpecifier(ImplicitTypenameContext::No)) {
+if (isDeclarationSpecifier(ImplicitTypenameContext::No) ||
+Tok.is(tok::kw_namespace)) {
   // If there is an invalid declaration specifier right after the

It'd help to update the comment below since it doesn't mention why namespaces 
are handled specially.



Comment at: clang/lib/Parse/ParseDecl.cpp:2306-2310
 // Otherwise things are very confused and we skip to recover.
 if (!isDeclarationSpecifier(ImplicitTypenameContext::No)) {
-  SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
-  TryConsumeToken(tok::semi);
+  SkipMalformedDecl();
 }
   }

Changes for our coding style.

There seems to be some unfortunate interplay here though. Consider:
```
void bar() namespace foo { int i; }

int main() {
  foo::i = 12;
}
```
Calling `SkipMalformedDecl()` changes the behavior for this test case because 
we don't recover as well. With your patch applied, this gives us two 
diagnostics:
```
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:11: error: 
expected ';' after top level declarator
void bar() namespace foo { int i; }
  ^
  ;
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:3: error: use 
of undeclared identifier 'foo'
  foo::i = 12;
  ^
2 errors generated.
```
If the `namespace` keyword is on its own line, then we recover gracefully and 
don't emit the "use of undeclared identifier" warning.

While this is technically a regression in behavior for this test case, I think 
the overall changes are still an improvement. I suspect not a whole lot of code 
puts `namespace` somewhere other than the start of a line (same for `inline 
namespace` which has the same behavior with your patch).



Comment at: clang/lib/Parse/ParseTemplate.cpp:278
   if (!DeclaratorInfo.hasName()) {
-// If so, skip until the semi-colon or a }.
-SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
-if (Tok.is(tok::semi))
-  ConsumeToken();
+SkipMalformedDecl();
 return nullptr;

We'll have the same sort of recovery issues here unless the namespace is on its 
own line, but that also seems like a highly unlikely scenario.



Comment at: clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp:1
+// RUN: %clang_cc1 -verify %s
+





Comment at: clang/test/Parser/cxx-template-recovery.cpp:1
+// RUN: %clang_cc1 -verify %s
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150258

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

[PATCH] D150340: [SEH]:Fix assertion when try is used inside catch(...) block with /EHa

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGException.cpp:650
+llvm::FunctionCallee SehCppScope = getSehTryBeginFn(CGM);
+EmitSehScope(SehCppScope);
+  }

Do we need to make the same change in EmitSEHTryStmt/ExitSEHTryStmt?

Is there some reason not to just call EmitSehTryScopeBegin here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150340

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

[PATCH] D150352: [clang][dataflow] Don't analyze templated declarations.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:36
+  /// `S` resides. `D.isTemplated()` must be false.
+  static llvm::Expected build(const Decl &D, Stmt &S,
+  ASTContext &C);

Maybe this is too much for a drive-by fix, but since you are changing this API 
anyway, I have to ask - now that D is nonnull, isn't S strictly redundant? 
Isn't D always a FunctionDecl, and S its body?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150352

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

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 521427.
MaskRay marked an inline comment as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.

add an assert to addTimeTraceFile.
improve a -dumpdir test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282

Files:
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ftime-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -213,9 +213,7 @@
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
 Argv, Diags, Argv0);
 
-  if (Clang->getFrontendOpts().TimeTrace ||
-  !Clang->getFrontendOpts().TimeTracePath.empty()) {
-Clang->getFrontendOpts().TimeTrace = 1;
+  if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
 llvm::timeTraceProfilerInitialize(
 Clang->getFrontendOpts().TimeTraceGranularity, Argv0);
   }
@@ -257,16 +255,6 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
-
 // It is possible that the compiler instance doesn't own a file manager here
 // if we're compiling a module unit. Since the file manager are owned by AST
 // when we're compiling a module unit. So the file manager may be invalid
@@ -280,7 +268,8 @@
   Clang->getInvocation(), Clang->getDiagnostics()));
 
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+Clang->getFrontendOpts().TimeTracePath, /*Binary=*/false,
+/*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/ftime-trace.cpp
===
--- clang/test/Driver/ftime-trace.cpp
+++ clang/test/Driver/ftime-trace.cpp
@@ -31,6 +31,35 @@
 // CHECK:  "name": "process_name"
 // CHECK:  "name": "thread_name"
 
+// RUN: mkdir d e f && cp %s d/a.cpp && touch d/b.c
+
+// RUN: %clang -### -c -ftime-trace -ftime-trace-granularity=0 d/a.cpp -o e/a.o 2>&1 | FileCheck %s --check-prefix=COMPILE1
+// COMPILE1: -cc1{{.*}} "-ftime-trace=e/a.json" "-ftime-trace-granularity=0"
+
+// RUN: %clang -### -c -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=COMPILE2
+// COMPILE2: -cc1{{.*}} "-ftime-trace=f/a.json" "-ftime-trace-granularity=0"
+// COMPILE2: -cc1{{.*}} "-ftime-trace=f/b.json" "-ftime-trace-granularity=0"
+
+/// -o specifies the link output. Create ${output}-${basename}.json.
+// RUN: %clang -### -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -o e/x 2>&1 | FileCheck %s --check-prefix=LINK1
+// LINK1: -cc1{{.*}} "-ftime-trace=e/x-a.json" "-ftime-trace-granularity=0"
+// LINK1: -cc1{{.*}} "-ftime-trace=e/x-b.json" "-ftime-trace-granularity=0"
+
+/// -dumpdir is f/g, not ending with a path separator. We create f/g${basename}.json.
+// RUN: %clang -### -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -o e/x -dumpdir f/g 2>&1 | FileCheck %s --check-prefix=LINK2
+// LINK2: -cc1{{.*}} "-ftime-trace=f/ga.json" "-ftime-trace-granularity=0"
+// LINK2: -cc1{{.*}} "-ftime-trace=f/gb.json" "-ftime-trace-granularity=0"
+
+// RUN: %clang -### -ftime-trace=e -ftime-trace-granularity=0 d/a.cpp d/b.c -o f/x -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=LINK3
+// LINK3: -cc1{{.*}} "-ftime-trace=e/a-{{[^.]*}}.json" "-ftime-trace-granularity=0"
+// LINK3: -cc1{{.*}} "-ftime-trace=e/b-{{[^.]*}}.json" "-ftime-trace-granularity=0"
+
+// RUN: %clang -### -ftime-trace -ftime-trace=e -ftime-trace-granularity=1 -xassembler d/a.cpp 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=UNUSED --implicit-check-not=warning:
+// UNUSED: warning: argument unused during compilation: '-ftime-trace'
+// UNUSED: warning: argument unused during compilation: '-ftime-trace=e'
+// UNUSED: warning: argument unused during compilation: '-ftime-trace-granularity=1'
+
 template 
 struct Struct {
   T Num;
Index: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst:117
+
+.. option:: PrintFunction
+

Is `PrintFunction` (and the soon-to-arrive `PrintlnFunction`) distinct enough 
from `PrintfLikeFunctions` and `FprintfLikeFunctions`? Should I use 
`ReplacementPrintFunction` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149280

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

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D150282#4335568 , @scott.linder 
wrote:

> Does GCC have the same `-ftime-trace=` option? It seems like it doesn't, as 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92396 is still open?

You found it!

I am trying to collect other auxiliary output file features in Clang and add 
them to https://maskray.me/blog/2023-04-25-compiler-output-files

> If so, I am happy with unifying more of the dump/aux handling, and I imagine 
> when/if GCC adds the option it will behave similarly.

Thanks.




Comment at: clang/include/clang/Driver/Compilation.h:279
+  void addTimeTraceFile(const char *Name, const JobAction *JA) {
+TimeTraceFiles[JA] = Name;
+  }

scott.linder wrote:
> If this is overriding previous paths should it be called `setTimeTraceFile`?
The naming is to match `add*File` above.
Do you want an assert that the entry hasn't been inserted before?



Comment at: clang/lib/Driver/Driver.cpp:5253
+Path = DumpDir->getValue();
+Path += llvm::sys::path::filename(BaseInput);
+  } else {

scott.linder wrote:
> Why `+=` instead of `append`? Do we just know the value of `dumpdir` is 
> terminated with the path separator?
`-dumpdir ` is a bit misnomer that it may or may not end with a path separator.

`clang -c -ftime-trace d/a.c -o e/xa.o -dumpdir f/` is intended to create 
`fa.json`

I updated a test and the description to give an example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282

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

[PATCH] D150340: [SEH]:Fix assertion when try is used inside catch(...) block with /EHa

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/CodeGen/CGException.cpp:650
+llvm::FunctionCallee SehCppScope = getSehTryBeginFn(CGM);
+EmitSehScope(SehCppScope);
+  }

efriedma wrote:
> Do we need to make the same change in EmitSEHTryStmt/ExitSEHTryStmt?
> 
> Is there some reason not to just call EmitSehTryScopeBegin here?
EmitSehTryScopeBegin:  it is emit seh.scope.begin

In here we want to emit seh.try.begin.  call EmitSehSCope with different 
function.

For EmitSEHTryStmt/ExitSEHTryStmt if is for __try /__except/__finally and it is 
for C code.  I thought about that.  And tried some test(BTW, try and __try can 
not be in same construct), I don't see the problem.  So I did not add that. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150340

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

[clang] 027aeec - [Clang] Respect `-L` options when compiling directly for AMDGPU

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2023-05-11T16:02:54-05:00
New Revision: 027aeec7da67afe33b159f5e42dec57488897454

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

LOG: [Clang] Respect `-L` options when compiling directly for AMDGPU

The AMDGPU linker is `lld`, which has full support for standard features
like static libraries. Previously the AMDGPU toolchain did not forward
`-L` arguments so we could not tell it where to find certain libraries.
This patch simply forwards it like the other toolchains.

Reviewed By: yaxunl, MaskRay

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/amdgpu-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index a6247b4cbe4ea..189c99e18572a 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -232,9 +232,11 @@ void tools::AddLinkerInputs(const ToolChain &TC, const 
InputInfoList &Inputs,
   Args.AddAllArgValues(CmdArgs, options::OPT_Zlinker_input);
 
   // LIBRARY_PATH are included before user inputs and only supported on native
-  // toolchains.
+  // toolchains. Otherwise only add the '-L' arguments requested by the user.
   if (!TC.isCrossCompiling())
 addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
+  else
+Args.AddAllArgs(CmdArgs, options::OPT_L);
 
   for (const auto &II : Inputs) {
 // If the current tool chain refers to an OpenMP offloading host, we

diff  --git a/clang/test/Driver/amdgpu-toolchain.c 
b/clang/test/Driver/amdgpu-toolchain.c
index b8b6667333d81..288dbbedd49d5 100644
--- a/clang/test/Driver/amdgpu-toolchain.c
+++ b/clang/test/Driver/amdgpu-toolchain.c
@@ -11,6 +11,6 @@
 // DWARF_VER: "-dwarf-version=5"
 
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
-// RUN:   -flto -fconvergent-functions %s 2>&1 | FileCheck -check-prefix=LTO %s
+// RUN:   -L. -flto -fconvergent-functions %s 2>&1 | FileCheck 
-check-prefix=LTO %s
 // LTO: clang{{.*}} "-flto=full"{{.*}}"-fconvergent-functions"
-// LTO: ld.lld{{.*}}-plugin-opt=mcpu=gfx906
+// LTO: ld.lld{{.*}}"-L."{{.*}}"-plugin-opt=mcpu=gfx906"



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

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG027aeec7da67: [Clang] Respect `-L` options when compiling 
directly for AMDGPU (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150013

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/amdgpu-toolchain.c


Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -11,6 +11,6 @@
 // DWARF_VER: "-dwarf-version=5"
 
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
-// RUN:   -flto -fconvergent-functions %s 2>&1 | FileCheck -check-prefix=LTO %s
+// RUN:   -L. -flto -fconvergent-functions %s 2>&1 | FileCheck 
-check-prefix=LTO %s
 // LTO: clang{{.*}} "-flto=full"{{.*}}"-fconvergent-functions"
-// LTO: ld.lld{{.*}}-plugin-opt=mcpu=gfx906
+// LTO: ld.lld{{.*}}"-L."{{.*}}"-plugin-opt=mcpu=gfx906"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -232,9 +232,11 @@
   Args.AddAllArgValues(CmdArgs, options::OPT_Zlinker_input);
 
   // LIBRARY_PATH are included before user inputs and only supported on native
-  // toolchains.
+  // toolchains. Otherwise only add the '-L' arguments requested by the user.
   if (!TC.isCrossCompiling())
 addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
+  else
+Args.AddAllArgs(CmdArgs, options::OPT_L);
 
   for (const auto &II : Inputs) {
 // If the current tool chain refers to an OpenMP offloading host, we


Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -11,6 +11,6 @@
 // DWARF_VER: "-dwarf-version=5"
 
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
-// RUN:   -flto -fconvergent-functions %s 2>&1 | FileCheck -check-prefix=LTO %s
+// RUN:   -L. -flto -fconvergent-functions %s 2>&1 | FileCheck -check-prefix=LTO %s
 // LTO: clang{{.*}} "-flto=full"{{.*}}"-fconvergent-functions"
-// LTO: ld.lld{{.*}}-plugin-opt=mcpu=gfx906
+// LTO: ld.lld{{.*}}"-L."{{.*}}"-plugin-opt=mcpu=gfx906"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -232,9 +232,11 @@
   Args.AddAllArgValues(CmdArgs, options::OPT_Zlinker_input);
 
   // LIBRARY_PATH are included before user inputs and only supported on native
-  // toolchains.
+  // toolchains. Otherwise only add the '-L' arguments requested by the user.
   if (!TC.isCrossCompiling())
 addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
+  else
+Args.AddAllArgs(CmdArgs, options::OPT_L);
 
   for (const auto &II : Inputs) {
 // If the current tool chain refers to an OpenMP offloading host, we
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Scott Linder via Phabricator via cfe-commits]

scott.linder added inline comments.



Comment at: clang/include/clang/Driver/Compilation.h:279
+  void addTimeTraceFile(const char *Name, const JobAction *JA) {
+TimeTraceFiles[JA] = Name;
+  }

MaskRay wrote:
> scott.linder wrote:
> > If this is overriding previous paths should it be called `setTimeTraceFile`?
> The naming is to match `add*File` above.
> Do you want an assert that the entry hasn't been inserted before?
If you think it is useful; otherwise consistency with the other options seems 
like a good enough reason



Comment at: clang/lib/Driver/Driver.cpp:5253
+Path = DumpDir->getValue();
+Path += llvm::sys::path::filename(BaseInput);
+  } else {

MaskRay wrote:
> scott.linder wrote:
> > Why `+=` instead of `append`? Do we just know the value of `dumpdir` is 
> > terminated with the path separator?
> `-dumpdir ` is a bit misnomer that it may or may not end with a path 
> separator.
> 
> `clang -c -ftime-trace d/a.c -o e/xa.o -dumpdir f/` is intended to create 
> `fa.json`
> 
> I updated a test and the description to give an example.
Thanks, I just forgot this point since the previous discussions! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282

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