[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton updated this revision to Diff 468793.
hel-ableton added a comment.

Sorry about the plus sign! I'm fairly unfamiliar with your process, so 
something between making a diff and making a patch must have gone wrong. About 
the brackets, I was wondering why there weren't any, but didn't see the need to 
add them given the change. In any case, now there are brackets.


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

https://reviews.llvm.org/D136154

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -810,8 +810,9 @@
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
 if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
-+   Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment)
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;
+}
   } else if (Previous.is(TT_InheritanceColon)) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -810,8 +810,9 @@
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
 if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
-+   Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment)
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;
+}
   } else if (Previous.is(TT_InheritanceColon)) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton added a comment.

In D136154#3867328 , @MyDeveloperDay 
wrote:

> Can we log a GitHub issue I can’t see what you are trying to fix

I'm sorry if this is unclear. The background to this is that our repository is 
currently formatted using clang-format 6.0.0, and we regard this as a bug that 
must have been introduced somewhere between 7.1.0 and 10.0.0, which is 
resulting in unnecessary reformatting of a lot of code, and is what kept us 
from upgrading clang-format for quite a while now. Other circumstances now 
force us to upgrade, though, and we would like to avoid introducing these 
changes, not only for diff noise, but also because we don't think the 
indentation makes sense that way.

A GitHub issue would be fine by me, also I could use some help with how to 
effectively work with the llvm-project repo, as currently I'm suffering a bit 
from huge turnaround times, like having to recompile lots and lots of code if I 
want to compare the result of formatting with or without the proposed fix.


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

https://reviews.llvm.org/D136154

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

[PATCH] D136229: [include-cleaner] Fix link errors when -DBUILD_SHARED_LIBS=ON

[Sam McCall via Phabricator via cfe-commits]

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

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136229

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton added a comment.

In D136154#3867328 , @MyDeveloperDay 
wrote:

> Can we log a GitHub issue I can’t see what you are trying to fix

Without the fix, the arguments to the Base class would be on the same level as 
the Base class itself. But this also effects other code. I'll attach too 
screenshots, the diffs are going from with the fix to without the fix.

F24964948: InheritanceArguments.png 

F24964955: Ternary.png 


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

https://reviews.llvm.org/D136154

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

[clang-tools-extra] 2e73129 - [include-cleaner] Fix link errors when -DBUILD_SHARED_LIBS=ON

[Kai Luo via cfe-commits]

Author: Kai Luo
Date: 2022-10-19T07:26:08Z
New Revision: 2e73129483c4be78d32f4bbe3f9a3130d9fc83b7

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

LOG: [include-cleaner] Fix link errors when -DBUILD_SHARED_LIBS=ON

Fixed ppc buildbot https://lab.llvm.org/buildbot/#/builders/121/builds/24273 
which is using `-DBUILD_SHARED_LIBS=ON`.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/CMakeLists.txt
clang-tools-extra/include-cleaner/tool/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt 
b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
index f65f50c243b86..b255abca7499d 100644
--- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -6,7 +6,8 @@ add_clang_library(clangIncludeCleaner
   WalkAST.cpp
 
   LINK_LIBS
-  clangBasic
   clangAST
+  clangBasic
+  clangLex
   )
 

diff  --git a/clang-tools-extra/include-cleaner/tool/CMakeLists.txt 
b/clang-tools-extra/include-cleaner/tool/CMakeLists.txt
index bd407aec847d9..3fc8c44081cd2 100644
--- a/clang-tools-extra/include-cleaner/tool/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/tool/CMakeLists.txt
@@ -4,6 +4,8 @@ include_directories("../lib") # FIXME: use public APIs instead.
 add_clang_tool(clang-include-cleaner IncludeCleaner.cpp)
 clang_target_link_libraries(clang-include-cleaner PRIVATE
   clangBasic
+  clangFrontend
+  clangSerialization
   clangTooling
   )
 target_link_libraries(clang-include-cleaner PRIVATE



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

[PATCH] D136229: [include-cleaner] Fix link errors when -DBUILD_SHARED_LIBS=ON

[Kai Luo via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2e73129483c4: [include-cleaner] Fix link errors when 
-DBUILD_SHARED_LIBS=ON (authored by lkail).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136229

Files:
  clang-tools-extra/include-cleaner/lib/CMakeLists.txt
  clang-tools-extra/include-cleaner/tool/CMakeLists.txt


Index: clang-tools-extra/include-cleaner/tool/CMakeLists.txt
===
--- clang-tools-extra/include-cleaner/tool/CMakeLists.txt
+++ clang-tools-extra/include-cleaner/tool/CMakeLists.txt
@@ -4,6 +4,8 @@
 add_clang_tool(clang-include-cleaner IncludeCleaner.cpp)
 clang_target_link_libraries(clang-include-cleaner PRIVATE
   clangBasic
+  clangFrontend
+  clangSerialization
   clangTooling
   )
 target_link_libraries(clang-include-cleaner PRIVATE
Index: clang-tools-extra/include-cleaner/lib/CMakeLists.txt
===
--- clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -6,7 +6,8 @@
   WalkAST.cpp
 
   LINK_LIBS
-  clangBasic
   clangAST
+  clangBasic
+  clangLex
   )
 


Index: clang-tools-extra/include-cleaner/tool/CMakeLists.txt
===
--- clang-tools-extra/include-cleaner/tool/CMakeLists.txt
+++ clang-tools-extra/include-cleaner/tool/CMakeLists.txt
@@ -4,6 +4,8 @@
 add_clang_tool(clang-include-cleaner IncludeCleaner.cpp)
 clang_target_link_libraries(clang-include-cleaner PRIVATE
   clangBasic
+  clangFrontend
+  clangSerialization
   clangTooling
   )
 target_link_libraries(clang-include-cleaner PRIVATE
Index: clang-tools-extra/include-cleaner/lib/CMakeLists.txt
===
--- clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -6,7 +6,8 @@
   WalkAST.cpp
 
   LINK_LIBS
-  clangBasic
   clangAST
+  clangBasic
+  clangLex
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

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

balazske added a comment.

If `StdCLibraryFunctionsChecker` is added as dependency to `StreamChecker` and 
no other thing is changed these tests fail, and I could not find out the reason:
Errors in test **stream.c**:

  error: 'warning' diagnostics expected but not seen: 
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 7: 
Stream pointer might be NULL
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 13: 
Stream pointer might be NULL
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 76: 
Stream pointer might be NULL

Errors in test **stream-error.c**:

  error: 'warning' diagnostics expected but not seen: 
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 97: might be 'indeterminate'
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 100: Stream might be already closed
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 113: Read function called when stream is in EOF state
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 114: Read function called when stream is in EOF state
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 123: FALSE
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 124: FALSE
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 128: FALSE
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 129: TRUE
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 133: TRUE
File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
Line 134: FALSE

Probably the `StdLibraryFunctionsChecker` runs before the `StreamChecker`, and 
runs always after it if no dependency is there? But this does not explain all 
test errors and should cause no errors at all. Adding the dependency itself is 
not enough, `ModelPOSIX` option should be true too. If the option is set to 
true in the test file even more test errors appear, and it does not work even 
when the (StdLibraryFunctionsChecker) checker is added (to the RUN command). 
Without the dependency the tests do not fail if the order of checkers in the 
enabled checkers list is changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135247

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton added a comment.

> must have been introduced somewhere between 7.1.0 and 10.0.0

In fact, someone in our team thought that this must have been introduced 
exactly with this commit:

https://github.com/llvm/llvm-project/commit/4636debc271f09f730697ab6873137a657c828f9


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

https://reviews.llvm.org/D136154

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

[PATCH] D136040: [X86][1/2] Support PREFETCHI instructions

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 468806.
pengfei added a comment.

Remove the dependence to D136145 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136040

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/cpuid.h
  clang/lib/Headers/prfchiintrin.h
  clang/lib/Headers/x86gprintrin.h
  clang/test/CodeGen/X86/prefetchi-builtins.c
  clang/test/Driver/x86-target-features.c
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86DiscriminateMemOps.cpp
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/test/CodeGen/X86/prefetchi.ll
  llvm/test/MC/Disassembler/X86/x86-64.txt
  llvm/test/MC/X86/PREFETCH-64.s

Index: llvm/test/MC/X86/PREFETCH-64.s
===
--- llvm/test/MC/X86/PREFETCH-64.s
+++ llvm/test/MC/X86/PREFETCH-64.s
@@ -168,3 +168,50 @@
 // CHECK: encoding: [0x0f,0x0d,0x12]
 prefetchwt1 (%rdx) 
 
+// CHECK: prefetchit0 485498096
+// CHECK: encoding: [0x0f,0x18,0x3c,0x25,0xf0,0x1c,0xf0,0x1c]
+prefetchit0 485498096
+
+// CHECK: prefetchit0 64(%rdx)
+// CHECK: encoding: [0x0f,0x18,0x7a,0x40]
+prefetchit0 64(%rdx)
+
+// CHECK: prefetchit0 64(%rdx,%rax,4)
+// CHECK: encoding: [0x0f,0x18,0x7c,0x82,0x40]
+prefetchit0 64(%rdx,%rax,4)
+
+// CHECK: prefetchit0 -64(%rdx,%rax,4)
+// CHECK: encoding: [0x0f,0x18,0x7c,0x82,0xc0]
+prefetchit0 -64(%rdx,%rax,4)
+
+// CHECK: prefetchit0 64(%rdx,%rax)
+// CHECK: encoding: [0x0f,0x18,0x7c,0x02,0x40]
+prefetchit0 64(%rdx,%rax)
+
+// CHECK: prefetchit0 (%rdx)
+// CHECK: encoding: [0x0f,0x18,0x3a]
+prefetchit0 (%rdx)
+
+// CHECK: prefetchit1 485498096
+// CHECK: encoding: [0x0f,0x18,0x34,0x25,0xf0,0x1c,0xf0,0x1c]
+prefetchit1 485498096
+
+// CHECK: prefetchit1 64(%rdx)
+// CHECK: encoding: [0x0f,0x18,0x72,0x40]
+prefetchit1 64(%rdx)
+
+// CHECK: prefetchit1 64(%rdx,%rax,4)
+// CHECK: encoding: [0x0f,0x18,0x74,0x82,0x40]
+prefetchit1 64(%rdx,%rax,4)
+
+// CHECK: prefetchit1 -64(%rdx,%rax,4)
+// CHECK: encoding: [0x0f,0x18,0x74,0x82,0xc0]
+prefetchit1 -64(%rdx,%rax,4)
+
+// CHECK: prefetchit1 64(%rdx,%rax)
+// CHECK: encoding: [0x0f,0x18,0x74,0x02,0x40]
+prefetchit1 64(%rdx,%rax)
+
+// CHECK: prefetchit1 (%rdx)
+// CHECK: encoding: [0x0f,0x18,0x32]
+prefetchit1 (%rdx)
Index: llvm/test/MC/Disassembler/X86/x86-64.txt
===
--- llvm/test/MC/Disassembler/X86/x86-64.txt
+++ llvm/test/MC/Disassembler/X86/x86-64.txt
@@ -761,3 +761,9 @@
 
 # CHECK: rdpru
 0x0f,0x01,0xfd
+
+# CHECK: prefetchit0 (%rip)
+0x0f,0x18,0x3d,0x00,0x00,0x00,0x00
+
+# CHECK: prefetchit1 (%rip)
+0x0f,0x18,0x35,0x00,0x00,0x00,0x00
Index: llvm/test/CodeGen/X86/prefetchi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/prefetchi.ll
@@ -0,0 +1,21 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-- -mattr=+prefetchi | FileCheck %s
+
+define dso_local void @t(ptr %ptr) nounwind  {
+; CHECK-LABEL: t:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:prefetchit1 (%rdi)
+; CHECK-NEXT:prefetchit0 (%rdi)
+; CHECK-NEXT:prefetchit1 t(%rip)
+; CHECK-NEXT:prefetchit0 ext(%rip)
+; CHECK-NEXT:retq
+entry:
+  tail call void @llvm.prefetch(ptr %ptr, i32 0, i32 2, i32 0)
+  tail call void @llvm.prefetch(ptr %ptr, i32 0, i32 3, i32 0)
+  tail call void @llvm.prefetch(ptr @t,   i32 0, i32 2, i32 0)
+  tail call void @llvm.prefetch(ptr @ext, i32 0, i32 3, i32 0)
+  ret void
+}
+
+declare dso_local void @ext() nounwind
+declare void @llvm.prefetch(ptr, i32, i32, i32) nounwind
Index: llvm/lib/Target/X86/X86Subtarget.h
===
--- llvm/lib/Target/X86/X86Subtarget.h
+++ llvm/lib/Target/X86/X86Subtarget.h
@@ -221,7 +221,8 @@
 // We implicitly enable these when we have a write prefix supporting cache
 // level OR if we have prfchw, but don't already have a read prefetch from
 // 3dnow.
-return hasSSE1() || (hasPRFCHW() && !hasThreeDNow()) || hasPREFETCHWT1();
+return hasSSE1() || (hasPRFCHW() && !hasThreeDNow()) || hasPREFETCHWT1() ||
+   hasPREFETCHI();
   }
   bool canUseLAHFSAHF() const { return hasLAHFSAHF64() || !is64Bit(); }
   // These are generic getters that OR together all of the thunk types
Index: llvm/lib/Target/X86/X86InstrInfo.td
===
--- llvm/lib/Target/X86/X86InstrInfo.td
+++ llvm/lib/Target/X86/X86InstrInfo.td
@@ -956,6 +9

[PATCH] D136040: [X86][1/2] Support PREFETCHI instructions

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Headers/prfchiintrin.h:16
+/// Loads an instruction sequence containing the specified memory address into
+///all level cache.
+///

craig.topper wrote:
> craig.topper wrote:
> > It looks old that this indented differently than the "Loads" on the line 
> > above.
> That should have said "It looks ODD that this indented differently..."
We use this indention in most existing headers. I guess it's intended to align 
with parameters' description. We follow this rule in the ISAs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136040

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

[PATCH] D136145: [IR][RFC] Restrict read only when cache type of llvm.prefetch is instruction

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:7574
 
-  if (NumArgs > 3)
+  if (NumArgs > 4)
 return Diag(TheCall->getEndLoc(),

craig.topper wrote:
> craig.topper wrote:
> > Not clear to me that we should be changing the definition of 
> > `__builtin_prefetch`.
> > 
> > It wouldn't cost much to add a new builtin for X86 for the new instructions.
> It definitely shouldn't be buried in this patch the way it is currently 
> titled and described.
Sure, I have added a new builtin in D136040. But I don't see any concerns for 
this patch so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136145

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

[PATCH] D136078: [RFC] Use-after-return binary metadata.

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov updated this revision to Diff 468815.
dvyukov added a comment.

WIP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -67,14 +67,14 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {}
+FeatureMask(Feature) {}
 };
-const MetadataInfo MetadataInfo::Covered{"__sanitizer_metadata_covered",
- "sanmd_covered", -1};
-const MetadataInfo MetadataInfo::Atomics{"__sanitizer_metadata_atomics",
- "sanmd_atomics", 0};
+const MetadataInfo MetadataInfo::Covered{
+"__sanitizer_metadata_covered", "sanmd_covered", kSanitizerMetadataNone};
+const MetadataInfo MetadataInfo::Atomics{
+"__sanitizer_metadata_atomics", "sanmd_atomics", kSanitizerMetadataAtomics};
 
 // The only instances of MetadataInfo are the constants above, so a set of
 // them may simply store pointers to them. To deterministically generate code,
@@ -89,11 +89,16 @@
 cl::opt ClEmitAtomics("sanitizer-metadata-atomics",
 cl::desc("Emit PCs for atomic operations."),
 cl::Hidden, cl::init(false));
+cl::opt ClEmitUAR("sanitizer-metadata-uar",
+cl::desc("Emit PCs for start of functions that are "
+ "subject for use-after-return checking"),
+cl::Hidden, cl::init(false));
 
 //===--- Statistics ---===//
 
 STATISTIC(NumMetadataCovered, "Metadata attached to covered functions");
 STATISTIC(NumMetadataAtomics, "Metadata attached to atomics");
+STATISTIC(NumMetadataUAR, "Metadata attached to UAR functions");
 
 //===--===//
 
@@ -102,6 +107,7 @@
 transformOptionsFromCl(SanitizerBinaryMetadataOptions &&Opts) {
   Opts.Covered |= ClEmitCovered;
   Opts.Atomics |= ClEmitAtomics;
+  Opts.UAR |= ClEmitUAR;
   return std::move(Opts);
 }
 
@@ -142,7 +148,8 @@
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
-  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
+  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB,
+ uint32_t &PerInstrFeatureMask);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
@@ -235,16 +242,21 @@
   uint32_t PerInstrFeatureMask = getEnabledPerInstructionFeature();
   // Don't emit unnecessary covered metadata for all functions to save space.
   bool RequiresCovered = false;
-  if (PerInstrFeatureMask) {
+  if (PerInstrFeatureMask || Options.UAR) {
 for (BasicBlock &BB : F)
   for (Instruction &I : BB)
-RequiresCovered |= runOn(I, MIS, MDB);
+RequiresCovered |= runOn(I, MIS, MDB, PerInstrFeatureMask);
   }
 
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kSanitizerMetadataUAR;
+  if (PerInstrFeatureMask & kSanitizerMetadataUAR)
+NumMetadataUAR++;
+
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unambiguously interpret it for
   // modules compiled with SanitizerBinaryMetadata.
-  if (Options.Covered || RequiresCovered) {
+  if (Options.Covered || (PerInstrFeatureMask && RequiresCovered)) {
 NumMetadataCovered++;
 const auto *MI = &MetadataInfo::Covered;
 MIS.insert(MI);
@@ -258,10 +270,25 @@
 }
 
 bool SanitizerBinaryMetadata::runOn(Instruction &I, MetadataInfoSet &MIS,
- 

[PATCH] D136078: [RFC] Use-after-return binary metadata.

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov updated this revision to Diff 468816.
dvyukov added a comment.

WIP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -67,14 +67,14 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {}
+FeatureMask(Feature) {}
 };
-const MetadataInfo MetadataInfo::Covered{"__sanitizer_metadata_covered",
- "sanmd_covered", -1};
-const MetadataInfo MetadataInfo::Atomics{"__sanitizer_metadata_atomics",
- "sanmd_atomics", 0};
+const MetadataInfo MetadataInfo::Covered{
+"__sanitizer_metadata_covered", "sanmd_covered", kSanitizerMetadataNone};
+const MetadataInfo MetadataInfo::Atomics{
+"__sanitizer_metadata_atomics", "sanmd_atomics", kSanitizerMetadataAtomics};
 
 // The only instances of MetadataInfo are the constants above, so a set of
 // them may simply store pointers to them. To deterministically generate code,
@@ -89,11 +89,16 @@
 cl::opt ClEmitAtomics("sanitizer-metadata-atomics",
 cl::desc("Emit PCs for atomic operations."),
 cl::Hidden, cl::init(false));
+cl::opt ClEmitUAR("sanitizer-metadata-uar",
+cl::desc("Emit PCs for start of functions that are "
+ "subject for use-after-return checking"),
+cl::Hidden, cl::init(false));
 
 //===--- Statistics ---===//
 
 STATISTIC(NumMetadataCovered, "Metadata attached to covered functions");
 STATISTIC(NumMetadataAtomics, "Metadata attached to atomics");
+STATISTIC(NumMetadataUAR, "Metadata attached to UAR functions");
 
 //===--===//
 
@@ -102,6 +107,7 @@
 transformOptionsFromCl(SanitizerBinaryMetadataOptions &&Opts) {
   Opts.Covered |= ClEmitCovered;
   Opts.Atomics |= ClEmitAtomics;
+  Opts.UAR |= ClEmitUAR;
   return std::move(Opts);
 }
 
@@ -142,7 +148,8 @@
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
-  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
+  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB,
+ uint32_t &PerInstrFeatureMask);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
@@ -235,16 +242,21 @@
   uint32_t PerInstrFeatureMask = getEnabledPerInstructionFeature();
   // Don't emit unnecessary covered metadata for all functions to save space.
   bool RequiresCovered = false;
-  if (PerInstrFeatureMask) {
+  if (PerInstrFeatureMask || Options.UAR) {
 for (BasicBlock &BB : F)
   for (Instruction &I : BB)
-RequiresCovered |= runOn(I, MIS, MDB);
+RequiresCovered |= runOn(I, MIS, MDB, PerInstrFeatureMask);
   }
 
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kSanitizerMetadataUAR;
+  if (PerInstrFeatureMask & kSanitizerMetadataUAR)
+NumMetadataUAR++;
+
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unambiguously interpret it for
   // modules compiled with SanitizerBinaryMetadata.
-  if (Options.Covered || RequiresCovered) {
+  if (Options.Covered || (PerInstrFeatureMask && RequiresCovered)) {
 NumMetadataCovered++;
 const auto *MI = &MetadataInfo::Covered;
 MIS.insert(MI);
@@ -258,10 +270,25 @@
 }
 
 bool SanitizerBinaryMetadata::runOn(Instruction &I, MetadataInfoSet &MIS,
- 

[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D135247#3867445 , @balazske wrote:

> If `StdCLibraryFunctionsChecker` is added as dependency to `StreamChecker` 
> and no other thing is changed these tests fail, and I could not find out the 
> reason:
> Errors in test **stream.c**:
>
>   error: 'warning' diagnostics expected but not seen: 
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 7: 
> Stream pointer might be NULL
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 
> 13: Stream pointer might be NULL
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 
> 76: Stream pointer might be NULL
>
> Errors in test **stream-error.c**:
>
>   error: 'warning' diagnostics expected but not seen: 
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 97: might be 'indeterminate'
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 100: Stream might be already closed
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 113: Read function called when stream is in EOF state
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 114: Read function called when stream is in EOF state
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 123: FALSE
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 124: FALSE
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 128: FALSE
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 129: TRUE
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 133: TRUE
> File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c 
> Line 134: FALSE
>
> Probably the `StdLibraryFunctionsChecker` runs before the `StreamChecker`, 
> and runs always after it if no dependency is there? But this does not explain 
> all test errors and should cause no errors at all. Adding the dependency 
> itself is not enough, `ModelPOSIX` option should be true too. If the option 
> is set to true in the test file even more test errors appear, and it does not 
> work even when the (StdLibraryFunctionsChecker) checker is added (to the RUN 
> command). Without the dependency the tests do not fail if the order of 
> checkers in the enabled checkers list is changed.

Okay, this is bad. We must understand the reasons behind these failures. It is 
very strange that the `ModelPOSIX` option triggers these failures. I think we 
have to hunt down and examine each failing check, could you please continue the 
debugging of these?




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1060-1064
+} else if (NewState == State) {
+  if (const auto *D = dyn_cast_or_null(Call.getDecl()))
+if (const NoteTag *NT =
+Case.getErrnoConstraint().describe(C, D->getNameAsString()))
+  C.addTransition(NewState, NT);

balazske wrote:
> martong wrote:
> > Why do we need this change?
> It is possible that only the errno related state is changed, no new 
> constraints are added (if the constraint is already here from `evalCall` but 
> the errno was not set there, for example at `fclose` or other stream 
> functions maybe no new state is created here). In such case the note tag is 
> still needed.
Okay, please add that as a comment to this new hunk.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:604-613
+  // Return 0 on success, EOF on failure.
+  SValBuilder &SVB = C.getSValBuilder();
+  ProgramStateRef StateSuccess = State->BindExpr(
+  CE, C.getLocationContext(), SVB.makeIntVal(0, C.getASTContext().IntTy));
+  ProgramStateRef StateFailure =
+  State->BindExpr(CE, C.getLocationContext(),
+  SVB.makeIntVal(*EofVal, C.getASTContext().IntTy));

balazske wrote:
> martong wrote:
> > This is redundant with the summary in the `StdLibraryFunctionsChecker`. Why 
> > do we need this as well?
> It is probably needed to have a (any) bound value to the `fclose` function 
> call, otherwise setting constraints in the other checker do not work. It may 
> work to bind only a conjured value but it looks better if the correct return 
> value is used. This makes less inter-dependence between the two checkers 
> (`StdLibraryFunctionChecker` sets only the errno state as far as possible).
Ok, makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135247

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

[PATCH] D136078: [RFC] Use-after-return binary metadata.

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov added a comment.

Marco, does this look reasonable? Any high-level comments?

There are lots of plumbing to add a new flag and integrate a new pass, so you 
can look only at:
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
llvm/lib/CodeGen/SanitizerMetadata.cpp

As far as I understand the stack layout is only known in machine passes via 
MachineFrameInfo,
so I had to add a machine pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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

[PATCH] D136078: [RFC] Use-after-return binary metadata.

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov added a comment.

Unrelated, but looking at the metadata format more closely, I think this can 
benefit from LEB128-like varlen encoding.
Function size is small.
Features are very small.
Stack args size is small.
Function start (if we encode it from the end of the previous one) is very small 
as well.
So potentially it can reduce the entry size from 16 bytes to just 4.
I am just thinking if we use 10x more metadata in future, the size can grow 
from percents of binary to tens of percents.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

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

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

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

HazardyKnusperkeks added a comment.

In D129443#3867400 , @owenpan wrote:

> In D129443#3867364 , @rymiel wrote:
>
>> How should I proceed with a stale rejecting review?
>
> You can commit as https://reviews.llvm.org/D129443#3641571 has been addressed.

And given that he wasn't active in the last days (weeks?), he will probably not 
respond anytime soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129443

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

[PATCH] D136080: [flang] Add -ffp-contract option processing

[Tom Eccles via Phabricator via cfe-commits]

tblah marked 8 inline comments as done.
tblah added a comment.

>> The omission of the fast-honor-pragmas argument from the compiler driver is 
>> deliberate.
>
> Where is it omitted?

This argument is only supported in the frontend driver, not the compiler driver:

  flang-new -ffp-contract=fast-honor-pragmas test.f90 
  flang-new: error: unsupported argument 'fast-honor-pragmas' to option 
'-ffp-contract='
  
  flang-new -fc1 -ffp-contract=fast-honor-pragmas test.f90
  warning: ffp-contract= is not currently implemented




Comment at: flang/include/flang/Frontend/LangOptions.h:29
+
+// Enable the floating point pragma
+FPM_On,

awarzynski wrote:
> What are these pragmas? Perhaps you can add a test that would include them?
I copied this comment from clang. I believe the pragma is 
```
#pragma clang fp contract(fast)
```

See 
https://clang.llvm.org/docs/LanguageExtensions.html#extensions-to-specify-floating-point-flags

This patch only adds support for argument processing, so I can't test for the 
pragmas.



Comment at: flang/test/Driver/flang_fp_opts.f90:4
+
+! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s
+! CHECK: ffp-contract= is not currently implemented

awarzynski wrote:
> Can you test with `flang` as well?
We already test that these flags are passed to the frontend driver from the 
compiler driver in `flang/test/Driver/frontend-forwarding.f90`


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

https://reviews.llvm.org/D136080

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:54-62
+  explicit operator int8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int64_t() const { return toAPSInt().getExtValue(); }

tbaeder wrote:
> sepavloff wrote:
> > Conversions to integers are bitcasts+truncation but the conversion to bool 
> > is different. What semantics have these operations?
> They are basically used for casting. I didn't mean to make the bool 
> implementation different, just seemed to make sense to me to do it this way.
If they are used for casting, they need to preserve value, that is conversion 
float->int32 should transform 1.0->0x0001. The method `toAPSInt` uses 
bitcast, so it converts 1.0->0x3f80. `APFloat::convertToInteger` should be 
used to make value-preverving conversion but it requires knowledge of rounding 
mode.


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

https://reviews.llvm.org/D134859

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton added a comment.

Adding two inline comments.




Comment at: clang/unittests/Format/FormatTest.cpp:6599-6601
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;

HazardyKnusperkeks wrote:
> Do you need this for the observed effect? From the description I don't see 
> that.
I think it's necessary for the test to work, otherwise the value would fall 
back to 4, I think? Would you prefer the test to be re-written with the default 
value, instead? 



Comment at: clang/unittests/Format/FormatTest.cpp:6603
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  EXPECT_EQ(
+  "struct Derived {\n"

HazardyKnusperkeks wrote:
> Is the behavior different when the code is already formatted like you want 
> it? (It shouldn't.) Then you should use `verifyFormat()`.
Not sure I understand. Would you like the test to be re-written so that the 
code is already formatted like the result of the formatting? 


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

https://reviews.llvm.org/D136154

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

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D134337#3866267 , @mgorny wrote:

> In D134337#3865905 , @tstellar 
> wrote:
>
>> In D134337#3865753 , @mgorny wrote:
>>
>>> In D134337#3865541 , @tstellar 
>>> wrote:
>>>
 I know I'm a little late here, but having a default config file that's 
 always loaded makes triaging issues much harder, because now every time 
 someone files a bug, we need to ask for the contents of their config 
 directory.
>>>
>>> I don't see that as much worse than asking them for the values of 
>>> `*DEFAULT*` CMake options or whether they have patched their clang because 
>>> the driver code makes so many specific assumptions that it didn't work for 
>>> them. At least `clang -v` tells us whether they are actually using any 
>>> configs.
>>
>> At least as a distro maintainer, I know the CMake options used for the 
>> packages I'm trying to support.
>
> As a distro maintainer, you can decide to build clang without system/user 
> config directories set (which is the default), in which case only explicit 
> `--config` will work.

Strictly speaking in this case the default config file may be loaded from the 
binary directory but the latter is controlled by distro.

Users also may split configuration file into pieces and include them using 
`@file` construct, in this case single config file is not enough. I think clang 
could print driver options in output of `-v`. Another way is to implement a 
special command line option, say `-print-args`, which would print all driver 
options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134337

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

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

HazardyKnusperkeks added a comment.

In D136154#3867461 , @hel-ableton 
wrote:

>> must have been introduced somewhere between 7.1.0 and 10.0.0
>
> In fact, someone in our team thought that this must have been introduced 
> exactly with this commit:
>
> https://github.com/llvm/llvm-project/commit/4636debc271f09f730697ab6873137a657c828f9

You need to check that the operator is not an assignment, maybe add more test 
cases with the 3 options to show how it should behave.


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

https://reviews.llvm.org/D136154

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

[PATCH] D136080: [flang] Add -ffp-contract option processing

[Tom Eccles via Phabricator via cfe-commits]

tblah updated this revision to Diff 468840.

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

https://reviews.llvm.org/D136080

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/LangOptions.def
  flang/include/flang/Frontend/LangOptions.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/LangOptions.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/flang_fp_opts.f90
  flang/test/Driver/frontend-forwarding.f90

Index: flang/test/Driver/frontend-forwarding.f90
===
--- flang/test/Driver/frontend-forwarding.f90
+++ flang/test/Driver/frontend-forwarding.f90
@@ -8,6 +8,7 @@
 ! RUN: -fdefault-real-8 \
 ! RUN: -flarge-sizes \
 ! RUN: -fconvert=little-endian \
+! RUN: -ffp-contract=fast \
 ! RUN: -mllvm -print-before-all\
 ! RUN: -P \
 ! RUN:   | FileCheck %s
@@ -18,5 +19,6 @@
 ! CHECK: "-fdefault-integer-8"
 ! CHECK: "-fdefault-real-8"
 ! CHECK: "-flarge-sizes"
+! CHECK: "-ffp-contract=fast"
 ! CHECK: "-fconvert=little-endian"
 ! CHECK:  "-mllvm" "-print-before-all"
Index: flang/test/Driver/flang_fp_opts.f90
===
--- /dev/null
+++ flang/test/Driver/flang_fp_opts.f90
@@ -0,0 +1,4 @@
+! Test for handling of floating point options within the frontend driver
+
+! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s
+! CHECK: ffp-contract= is not currently implemented
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -31,6 +31,7 @@
 ! HELP-NEXT: -ffixed-form   Process source files in fixed form
 ! HELP-NEXT: -ffixed-line-length=
 ! HELP-NEXT: Use  as character line width in fixed mode
+! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast (fuses across statements disregarding pragmas) | on (only fuses in the same statement unless dictated by pragmas) | off (never fuses) | fast-honor-pragmas (fuses across statements unless diectated by pragmas). Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, and 'on' otherwise.
 ! HELP-NEXT: -ffree-formProcess source files in free form
 ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-NEXT: -finput-charset= Specify the default character set for source files
@@ -105,6 +106,7 @@
 ! HELP-FC1-NEXT: -ffixed-form   Process source files in fixed form
 ! HELP-FC1-NEXT: -ffixed-line-length=
 ! HELP-FC1-NEXT: Use  as character line width in fixed mode
+! HELP-FC1-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast (fuses across statements disregarding pragmas) | on (only fuses in the same statement unless dictated by pragmas) | off (never fuses) | fast-honor-pragmas (fuses across statements unless diectated by pragmas). Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, and 'on' otherwise.
 ! HELP-FC1-NEXT: -ffree-formProcess source files in free form
 ! HELP-FC1-NEXT: -fget-definition   
 ! HELP-FC1-NEXT:Get the symbol definition from   
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -31,6 +31,7 @@
 ! CHECK-NEXT: -ffixed-form   Process source files in fixed form
 ! CHECK-NEXT: -ffixed-line-length=
 ! CHECK-NEXT: Use  as character line width in fixed mode
+! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast (fuses across statements disregarding pragmas) | on (only fuses in the same statement unless dictated by pragmas) | off (never fuses) | fast-honor-pragmas (fuses across statements unless diectated by pragmas). Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, and 'on' otherwise.
 ! CHECK-NEXT: -ffree-formProcess source files in free form
 ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! CHECK-NEXT: -finput-charset= Specify the default character set for source files
Index: flang/lib/Frontend/LangOptions.cpp
===
--- /dev/null
+++ flang/lib/Frontend/LangOptions.cpp
@@ -0,0 +1,24 @@
+//===-- LangOptions.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Coding style: https://mlir.llvm.org/

[PATCH] D136239: [testcase] [OpenMP] Fix the testcase error of check-all when DCLANG_DEFAULT_OPENMP_RUNTIME is not libomp

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu created this revision.
zixuan-wu added reviewers: jdoerfert, MaskRay.
Herald added subscribers: StephenFan, arphaman, guansong, yaxunl.
Herald added a project: All.
zixuan-wu requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

When DCLANG_DEFAULT_OPENMP_RUNTIME is set to libgomp, there is some check-all 
error. The expected CHECK result only displays when fopenmp=libomp is specified 
explicitly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136239

Files:
  clang/test/Driver/fopenmp.c
  clang/test/Index/openmp-tile.c


Index: clang/test/Index/openmp-tile.c
===
--- clang/test/Index/openmp-tile.c
+++ clang/test/Index/openmp-tile.c
@@ -1,4 +1,4 @@
-// RUN: c-index-test -test-load-source local %s -fopenmp -fopenmp-version=51 | 
FileCheck %s
+// RUN: c-index-test -test-load-source local %s -fopenmp=libomp 
-fopenmp-version=51 | FileCheck %s
 
 void test() {
 #pragma omp tile sizes(5)
Index: clang/test/Driver/fopenmp.c
===
--- clang/test/Driver/fopenmp.c
+++ clang/test/Driver/fopenmp.c
@@ -140,7 +140,7 @@
 // CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC: "-{{B?}}static" {{.*}} "-liomp5"
 // CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC-NOT: "-Bdynamic"
 //
-// RUN: %clang -target x86_64-linux-gnu -fopenmp -fopenmp-enable-irbuilder -c 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CC1-OPENMPIRBUILDER
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libomp 
-fopenmp-enable-irbuilder -c %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-CC1-OPENMPIRBUILDER
 //
 // CHECK-CC1-OPENMPIRBUILDER: "-cc1"
 // CHECK-CC1-OPENMPIRBUILDER-SAME: "-fopenmp"


Index: clang/test/Index/openmp-tile.c
===
--- clang/test/Index/openmp-tile.c
+++ clang/test/Index/openmp-tile.c
@@ -1,4 +1,4 @@
-// RUN: c-index-test -test-load-source local %s -fopenmp -fopenmp-version=51 | FileCheck %s
+// RUN: c-index-test -test-load-source local %s -fopenmp=libomp -fopenmp-version=51 | FileCheck %s
 
 void test() {
 #pragma omp tile sizes(5)
Index: clang/test/Driver/fopenmp.c
===
--- clang/test/Driver/fopenmp.c
+++ clang/test/Driver/fopenmp.c
@@ -140,7 +140,7 @@
 // CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC: "-{{B?}}static" {{.*}} "-liomp5"
 // CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC-NOT: "-Bdynamic"
 //
-// RUN: %clang -target x86_64-linux-gnu -fopenmp -fopenmp-enable-irbuilder -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CC1-OPENMPIRBUILDER
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-enable-irbuilder -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CC1-OPENMPIRBUILDER
 //
 // CHECK-CC1-OPENMPIRBUILDER: "-cc1"
 // CHECK-CC1-OPENMPIRBUILDER-SAME: "-fopenmp"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0dec5e1 - Keep configuration file search directories in ExpansionContext. NFC

[Serge Pavlov via cfe-commits]

Author: Serge Pavlov
Date: 2022-10-19T17:20:14+07:00
New Revision: 0dec5e164f9d289b6e576655c7cf21a3dd0389f8

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

LOG: Keep configuration file search directories in ExpansionContext. NFC

Class ExpansionContext encapsulates options for search and expansion of
response files, including configuration files. With this change the
directories which are searched for configuration files are also stored
in ExpansionContext.

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

Added: 


Modified: 
clang/include/clang/Driver/Driver.h
clang/lib/Driver/Driver.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index fe2b4bd629fae..6eb168e56510d 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -36,6 +36,9 @@ class Triple;
 namespace vfs {
 class FileSystem;
 }
+namespace cl {
+class ExpansionContext;
+}
 } // namespace llvm
 
 namespace clang {
@@ -677,13 +680,14 @@ class Driver {
   /// executable filename).
   ///
   /// \returns true if error occurred.
-  bool loadDefaultConfigFiles(ArrayRef CfgFileSearchDirs);
+  bool loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx);
 
   /// Read options from the specified file.
   ///
   /// \param [in] FileName File to read.
+  /// \param [in] Search and expansion options.
   /// \returns true, if error occurred while reading.
-  bool readConfigFile(StringRef FileName);
+  bool readConfigFile(StringRef FileName, llvm::cl::ExpansionContext &ExpCtx);
 
   /// Set the driver mode (cl, gcc, etc) from the value of the `--driver-mode`
   /// option.

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 45530f5d68e6c..26729f1d03ea8 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -922,35 +922,6 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation 
&C,
   //
 }
 
-/// Looks the given directories for the specified file.
-///
-/// \param[out] FilePath File path, if the file was found.
-/// \param[in]  Dirs Directories used for the search.
-/// \param[in]  FileName Name of the file to search for.
-/// \return True if file was found.
-///
-/// Looks for file specified by FileName sequentially in directories specified
-/// by Dirs.
-///
-static bool searchForFile(SmallVectorImpl &FilePath,
-  ArrayRef Dirs, StringRef FileName,
-  llvm::vfs::FileSystem &FS) {
-  SmallString<128> WPath;
-  for (const StringRef &Dir : Dirs) {
-if (Dir.empty())
-  continue;
-WPath.clear();
-llvm::sys::path::append(WPath, Dir, FileName);
-llvm::sys::path::native(WPath);
-auto Status = FS.status(WPath);
-if (Status && Status->getType() == llvm::sys::fs::file_type::regular_file) 
{
-  FilePath = std::move(WPath);
-  return true;
-}
-  }
-  return false;
-}
-
 static void appendOneArg(InputArgList &Args, const Arg *Opt,
  const Arg *BaseArg) {
   // The args for config files or /clang: flags belong to 
diff erent InputArgList
@@ -967,11 +938,10 @@ static void appendOneArg(InputArgList &Args, const Arg 
*Opt,
   Args.append(Copy);
 }
 
-bool Driver::readConfigFile(StringRef FileName) {
+bool Driver::readConfigFile(StringRef FileName,
+llvm::cl::ExpansionContext &ExpCtx) {
   // Try reading the given file.
   SmallVector NewCfgArgs;
-  llvm::cl::ExpansionContext ExpCtx(Alloc, llvm::cl::tokenizeConfigFile);
-  ExpCtx.setVFS(&getVFS());
   if (!ExpCtx.readConfigFile(FileName, NewCfgArgs)) {
 Diag(diag::err_drv_cannot_read_config_file) << FileName;
 return true;
@@ -1012,6 +982,10 @@ bool Driver::readConfigFile(StringRef FileName) {
 }
 
 bool Driver::loadConfigFiles() {
+  llvm::cl::ExpansionContext ExpCtx(Saver.getAllocator(),
+llvm::cl::tokenizeConfigFile);
+  ExpCtx.setVFS(&getVFS());
+
   // Process options that change search path for config files.
   if (CLOptions) {
 if (CLOptions->hasArg(options::OPT_config_system_dir_EQ)) {
@@ -1036,19 +1010,20 @@ bool Driver::loadConfigFiles() {
 
   // Prepare list of directories where config file is searched for.
   StringRef CfgFileSearchDirs[] = {UserConfigDir, SystemConfigDir, Dir};
+  ExpCtx.setSearchDirs(CfgFileSearchDirs);
 
   // First try to load configuration from the default files, return on error.
-  if (loadDefaultConfigFiles(CfgFileSearchDirs))
+  if (loadDefaultConfigFiles(ExpCtx))
 return true;
 
   // Then load configuration files specified explicitly.
-  llvm::SmallString<128> CfgFilePath;
+  SmallString<128> CfgFilePath;
   if (CLOptions) {

[PATCH] D135439: Keep configuration file search directories in ExpansionContext. NFC

[Serge Pavlov 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 rG0dec5e164f9d: Keep configuration file search directories in 
ExpansionContext. NFC (authored by sepavloff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135439

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1350,6 +1350,43 @@
 ExpansionContext::ExpansionContext(BumpPtrAllocator &A, TokenizerCallback T)
 : Saver(A), Tokenizer(T), FS(vfs::getRealFileSystem().get()) {}
 
+bool ExpansionContext::findConfigFile(StringRef FileName,
+  SmallVectorImpl &FilePath) {
+  SmallString<128> CfgFilePath;
+  const auto FileExists = [this](SmallString<128> Path) -> bool {
+auto Status = FS->status(Path);
+return Status &&
+   Status->getType() == llvm::sys::fs::file_type::regular_file;
+  };
+
+  // If file name contains directory separator, treat it as a path to
+  // configuration file.
+  if (llvm::sys::path::has_parent_path(FileName)) {
+CfgFilePath = FileName;
+if (llvm::sys::path::is_relative(FileName) && FS->makeAbsolute(CfgFilePath))
+  return false;
+if (!FileExists(CfgFilePath))
+  return false;
+FilePath.assign(CfgFilePath.begin(), CfgFilePath.end());
+return true;
+  }
+
+  // Look for the file in search directories.
+  for (const StringRef &Dir : SearchDirs) {
+if (Dir.empty())
+  continue;
+CfgFilePath.assign(Dir);
+llvm::sys::path::append(CfgFilePath, FileName);
+llvm::sys::path::native(CfgFilePath);
+if (FileExists(CfgFilePath)) {
+  FilePath.assign(CfgFilePath.begin(), CfgFilePath.end());
+  return true;
+}
+  }
+
+  return false;
+}
+
 bool ExpansionContext::readConfigFile(StringRef CfgFile,
   SmallVectorImpl &Argv) {
   SmallString<128> AbsPath;
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -2140,6 +2140,9 @@
   /// current directory is used instead.
   StringRef CurrentDir;
 
+  /// Directories used for search of config files.
+  ArrayRef SearchDirs;
+
   /// True if names of nested response files must be resolved relative to
   /// including file.
   bool RelativeNames = false;
@@ -2172,11 +2175,27 @@
 return *this;
   }
 
+  ExpansionContext &setSearchDirs(ArrayRef X) {
+SearchDirs = X;
+return *this;
+  }
+
   ExpansionContext &setVFS(vfs::FileSystem *X) {
 FS = X;
 return *this;
   }
 
+  /// Looks for the specified configuration file.
+  ///
+  /// \param[in]  FileName Name of the file to search for.
+  /// \param[out] FilePath File absolute path, if it was found.
+  /// \return True if file was found.
+  ///
+  /// If the specified file name contains a directory separator, it is searched
+  /// for by its absolute path. Otherwise looks for file sequentially in
+  /// directories specified by SearchDirs field.
+  bool findConfigFile(StringRef FileName, SmallVectorImpl &FilePath);
+
   /// Reads command line options from the given configuration file.
   ///
   /// \param [in] CfgFile Path to configuration file.
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -922,35 +922,6 @@
   //
 }
 
-/// Looks the given directories for the specified file.
-///
-/// \param[out] FilePath File path, if the file was found.
-/// \param[in]  Dirs Directories used for the search.
-/// \param[in]  FileName Name of the file to search for.
-/// \return True if file was found.
-///
-/// Looks for file specified by FileName sequentially in directories specified
-/// by Dirs.
-///
-static bool searchForFile(SmallVectorImpl &FilePath,
-  ArrayRef Dirs, StringRef FileName,
-  llvm::vfs::FileSystem &FS) {
-  SmallString<128> WPath;
-  for (const StringRef &Dir : Dirs) {
-if (Dir.empty())
-  continue;
-WPath.clear();
-llvm::sys::path::append(WPath, Dir, FileName);
-llvm::sys::path::native(WPath);
-auto Status = FS.status(WPath);
-if (Status && Status->getType() == llvm::sys::fs::file_type::regular_file) {
-  FilePath = std::move(WPath);
-  return true;
-}
-  }
-  return false;
-}
-
 static void appendOneArg(InputArgList &Args, const Arg *Opt,
  const Arg *BaseArg) {
   // The args for config files or /clang: flags belong to different InputArgList
@@

[clang] 66bd607 - [Attr][Doc] Fix pragma unroll documentation.

[Alexey Bader via cfe-commits]

Author: Alexey Bader
Date: 2022-10-19T03:50:47-07:00
New Revision: 66bd6074c133402e45075b591c062c22f308ef26

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

LOG: [Attr][Doc] Fix pragma unroll documentation.

There is a contradiction in the #pragma unroll behavior documentation.
It says that specifying `#pragma unroll` without a parameter directs the
loop unroller to attempt to partially unroll the loop if the trip count
is not known at compile time. At the same time later it states that
`#pragma unroll` has identical semantics to `#pragma clang loop unroll(full)`,
which doesn't attempt to unroll partially if the trip count is not known
at compile time.

pragma clang loop unroll(enable):
If unroll(enable) is specified the unroller will attempt to fully unroll
the loop if the trip count is known at compile time. If the fully
unrolled code size is greater than an internal limit the loop will be
partially unrolled up to this limit. If the trip count is not known at
compile time the loop will be partially unrolled with a heuristically
chosen unroll factor.

pragma clang loop unroll(full):
If unroll(full) is specified the unroller will attempt to fully unroll
the loop if the trip count is known at compile time identically to
unroll(enable). However, with unroll(full) the loop will not be unrolled
if the loop count is not known at compile time.

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

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 33a18ac033252..484052f4db8ad 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3569,7 +3569,7 @@ Specifying ``#pragma nounroll`` indicates that the loop 
should not be unrolled:
   }
 
 ``#pragma unroll`` and ``#pragma unroll _value_`` have identical semantics to
-``#pragma clang loop unroll(full)`` and
+``#pragma clang loop unroll(enable)`` and
 ``#pragma clang loop unroll_count(_value_)`` respectively. ``#pragma nounroll``
 is equivalent to ``#pragma clang loop unroll(disable)``. See
 `language extensions



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

[PATCH] D136160: [Attr][Doc] Fix pragma unroll documentation.

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG66bd6074c133: [Attr][Doc] Fix pragma unroll documentation. 
(authored by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136160

Files:
  clang/include/clang/Basic/AttrDocs.td


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3569,7 +3569,7 @@
   }
 
 ``#pragma unroll`` and ``#pragma unroll _value_`` have identical semantics to
-``#pragma clang loop unroll(full)`` and
+``#pragma clang loop unroll(enable)`` and
 ``#pragma clang loop unroll_count(_value_)`` respectively. ``#pragma nounroll``
 is equivalent to ``#pragma clang loop unroll(disable)``. See
 `language extensions


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3569,7 +3569,7 @@
   }
 
 ``#pragma unroll`` and ``#pragma unroll _value_`` have identical semantics to
-``#pragma clang loop unroll(full)`` and
+``#pragma clang loop unroll(enable)`` and
 ``#pragma clang loop unroll_count(_value_)`` respectively. ``#pragma nounroll``
 is equivalent to ``#pragma clang loop unroll(disable)``. See
 `language extensions
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

You've dropped the test on your most recent updated




Comment at: clang/unittests/Format/FormatTest.cpp:6603-6622
+  EXPECT_EQ(
+  "struct Derived {\n"
+  "  Derived(\n"
+  "int firstArgWithLongName,\n"
+  "int secondArgWithLongName,\n"
+  "int thirdArgWithLongName,\n"
+  "int fourthArgWithLongName)\n"

hel-ableton wrote:
> HazardyKnusperkeks wrote:
> > Is the behavior different when the code is already formatted like you want 
> > it? (It shouldn't.) Then you should use `verifyFormat()`.
> Not sure I understand. Would you like the test to be re-written so that the 
> code is already formatted like the result of the formatting? 
The starting state of the code should not be important, verifyFormat will 
"messUp" the code spacing to test this out, your are verifiying that it 
maintains what you want.


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I get why this fix works for you, but I don't agree that the fix is the correct 
one..

for example you don't have any BinaryOperators in that code, lets just say in 
my style I had

  BreakBeforeBinaryOperators: All

Then your fix wouldn't cover me and I'd get exactly the same issue, (but why 
should i)

  struct Foo {
Foo(
  int firstArgWithLongName,
  int secondArgWithLongName,
  int thirdArgWithLongName,
  int fourthArgWithLongName)
: Base(
  firstArgWithLongName,
  secondArgWithLongName,
  thirdArgWithLongName,
  fourthArgWithLongName) {}

The problem as I see it was that the original bug, highly constrained the cases 
where "CurrentState.LastSpace = State.Column;" to one particular style (which 
if it happens to be your style great but not if its not.


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Its probably worth looking at how to fix this more in the context of the 
original bug, Its something I don't like is where we pile in all these 
expressions, without any comments about what cases we are handling here..
I'd rather handle each one at a time.. as I don't see how TT_BinaryOperator and 
TT_CtorInitializerColon are likely to be related.

  else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
 TT_CtorInitializerColon)) &&
   ((Previous.getPrecedence() != prec::Assignment &&
 (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 
||
  Previous.NextOperator)) ||
Current.StartsBinaryExpression)) {
  // Indent relative to the RHS of the expression unless this is a simple
  // assignment without binary expression on the RHS. Also indent relative 
to
  // unary operators and the colons of constructor initializers.
  if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
  +   Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment)
CurrentState.LastSpace = State.Column;


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Doesn't something like this  achieve the same

   CurrentState.Indent = State.Column;
   CurrentState.LastSpace = State.Column;
  -  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
  -   TT_CtorInitializerColon)) &&
  +  } else if (Previous.is(TT_CtorInitializerColon)) {
  +CurrentState.LastSpace = State.Column;
  +  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
((Previous.getPrecedence() != prec::Assignment &&
  (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 
||

Regardless of the choice of BreakBeforeBinaryOperators?

BTW I checked such a change passes all the other unit tests. (Beyonce rule!!)


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I think in hindsight D50699: [clang-format] fix PR38525 - Extraneous 
continuation indent spaces with BreakBeforeBinaryOperators set to All 
 was overly aggressive again the 
`TT_CtorInitializerColon` case


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

https://reviews.llvm.org/D136154

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

[PATCH] D136162: [analyzer] Fix assertion failure in RegionStore within bindArray()

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

steakhal added a comment.

In D136162#3865081 , @martong wrote:

> Hmm, seems like the conflicting prototype (i.e. the obsolescent use of zero 
> parameters) is needed to reproduce the assertion failure. That makes me 
> wonder, how does the redecl chain of `b` looks like? Is `void b()` chained 
> with `void b(int*)`, or are they represented independently from each other? I 
> guess they form the same redecl chain. Which drives us to the next questions.
> When the analyzer reaches the CallExpr `b(&buffer)` which FunctionDecl does 
> it see? Is it `b()` or `b(int*)`? My bet, it sees and works with `b()`.

Yes, they form the same redecl chain, indeed.
The call `b(&buffer)` refers to the `void ()` decl - with no parameters.
The range of `->redecls()` of that decl has two items:
The decl of `void ()`, and the decl of `void (int *)`.

> Could we detect if the arguments of the CallExpr does not match the 
> parameters of the FunctionDecl? And if that is the case, could we iterate 
> through the redecl chain to find an appropriate matching FunctionDecl? That 
> would be `b(int*)` in this case ... and the original `bindArray()` should 
> work then.

Good idea. Actually, the CFG already refers to the `void ()` delc, so we should 
probably change it there instead of doing the same in the CSA.
Let me investigate this route.

In D136162#3865094 , @martong wrote:

>> A similar situation could happen if we reinterpret cast pointers, etc. so 
>> the situation is not limited to conflicting function prototypes.
>
> Please provide tests for those cases.

It was a harsh guess on my part, simply by looking at the missing 
`ElementRegion`, I thought that I could reconstruct the example by some 
reinterpret-cast tricks - but I could not pull it off.
Disregard that part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136162

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

[PATCH] D136248: [Index] consider `delete X` a reference to ~X() if it can be resolved

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

Fixes https://github.com/clangd/clangd/issues/1321


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136248

Files:
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/Index/IndexBody.cpp
  clang/test/Index/index-refs.cpp

Index: clang/test/Index/index-refs.cpp
===
--- clang/test/Index/index-refs.cpp
+++ clang/test/Index/index-refs.cpp
@@ -24,6 +24,7 @@
   S& operator=(int x);
   S& operator!=(int x);
   S& operator()(int x);
+  ~S();
 };
 
 void foo2(S &s) {
@@ -32,6 +33,7 @@
   s = 3;
   (void)(s != 3);
   s(3);
+  delete &s;
 }
 
 namespace NS {
@@ -98,41 +100,42 @@
 // CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: operator=
 // CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: operator!=
 // CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: operator()
-
-// CHECK:  [indexEntityReference]: kind: namespace | name: NS | {{.*}} | loc: 42:17
-// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: NS | {{.*}} | loc: 43:17
-// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: Inn | {{.*}} | loc: 43:21
-// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: NS | {{.*}} | loc: 44:7
-// CHECK-NEXT: [indexEntityReference]: kind: typedef | name: Foo | {{.*}} | loc: 44:11
-
-// CHECK:  [indexDeclaration]: kind: c++-class-template | name: TS | {{.*}} | loc: 47:8
-// CHECK-NEXT: [indexDeclaration]: kind: struct-template-partial-spec | name: TS | USR: c:@SP>1#T@TS>#t0.0#I | {{.*}} | loc: 50:8
-// CHECK-NEXT: [indexDeclaration]: kind: typedef | name: MyInt | USR: c:index-refs.cpp@SP>1#T@TS>#t0.0#I@T@MyInt | {{.*}} | loc: 51:15 | semantic-container: [TS:50:8] | lexical-container: [TS:50:8]
-// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: TS | USR: c:@ST>2#T#T@TS | lang: C++ | cursor: TemplateRef=TS:47:8 | loc: 50:8 | :: <> | container: [TU] | refkind: direct | role: ref
+// CHECK-NEXT: [indexEntityReference]: kind: destructor | name: ~S
+
+// CHECK:  [indexEntityReference]: kind: namespace | name: NS | {{.*}} | loc: 44:17
+// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: NS | {{.*}} | loc: 45:17
+// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: Inn | {{.*}} | loc: 45:21
+// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: NS | {{.*}} | loc: 46:7
+// CHECK-NEXT: [indexEntityReference]: kind: typedef | name: Foo | {{.*}} | loc: 46:11
+
+// CHECK:  [indexDeclaration]: kind: c++-class-template | name: TS | {{.*}} | loc: 49:8
+// CHECK-NEXT: [indexDeclaration]: kind: struct-template-partial-spec | name: TS | USR: c:@SP>1#T@TS>#t0.0#I | {{.*}} | loc: 52:8
+// CHECK-NEXT: [indexDeclaration]: kind: typedef | name: MyInt | USR: c:index-refs.cpp@SP>1#T@TS>#t0.0#I@T@MyInt | {{.*}} | loc: 53:15 | semantic-container: [TS:52:8] | lexical-container: [TS:52:8]
+// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: TS | USR: c:@ST>2#T#T@TS | lang: C++ | cursor: TemplateRef=TS:49:8 | loc: 52:8 | :: <> | container: [TU] | refkind: direct | role: ref
 /* when indexing implicit instantiations
-  [indexDeclaration]: kind: struct-template-spec | name: TS | USR: c:@S@TS>#I | {{.*}} | loc: 50:8
-  [indexDeclaration]: kind: typedef | name: MyInt | USR: c:index-refs.cpp@593@S@TS>#I@T@MyInt | {{.*}} | loc: 51:15 | semantic-container: [TS:50:8] | lexical-container: [TS:50:8]
+  [indexDeclaration]: kind: struct-template-spec | name: TS | USR: c:@S@TS>#I | {{.*}} | loc: 52:8
+  [indexDeclaration]: kind: typedef | name: MyInt | USR: c:index-refs.cpp@593@S@TS>#I@T@MyInt | {{.*}} | loc: 53:15 | semantic-container: [TS:52:8] | lexical-container: [TS:52:8]
  */
 // CHECK-NEXT: [indexDeclaration]: kind: function | name: foo3
 /* when indexing implicit instantiations
-  [indexEntityReference]: kind: struct-template-spec | name: TS | USR: c:@S@TS>#I | {{.*}} | loc: 55:3
+  [indexEntityReference]: kind: struct-template-spec | name: TS | USR: c:@S@TS>#I | {{.*}} | loc: 57:3
 */
-// CHECK-NEXT: [indexEntityReference]: kind: struct-template-partial-spec | name: TS | USR: c:@SP>1#T@TS>#t0.0#I | {{.*}} | loc: 55:3
+// CHECK-NEXT: [indexEntityReference]: kind: struct-template-partial-spec | name: TS | USR: c:@SP>1#T@TS>#t0.0#I | {{.*}} | loc: 57:3
 
-// CHECK:  [indexEntityReference]: kind: variable | name: array_size | {{.*}} | loc: 59:22
-// CHECK:  [indexEntityReference]: kind: variable | name: default_param | {{.*}} | loc: 62:19 | {{.*}} | role: ref read
-// CHECK-NOT:  [indexEntityReference]: kind: variable | name: default_param | {{.*}} | loc: 62:19
+// CHECK:  [indexEntityReference]: 

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton added a comment.

In D136154#3867935 , @MyDeveloperDay 
wrote:

> You've dropped the test on your most recent updated

Oh, that's why it appeared from the diff. Apologies again, I'm really 
unfamiliar with your process. I guess it puzzles me why it's described in 
https://llvm.org/docs/Phabricator.html#phabricator-request-review-web that I 
should make any commits at all, when it's just diffs that I'm supposed to 
submit. Anyway, I'll try to bring it back.


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

https://reviews.llvm.org/D136154

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

[PATCH] D136224: [clang-tidy] Skip private default ctors in modernize-use-equals-default

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:233
+  auto IsPublicOrOutOfLineUntilCPP20 =
+  LangOpts.CPlusPlus20 || LangOpts.CPlusPlus2b
+  ? cxxConstructorDecl()

You don't need to check for CPlusPlus2b. When CPlusPlus2b is set, CPlusPlus20 
will be set as well.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx20.cpp:1
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-equals-default %t -- -- 
-fno-delayed-template-parsing -fexceptions
+

If you don't mind, it would be also helpful to add a test in c++17 mode that 
shows that the check does not suggest the fix then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136224

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton updated this revision to Diff 468874.
hel-ableton added a comment.

This should bring back the formerly introduced unit test.


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

https://reviews.llvm.org/D136154

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6594,6 +6594,32 @@
"(someOtherLongishConditionPart1 || "
"someOtherEvenLongerNestedConditionPart2);",
Style));
+
+  Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  EXPECT_EQ(
+  "struct Derived {\n"
+  "  Derived(\n"
+  "int firstArgWithLongName,\n"
+  "int secondArgWithLongName,\n"
+  "int thirdArgWithLongName,\n"
+  "int fourthArgWithLongName)\n"
+  "  : Base(\n"
+  "  firstArgWithLongName,\n"
+  "  secondArgWithLongName,\n"
+  "  thirdArgWithLongName,\n"
+  "  fourthArgWithLongName) {}\n"
+  "};",
+  format("struct Derived {"
+ "  Derived(int firstArgWithLongName, int secondArgWithLongName, "
+ "int thirdArgWithLongName, int fourthArgWithLongName)"
+ ": Base(firstArgWithLongName, secondArgWithLongName, "
+ "thirdArgWithLongName, fourthArgWithLongName) {}"
+ "};",
+ Style));
 }
 
 TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -809,8 +809,10 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;
+}
   } else if (Previous.is(TT_InheritanceColon)) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6594,6 +6594,32 @@
"(someOtherLongishConditionPart1 || "
"someOtherEvenLongerNestedConditionPart2);",
Style));
+
+  Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  EXPECT_EQ(
+  "struct Derived {\n"
+  "  Derived(\n"
+  "int firstArgWithLongName,\n"
+  "int secondArgWithLongName,\n"
+  "int thirdArgWithLongName,\n"
+  "int fourthArgWithLongName)\n"
+  "  : Base(\n"
+  "  firstArgWithLongName,\n"
+  "  secondArgWithLongName,\n"
+  "  thirdArgWithLongName,\n"
+  "  fourthArgWithLongName) {}\n"
+  "};",
+  format("struct Derived {"
+ "  Derived(int firstArgWithLongName, int secondArgWithLongName, "
+ "int thirdArgWithLongName, int fourthArgWithLongName)"
+ ": Base(firstArgWithLongName, secondArgWithLongName, "
+ "thirdArgWithLongName, fourthArgWithLongName) {}"
+ "};",
+ Style));
 }
 
 TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -809,8 +809,10 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;
+}
   } else if (Previous.is(TT_InheritanceColon)) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = Sta

[PATCH] D133668: [HLSL] Disable integer promotion to avoid int16_t being promoted to int for HLSL.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:841
+if (Ty->isPromotableIntegerType() &&
+// Avoid promote integer type to int.
+!getLangOpts().HLSL) {

rjmccall wrote:
> 
Hmm, the downside to the change being applied here is that 
`isPromotableIntegerType()` is a lie for HLSL and other callers are going to be 
surprised. I think a better way to achieve this same goal is to move 
`isPromotableIntegerType()` from `Type` to `ASTContext` so that it has access 
to the language options, and then hoist the language mode check so that 
`isPromotableIntegerType()` returns `false` for integer types in HLSL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133668

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

[PATCH] D136212: [clangd] consider ~^foo() to target the destructor, not the type

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added a comment.

Ugh, sorry for the sloppiness, think I was tired.




Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:469
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
-  // FIXME: The following to should be class itself instead.
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},

nridge wrote:
> The FIXME looks like it pertains to the next line where the behaviour is 
> unchanged -- should it be preserved?
I'm pretty sure the reason for the FIXME was to consistently handle type names 
within constructors/destructors as referencing the type - with this patch we're 
(hopefully) consistent in the other direction instead.

(I think I probably wrote this comment, I wish I'd included more context)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136212

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

[clang-tools-extra] 62116c8 - [clangd] consider ~^foo() to target the destructor, not the type

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-10-19T14:12:31+02:00
New Revision: 62116c8f0b5b7aac3d53f1bd2445356ae6866048

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

LOG: [clangd] consider ~^foo() to target the destructor, not the type

This behavior was once deliberate, but i've yet to find someone who likes it.
The reference behavior is unchanged: the `foo` within ~foo is still considered
a reference to the type. This means rename etc still works.

fixes https://github.com/clangd/clangd/issues/179

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 80763701d1673..32b23fa4d3945 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -860,7 +860,7 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
   // is not available to the node's children.
   // Usually empty, but sometimes children cover tokens but shouldn't own them.
   SourceRange earlySourceRange(const DynTypedNode &N) {
-if (const Decl *D = N.get()) {
+if (const Decl *VD = N.get()) {
   // We want the name in the var-decl to be claimed by the decl itself and
   // not by any children. Ususally, we don't need this, because source
   // ranges of children are not overlapped with their parent's.
@@ -869,8 +869,20 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
   //auto fun = [bar = foo]() { ... }
   //~   VarDecl
   //~~~ |- AutoTypeLoc
-  if (const auto *DD = llvm::dyn_cast(D))
-return DD->getLocation();
+  return VD->getLocation();
+}
+
+// When referring to a destructor ~Foo(), attribute Foo to the destructor
+// rather than the TypeLoc nested inside it.
+// We still traverse the TypeLoc, because it may contain other targeted
+// things like the T in ~Foo().
+if (const auto *CDD = N.get())
+  return CDD->getNameInfo().getNamedTypeInfo()->getTypeLoc().getBeginLoc();
+if (const auto *ME = N.get()) {
+  auto NameInfo = ME->getMemberNameInfo();
+  if (NameInfo.getName().getNameKind() ==
+  DeclarationName::CXXDestructorName)
+return NameInfo.getNamedTypeInfo()->getTypeLoc().getBeginLoc();
 }
 
 return SourceRange();

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index bd3df2404fcf6..d7ea34c3c7054 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -466,7 +466,10 @@ TEST(SelectionTest, CommonAncestor) {
   {"struct foo { [[int has^h<:32:>]]; };", "FieldDecl"},
   {"struct foo { [[op^erator int()]]; };", "CXXConversionDecl"},
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
-  // FIXME: The following to should be class itself instead.
+  {"struct foo { [[~^foo()]]; };", "CXXDestructorDecl"},
+  {"template  struct foo { ~foo<[[^T]]>(){} };",
+   "TemplateTypeParmTypeLoc"},
+  {"struct foo {}; void bar(foo *f) { [[f->~^foo]](); }", "MemberExpr"},
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
 
   {R"cpp(



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

[PATCH] D136212: [clangd] consider ~^foo() to target the destructor, not the type

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG62116c8f0b5b: [clangd] consider ~^foo() to target the 
destructor, not the type (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D136212?vs=468741&id=468875#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136212

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -466,7 +466,10 @@
   {"struct foo { [[int has^h<:32:>]]; };", "FieldDecl"},
   {"struct foo { [[op^erator int()]]; };", "CXXConversionDecl"},
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
-  // FIXME: The following to should be class itself instead.
+  {"struct foo { [[~^foo()]]; };", "CXXDestructorDecl"},
+  {"template  struct foo { ~foo<[[^T]]>(){} };",
+   "TemplateTypeParmTypeLoc"},
+  {"struct foo {}; void bar(foo *f) { [[f->~^foo]](); }", "MemberExpr"},
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
 
   {R"cpp(
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -860,7 +860,7 @@
   // is not available to the node's children.
   // Usually empty, but sometimes children cover tokens but shouldn't own them.
   SourceRange earlySourceRange(const DynTypedNode &N) {
-if (const Decl *D = N.get()) {
+if (const Decl *VD = N.get()) {
   // We want the name in the var-decl to be claimed by the decl itself and
   // not by any children. Ususally, we don't need this, because source
   // ranges of children are not overlapped with their parent's.
@@ -869,8 +869,20 @@
   //auto fun = [bar = foo]() { ... }
   //~   VarDecl
   //~~~ |- AutoTypeLoc
-  if (const auto *DD = llvm::dyn_cast(D))
-return DD->getLocation();
+  return VD->getLocation();
+}
+
+// When referring to a destructor ~Foo(), attribute Foo to the destructor
+// rather than the TypeLoc nested inside it.
+// We still traverse the TypeLoc, because it may contain other targeted
+// things like the T in ~Foo().
+if (const auto *CDD = N.get())
+  return CDD->getNameInfo().getNamedTypeInfo()->getTypeLoc().getBeginLoc();
+if (const auto *ME = N.get()) {
+  auto NameInfo = ME->getMemberNameInfo();
+  if (NameInfo.getName().getNameKind() ==
+  DeclarationName::CXXDestructorName)
+return NameInfo.getNamedTypeInfo()->getTypeLoc().getBeginLoc();
 }
 
 return SourceRange();


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -466,7 +466,10 @@
   {"struct foo { [[int has^h<:32:>]]; };", "FieldDecl"},
   {"struct foo { [[op^erator int()]]; };", "CXXConversionDecl"},
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
-  // FIXME: The following to should be class itself instead.
+  {"struct foo { [[~^foo()]]; };", "CXXDestructorDecl"},
+  {"template  struct foo { ~foo<[[^T]]>(){} };",
+   "TemplateTypeParmTypeLoc"},
+  {"struct foo {}; void bar(foo *f) { [[f->~^foo]](); }", "MemberExpr"},
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
 
   {R"cpp(
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -860,7 +860,7 @@
   // is not available to the node's children.
   // Usually empty, but sometimes children cover tokens but shouldn't own them.
   SourceRange earlySourceRange(const DynTypedNode &N) {
-if (const Decl *D = N.get()) {
+if (const Decl *VD = N.get()) {
   // We want the name in the var-decl to be claimed by the decl itself and
   // not by any children. Ususally, we don't need this, because source
   // ranges of children are not overlapped with their parent's.
@@ -869,8 +869,20 @@
   //auto fun = [bar = foo]() { ... }
   //~   VarDecl
   //~~~ |- AutoTypeLoc
-  if (const auto *DD = llvm::dyn_cast(D))
-return DD->getLocation();
+  return VD->getLocation();
+}
+
+// When referring to a destructor ~Foo(), attribute Foo to the destructor
+// rather than the TypeLoc nested ins

[PATCH] D136078: [RFC] Use-after-return binary metadata.

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov updated this revision to Diff 468876.
dvyukov added a comment.

take args alignment into account


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136078

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/CodeGen/CodeGenPassBuilder.h
  llvm/include/llvm/CodeGen/MachinePassRegistry.def
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/SanitizerMetadata.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -67,14 +67,14 @@
 private:
   // Forbid construction elsewhere.
   explicit constexpr MetadataInfo(StringRef FunctionPrefix,
-  StringRef SectionSuffix, int Feature)
+  StringRef SectionSuffix, uint32_t Feature)
   : FunctionPrefix(FunctionPrefix), SectionSuffix(SectionSuffix),
-FeatureMask(Feature != -1 ? (1u << Feature) : 0) {}
+FeatureMask(Feature) {}
 };
-const MetadataInfo MetadataInfo::Covered{"__sanitizer_metadata_covered",
- "sanmd_covered", -1};
-const MetadataInfo MetadataInfo::Atomics{"__sanitizer_metadata_atomics",
- "sanmd_atomics", 0};
+const MetadataInfo MetadataInfo::Covered{
+"__sanitizer_metadata_covered", "sanmd_covered", kSanitizerMetadataNone};
+const MetadataInfo MetadataInfo::Atomics{
+"__sanitizer_metadata_atomics", "sanmd_atomics", kSanitizerMetadataAtomics};
 
 // The only instances of MetadataInfo are the constants above, so a set of
 // them may simply store pointers to them. To deterministically generate code,
@@ -89,11 +89,16 @@
 cl::opt ClEmitAtomics("sanitizer-metadata-atomics",
 cl::desc("Emit PCs for atomic operations."),
 cl::Hidden, cl::init(false));
+cl::opt ClEmitUAR("sanitizer-metadata-uar",
+cl::desc("Emit PCs for start of functions that are "
+ "subject for use-after-return checking"),
+cl::Hidden, cl::init(false));
 
 //===--- Statistics ---===//
 
 STATISTIC(NumMetadataCovered, "Metadata attached to covered functions");
 STATISTIC(NumMetadataAtomics, "Metadata attached to atomics");
+STATISTIC(NumMetadataUAR, "Metadata attached to UAR functions");
 
 //===--===//
 
@@ -102,6 +107,7 @@
 transformOptionsFromCl(SanitizerBinaryMetadataOptions &&Opts) {
   Opts.Covered |= ClEmitCovered;
   Opts.Atomics |= ClEmitAtomics;
+  Opts.UAR |= ClEmitUAR;
   return std::move(Opts);
 }
 
@@ -142,7 +148,8 @@
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
-  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
+  bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB,
+ uint32_t &PerInstrFeatureMask);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
@@ -235,16 +242,21 @@
   uint32_t PerInstrFeatureMask = getEnabledPerInstructionFeature();
   // Don't emit unnecessary covered metadata for all functions to save space.
   bool RequiresCovered = false;
-  if (PerInstrFeatureMask) {
+  if (PerInstrFeatureMask || Options.UAR) {
 for (BasicBlock &BB : F)
   for (Instruction &I : BB)
-RequiresCovered |= runOn(I, MIS, MDB);
+RequiresCovered |= runOn(I, MIS, MDB, PerInstrFeatureMask);
   }
 
+  if (F.isVarArg())
+PerInstrFeatureMask &= ~kSanitizerMetadataUAR;
+  if (PerInstrFeatureMask & kSanitizerMetadataUAR)
+NumMetadataUAR++;
+
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unambiguously interpret it for
   // modules compiled with SanitizerBinaryMetadata.
-  if (Options.Covered || RequiresCovered) {
+  if (Options.Covered || (PerInstrFeatureMask && RequiresCovered)) {
 NumMetadataCovered++;
 const auto *MI = &MetadataInfo::Covered;
 MIS.insert(MI);
@@ -258,10 +270,25 @@
 }
 
 bool SanitizerBinaryMetadata::runOn(Instruction

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton added a comment.

> The problem as I see it was that the original bug, highly constrained the 
> cases where "CurrentState.LastSpace = State.Column;" to one particular style 
> (which if it happens to be your style great but not if its not.

You mean the original bugfix was already unnecessarily constrained, and now my 
proposed fix is only opening it up for one my case? That may be so. In any case 
this might really not be the ideal fix, and I'm open to any other, better way 
of fixing it.


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton updated this revision to Diff 468878.
hel-ableton added a comment.

Using verifyFormat() now instead of EXPECT_EQ()


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

https://reviews.llvm.org/D136154

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6594,7 +6594,25 @@
"(someOtherLongishConditionPart1 || "
"someOtherEvenLongerNestedConditionPart2);",
Style));
-}
+
+  Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  verifyFormat(
+  "struct Derived {\n"
+  "  Derived(\n"
+  "int firstArgWithLongName,\n"
+  "int secondArgWithLongName,\n"
+  "int thirdArgWithLongName,\n"
+  "int fourthArgWithLongName)\n"
+  "  : Base(\n"
+  "  firstArgWithLongName,\n"
+  "  secondArgWithLongName,\n"
+  "  thirdArgWithLongName,\n"
+  "  fourthArgWithLongName) {}\n"
+  "};", Style);}
 
 TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
   FormatStyle Style = getLLVMStyle();
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -809,8 +809,10 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;
+}
   } else if (Previous.is(TT_InheritanceColon)) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6594,7 +6594,25 @@
"(someOtherLongishConditionPart1 || "
"someOtherEvenLongerNestedConditionPart2);",
Style));
-}
+
+  Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  verifyFormat(
+  "struct Derived {\n"
+  "  Derived(\n"
+  "int firstArgWithLongName,\n"
+  "int secondArgWithLongName,\n"
+  "int thirdArgWithLongName,\n"
+  "int fourthArgWithLongName)\n"
+  "  : Base(\n"
+  "  firstArgWithLongName,\n"
+  "  secondArgWithLongName,\n"
+  "  thirdArgWithLongName,\n"
+  "  fourthArgWithLongName) {}\n"
+  "};", Style);}
 
 TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
   FormatStyle Style = getLLVMStyle();
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -809,8 +809,10 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;
+}
   } else if (Previous.is(TT_InheritanceColon)) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134453: Disambiguate type names when printing NTTP types

[Nenad Mikša via Phabricator via cfe-commits]

DoDoENT updated this revision to Diff 468881.
DoDoENT retitled this revision from "Introduce the 
`AlwaysIncludeTypeForNonTypeTemplateArgument` into printing policy" to 
"Disambiguate type names when printing NTTP types".
DoDoENT edited the summary of this revision.
DoDoENT added a comment.

Implemented changes requested in the review:

- type name of NTTP is now always printed, as C++ requires
- keep full NTTP type printing behind a policy, for those that want/need that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/SemaCXX/cxx2a-nttp-printing.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
  clang/test/SemaTemplate/temp_arg_string_printing.cpp
  clang/unittests/AST/TypePrinterTest.cpp

Index: clang/unittests/AST/TypePrinterTest.cpp
===
--- clang/unittests/AST/TypePrinterTest.cpp
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -48,7 +48,7 @@
   std::string Code = R"cpp(
 namespace N {
   template  struct Type {};
-  
+
   template 
   void Foo(const Type &Param);
 }
@@ -115,15 +115,65 @@
 
   ASSERT_TRUE(PrintedTypeMatches(
   Code, {"-std=c++20"}, Matcher,
-  R"(ASCII<{"this nontype template argument is [...]"}> &&)",
+  R"(ASCII{"this nontype template argument is [...]"}> &&)",
   [](PrintingPolicy &Policy) {
 Policy.EntireContentsOfLargeArray = false;
   }));
 
   ASSERT_TRUE(PrintedTypeMatches(
   Code, {"-std=c++20"}, Matcher,
-  R"(ASCII<{"this nontype template argument is too long to print"}> &&)",
+  R"(ASCII{"this nontype template argument is too long to print"}> &&)",
   [](PrintingPolicy &Policy) {
 Policy.EntireContentsOfLargeArray = true;
   }));
 }
+
+TEST(TypePrinter, TemplateIdWithComplexFullTypeNTTP) {
+  constexpr char Code[] = R"cpp(
+  template< typename T, auto ... dims >
+  struct NDArray {};
+
+  struct Dimension
+  {
+  using value_type = unsigned short;
+
+  value_type size{ value_type( 0 ) };
+  };
+
+  template < typename ConcreteDim >
+  struct DimensionImpl : Dimension {};
+
+  struct Width: DimensionImpl< Width> {};
+  struct Height   : DimensionImpl< Height   > {};
+  struct Channels : DimensionImpl< Channels > {};
+
+  inline constexpr WidthW;
+  inline constexpr Height   H;
+  inline constexpr Channels C;
+
+  template< auto ... Dims >
+  consteval auto makeArray() noexcept
+  {
+  return NDArray< float, Dims ... >{};
+  }
+
+  [[ maybe_unused ]] auto x { makeArray< H, W, C >() };
+
+  )cpp";
+  auto Matcher = varDecl(
+  allOf(hasAttr(attr::Kind::Unused), hasType(qualType().bind("id";
+
+  ASSERT_TRUE(PrintedTypeMatches(
+  Code, {"-std=c++20"}, Matcher,
+  R"(NDArray)",
+  [](PrintingPolicy &Policy) {
+Policy.AlwaysIncludeTypeForNonTypeTemplateArgument = false;
+  }));
+
+  ASSERT_TRUE(PrintedTypeMatches(
+  Code, {"-std=c++20"}, Matcher,
+  R"(NDArray{Dimension{0}}}, Width{DimensionImpl{Dimension{0}}}, Channels{DimensionImpl{Dimension{0}}}>)",
+  [](PrintingPolicy &Policy) {
+Policy.AlwaysIncludeTypeForNonTypeTemplateArgument = true;
+  }));
+}
Index: clang/test/SemaTemplate/temp_arg_string_printing.cpp
===
--- clang/test/SemaTemplate/temp_arg_string_printing.cpp
+++ clang/test/SemaTemplate/temp_arg_string_printing.cpp
@@ -19,111 +19,111 @@
 template  class ASCII {};
 
 void not_string() {
-  // CHECK{LITERAL}: ASCII<{{9, -1, 42}}>
+  // CHECK{LITERAL}: ASCII{{9, -1, 42}}>
   new ASCII<(int[]){9, -1, 42}>;
-  // CHECK{LITERAL}: ASCII<{{3.14e+00, 0.00e+00, 4.20e+01}}>
+  // CHECK{LITERAL}: ASCII{{3.14e+00, 0.00e+00, 4.20e+01}}>
   new ASCII<(double[]){3.14, 0., 42.}>;
 }
 
 void narrow() {
-  // CHECK{LITERAL}: ASCII<{""}>
+  // CHECK{LITERAL}: ASCII{""}>
   new ASCII<"">;
-  // CHECK{LITERAL}: ASCII<{"the quick brown fox jumps"}>
+  // CHECK{LITERAL}: ASCII{"the quick brown fox jumps"}>
   new ASCII<"the quick brown fox jumps">;
-  // CHECK{LITERAL}: ASCII<{"OVER THE LAZY DOG 0123456789"}>
+  // CHECK{LITERAL}: ASCII{"OVER THE LAZY DOG 0123456789"}>
   new ASCII<"OVER THE LAZY DOG 0123456789">;
-  // CHECK{LITERAL}: ASCII<{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  // CHECK{LITERAL}: ASCII{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
   new ASCII?/)">;
-  // CHECK{LITERAL}: ASCII<{{101, 115, 99, 97, 112, 101, 0, 0}}>
+  // CHECK{LITERAL}: ASCII{{101, 115, 99, 97, 112, 101, 0, 0}}>
   new ASCII<"escape\0">;
-  // CHECK{LITERAL}: ASCII<{"escape\r\n"}>
+  // CHECK{LITERAL}: ASCII{"escape\r\n"}>
   new ASCII<"escape\r\n">;
-  // CHECK{LITERAL}: ASCII<{"escape\\\t\f\v"}>
+  // CHECK{LITERAL}: ASCII{"escap

[PATCH] D134453: Disambiguate type names when printing NTTP types

[Nenad Mikša via Phabricator via cfe-commits]

DoDoENT updated this revision to Diff 468883.
DoDoENT added a comment.

Update comment/documentation of the new policy to correctly reflect the change 
in previous diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/SemaCXX/cxx2a-nttp-printing.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
  clang/test/SemaTemplate/temp_arg_string_printing.cpp
  clang/unittests/AST/TypePrinterTest.cpp

Index: clang/unittests/AST/TypePrinterTest.cpp
===
--- clang/unittests/AST/TypePrinterTest.cpp
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -48,7 +48,7 @@
   std::string Code = R"cpp(
 namespace N {
   template  struct Type {};
-  
+
   template 
   void Foo(const Type &Param);
 }
@@ -115,15 +115,65 @@
 
   ASSERT_TRUE(PrintedTypeMatches(
   Code, {"-std=c++20"}, Matcher,
-  R"(ASCII<{"this nontype template argument is [...]"}> &&)",
+  R"(ASCII{"this nontype template argument is [...]"}> &&)",
   [](PrintingPolicy &Policy) {
 Policy.EntireContentsOfLargeArray = false;
   }));
 
   ASSERT_TRUE(PrintedTypeMatches(
   Code, {"-std=c++20"}, Matcher,
-  R"(ASCII<{"this nontype template argument is too long to print"}> &&)",
+  R"(ASCII{"this nontype template argument is too long to print"}> &&)",
   [](PrintingPolicy &Policy) {
 Policy.EntireContentsOfLargeArray = true;
   }));
 }
+
+TEST(TypePrinter, TemplateIdWithComplexFullTypeNTTP) {
+  constexpr char Code[] = R"cpp(
+  template< typename T, auto ... dims >
+  struct NDArray {};
+
+  struct Dimension
+  {
+  using value_type = unsigned short;
+
+  value_type size{ value_type( 0 ) };
+  };
+
+  template < typename ConcreteDim >
+  struct DimensionImpl : Dimension {};
+
+  struct Width: DimensionImpl< Width> {};
+  struct Height   : DimensionImpl< Height   > {};
+  struct Channels : DimensionImpl< Channels > {};
+
+  inline constexpr WidthW;
+  inline constexpr Height   H;
+  inline constexpr Channels C;
+
+  template< auto ... Dims >
+  consteval auto makeArray() noexcept
+  {
+  return NDArray< float, Dims ... >{};
+  }
+
+  [[ maybe_unused ]] auto x { makeArray< H, W, C >() };
+
+  )cpp";
+  auto Matcher = varDecl(
+  allOf(hasAttr(attr::Kind::Unused), hasType(qualType().bind("id";
+
+  ASSERT_TRUE(PrintedTypeMatches(
+  Code, {"-std=c++20"}, Matcher,
+  R"(NDArray)",
+  [](PrintingPolicy &Policy) {
+Policy.AlwaysIncludeTypeForNonTypeTemplateArgument = false;
+  }));
+
+  ASSERT_TRUE(PrintedTypeMatches(
+  Code, {"-std=c++20"}, Matcher,
+  R"(NDArray{Dimension{0}}}, Width{DimensionImpl{Dimension{0}}}, Channels{DimensionImpl{Dimension{0}}}>)",
+  [](PrintingPolicy &Policy) {
+Policy.AlwaysIncludeTypeForNonTypeTemplateArgument = true;
+  }));
+}
Index: clang/test/SemaTemplate/temp_arg_string_printing.cpp
===
--- clang/test/SemaTemplate/temp_arg_string_printing.cpp
+++ clang/test/SemaTemplate/temp_arg_string_printing.cpp
@@ -19,111 +19,111 @@
 template  class ASCII {};
 
 void not_string() {
-  // CHECK{LITERAL}: ASCII<{{9, -1, 42}}>
+  // CHECK{LITERAL}: ASCII{{9, -1, 42}}>
   new ASCII<(int[]){9, -1, 42}>;
-  // CHECK{LITERAL}: ASCII<{{3.14e+00, 0.00e+00, 4.20e+01}}>
+  // CHECK{LITERAL}: ASCII{{3.14e+00, 0.00e+00, 4.20e+01}}>
   new ASCII<(double[]){3.14, 0., 42.}>;
 }
 
 void narrow() {
-  // CHECK{LITERAL}: ASCII<{""}>
+  // CHECK{LITERAL}: ASCII{""}>
   new ASCII<"">;
-  // CHECK{LITERAL}: ASCII<{"the quick brown fox jumps"}>
+  // CHECK{LITERAL}: ASCII{"the quick brown fox jumps"}>
   new ASCII<"the quick brown fox jumps">;
-  // CHECK{LITERAL}: ASCII<{"OVER THE LAZY DOG 0123456789"}>
+  // CHECK{LITERAL}: ASCII{"OVER THE LAZY DOG 0123456789"}>
   new ASCII<"OVER THE LAZY DOG 0123456789">;
-  // CHECK{LITERAL}: ASCII<{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  // CHECK{LITERAL}: ASCII{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
   new ASCII?/)">;
-  // CHECK{LITERAL}: ASCII<{{101, 115, 99, 97, 112, 101, 0, 0}}>
+  // CHECK{LITERAL}: ASCII{{101, 115, 99, 97, 112, 101, 0, 0}}>
   new ASCII<"escape\0">;
-  // CHECK{LITERAL}: ASCII<{"escape\r\n"}>
+  // CHECK{LITERAL}: ASCII{"escape\r\n"}>
   new ASCII<"escape\r\n">;
-  // CHECK{LITERAL}: ASCII<{"escape\\\t\f\v"}>
+  // CHECK{LITERAL}: ASCII{"escape\\\t\f\v"}>
   new ASCII<"escape\\\t\f\v">;
-  // CHECK{LITERAL}: ASCII<{"escape\a\bc"}>
+  // CHECK{LITERAL}: ASCII{"escape\a\bc"}>
   new ASCII<"escape\a\b\c">;
-  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  // CHECK{LITERAL}: ASCII{{110, 111, 116, 17, 0}}>
   new ASCII<"not\x11">;
-  // CHEC

[PATCH] D135964: [clang][dataflow] Add equivalence relation for `Value` type.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 468884.
ymandel marked an inline comment as done.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135964

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Value.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/ValueTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
@@ -0,0 +1,85 @@
+//===- unittests/Analysis/FlowSensitive/ValueTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace {
+
+using namespace clang;
+using namespace dataflow;
+
+TEST(ValueTest, EquivalenceReflexive) {
+  StructValue V;
+  EXPECT_TRUE(areEquivalentValues(V, V));
+}
+
+TEST(ValueTest, AliasedReferencesEquivalent) {
+  auto L = ScalarStorageLocation(QualType());
+  ReferenceValue V1(L);
+  ReferenceValue V2(L);
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, AliasedPointersEquivalent) {
+  auto L = ScalarStorageLocation(QualType());
+  PointerValue V1(L);
+  PointerValue V2(L);
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, TopsEquivalent) {
+  TopBoolValue V1;
+  TopBoolValue V2;
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, EquivalentValuesWithDifferentPropsEquivalent) {
+  TopBoolValue Prop1;
+  TopBoolValue Prop2;
+  TopBoolValue V1;
+  TopBoolValue V2;
+  V1.setProperty("foo", Prop1);
+  V2.setProperty("bar", Prop2);
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, DifferentKindsNotEquivalent) {
+  auto L = ScalarStorageLocation(QualType());
+  ReferenceValue V1(L);
+  TopBoolValue V2;
+  EXPECT_FALSE(areEquivalentValues(V1, V2));
+  EXPECT_FALSE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, NotAliasedReferencesNotEquivalent) {
+  auto L1 = ScalarStorageLocation(QualType());
+  ReferenceValue V1(L1);
+  auto L2 = ScalarStorageLocation(QualType());
+  ReferenceValue V2(L2);
+  EXPECT_FALSE(areEquivalentValues(V1, V2));
+  EXPECT_FALSE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, NotAliasedPointersNotEquivalent) {
+  auto L1 = ScalarStorageLocation(QualType());
+  PointerValue V1(L1);
+  auto L2 = ScalarStorageLocation(QualType());
+  PointerValue V2(L2);
+  EXPECT_FALSE(areEquivalentValues(V1, V2));
+  EXPECT_FALSE(areEquivalentValues(V2, V1));
+}
+
+} // namespace
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -517,11 +517,10 @@
 Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
   return false;
 
-auto *Prop1  = Val1.getProperty("has_value");
+auto *Prop1 = Val1.getProperty("has_value");
 auto *Prop2 = Val2.getProperty("has_value");
-return Prop1 == Prop2 ||
-   (Prop1 != nullptr && Prop2 != nullptr && isa(Prop1) &&
-isa(Prop2));
+assert(Prop1 != nullptr && Prop2 != nullptr);
+return areEquivalentValues(*Prop1, *Prop2);
   }
 
   bool merge(QualType Type, const Value &Val1, const Environment &Env1,
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -19,6 +19,7 @@
   TypeErasedDataflowAnalysisTest.cpp
   SolverTest.cpp
   UncheckedOptionalAccessModelTest.cpp
+  ValueTest.cpp
   )
 
 clang_target_link_libraries(ClangAnalysisFlowSensitiveTests
Index: clang/lib/Analysis/FlowSensitive/Value.cpp
===
--- /dev/null
+++ clang/lib/Analysis/FlowSensitive/Value.cpp
@@ -0,0 +1,39 @@
+//===-- Value.cpp ---*- C++ 

[clang] 8cadac4 - [clang][dataflow] Add equivalence relation `Value` type.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-10-19T12:23:09Z
New Revision: 8cadac41e9f63b2494805042573792cc2cc2a8ea

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

LOG: [clang][dataflow] Add equivalence relation `Value` type.

Defines an equivalence relation on the `Value` type to standardize several
places in the code where we replicate the ~same equivalence comparison.

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

Added: 
clang/lib/Analysis/FlowSensitive/Value.cpp
clang/unittests/Analysis/FlowSensitive/ValueTest.cpp

Modified: 
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/CMakeLists.txt
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/Value.h 
b/clang/include/clang/Analysis/FlowSensitive/Value.h
index b60f93a51965b..334c1388ecf75 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -77,6 +77,17 @@ class Value {
   llvm::StringMap Properties;
 };
 
+/// An equivalence relation for values. It obeys reflexivity, symmetry and
+/// transitivity. It does *not* include comparison of `Properties`.
+///
+/// Computes equivalence for these subclasses:
+/// * ReferenceValue, PointerValue -- pointee locations are equal. Does not
+///   compute deep equality of `Value` at said location.
+/// * TopBoolValue -- both are `TopBoolValue`s.
+///
+/// Otherwise, falls back to pointer equality.
+bool areEquivalentValues(const Value &Val1, const Value &Val2);
+
 /// Models a boolean.
 class BoolValue : public Value {
 public:

diff  --git a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt 
b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
index b57ecc97d630f..1a49998c39c20 100644
--- a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
+++ b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
@@ -4,6 +4,7 @@ add_clang_library(clangAnalysisFlowSensitive
   DataflowEnvironment.cpp
   Transfer.cpp
   TypeErasedDataflowAnalysis.cpp
+  Value.cpp
   WatchedLiteralsSolver.cpp
   DebugSupport.cpp
 

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 29fb2b7603e30..0b098c43ba3d1 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -49,28 +49,6 @@ llvm::DenseMap intersectDenseMaps(const 
llvm::DenseMap &Map1,
   return Result;
 }
 
-static bool areEquivalentIndirectionValues(Value *Val1, Value *Val2) {
-  if (auto *IndVal1 = dyn_cast(Val1)) {
-auto *IndVal2 = cast(Val2);
-return &IndVal1->getReferentLoc() == &IndVal2->getReferentLoc();
-  }
-  if (auto *IndVal1 = dyn_cast(Val1)) {
-auto *IndVal2 = cast(Val2);
-return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc();
-  }
-  return false;
-}
-
-/// Returns true if and only if `Val1` is equivalent to `Val2`.
-static bool equivalentValues(QualType Type, Value *Val1,
- const Environment &Env1, Value *Val2,
- const Environment &Env2,
- Environment::ValueModel &Model) {
-  return Val1 == Val2 || (isa(Val1) && isa(Val2)) 
||
- areEquivalentIndirectionValues(Val1, Val2) ||
- Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2);
-}
-
 /// Attempts to merge distinct values `Val1` and `Val2` in `Env1` and `Env2`,
 /// respectively, of the same type `Type`. Merging generally produces a single
 /// value that (soundly) approximates the two inputs, although the actual
@@ -93,11 +71,6 @@ static Value *mergeDistinctValues(QualType Type, Value *Val1,
 return &MergedVal;
   }
 
-  // FIXME: add unit tests that cover this statement.
-  if (areEquivalentIndirectionValues(Val1, Val2)) {
-return Val1;
-  }
-
   // FIXME: Consider destroying `MergedValue` immediately if 
`ValueModel::merge`
   // returns false to avoid storing unneeded values in `DACtx`.
   if (Value *MergedVal = MergedEnv.createValue(Type))
@@ -321,7 +294,9 @@ bool Environment::equivalentTo(const Environment &Other,
   continue;
 assert(It->second != nullptr);
 
-if (!equivalentValues(Loc->getType(), Val, *this, It->second, Other, 
Model))
+if (!areEquivalentValues(*Val, *It->second) &&
+!Model.compareEquivalent(Loc->getType(), *Val, *this, *It->second,
+ Other))
   return false;
   }
 
@@ -372,8 +347,7 @@ LatticeJoinEffect Environment::join(const Environment 
&Other,
   continue;
 assert(It->second != nullptr);
 
-   

[PATCH] D135964: [clang][dataflow] Add equivalence relation for `Value` type.

[Yitzhak Mandelbaum 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 rG8cadac41e9f6: [clang][dataflow] Add equivalence relation 
`Value` type. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135964

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Value.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/ValueTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
@@ -0,0 +1,85 @@
+//===- unittests/Analysis/FlowSensitive/ValueTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace {
+
+using namespace clang;
+using namespace dataflow;
+
+TEST(ValueTest, EquivalenceReflexive) {
+  StructValue V;
+  EXPECT_TRUE(areEquivalentValues(V, V));
+}
+
+TEST(ValueTest, AliasedReferencesEquivalent) {
+  auto L = ScalarStorageLocation(QualType());
+  ReferenceValue V1(L);
+  ReferenceValue V2(L);
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, AliasedPointersEquivalent) {
+  auto L = ScalarStorageLocation(QualType());
+  PointerValue V1(L);
+  PointerValue V2(L);
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, TopsEquivalent) {
+  TopBoolValue V1;
+  TopBoolValue V2;
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, EquivalentValuesWithDifferentPropsEquivalent) {
+  TopBoolValue Prop1;
+  TopBoolValue Prop2;
+  TopBoolValue V1;
+  TopBoolValue V2;
+  V1.setProperty("foo", Prop1);
+  V2.setProperty("bar", Prop2);
+  EXPECT_TRUE(areEquivalentValues(V1, V2));
+  EXPECT_TRUE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, DifferentKindsNotEquivalent) {
+  auto L = ScalarStorageLocation(QualType());
+  ReferenceValue V1(L);
+  TopBoolValue V2;
+  EXPECT_FALSE(areEquivalentValues(V1, V2));
+  EXPECT_FALSE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, NotAliasedReferencesNotEquivalent) {
+  auto L1 = ScalarStorageLocation(QualType());
+  ReferenceValue V1(L1);
+  auto L2 = ScalarStorageLocation(QualType());
+  ReferenceValue V2(L2);
+  EXPECT_FALSE(areEquivalentValues(V1, V2));
+  EXPECT_FALSE(areEquivalentValues(V2, V1));
+}
+
+TEST(ValueTest, NotAliasedPointersNotEquivalent) {
+  auto L1 = ScalarStorageLocation(QualType());
+  PointerValue V1(L1);
+  auto L2 = ScalarStorageLocation(QualType());
+  PointerValue V2(L2);
+  EXPECT_FALSE(areEquivalentValues(V1, V2));
+  EXPECT_FALSE(areEquivalentValues(V2, V1));
+}
+
+} // namespace
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -517,11 +517,10 @@
 Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
   return false;
 
-auto *Prop1  = Val1.getProperty("has_value");
+auto *Prop1 = Val1.getProperty("has_value");
 auto *Prop2 = Val2.getProperty("has_value");
-return Prop1 == Prop2 ||
-   (Prop1 != nullptr && Prop2 != nullptr && isa(Prop1) &&
-isa(Prop2));
+assert(Prop1 != nullptr && Prop2 != nullptr);
+return areEquivalentValues(*Prop1, *Prop2);
   }
 
   bool merge(QualType Type, const Value &Val1, const Environment &Env1,
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -19,6 +19,7 @@
   TypeErasedDataflowAnalysisTest.cpp
   SolverTest.cpp
   UncheckedOptionalAccessModelTest.cpp
+  ValueTest.cpp
   )
 
 clang_target_link_libraries(ClangAnalysisFlowSensitiveTests
Index: clang/lib/Analysis/FlowSensitive/Value.cpp
===
--- /dev/null
+++ clang/lib/Analysis/Fl

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton marked an inline comment as done.
hel-ableton added a comment.

In D136154#3867968 , @MyDeveloperDay 
wrote:

> Doesn't something like this  achieve the same
>
>CurrentState.Indent = State.Column;
>CurrentState.LastSpace = State.Column;
>   -  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
>   -   TT_CtorInitializerColon)) &&
>   +  } else if (Previous.is(TT_CtorInitializerColon)) {
>   +CurrentState.LastSpace = State.Column;
>   +  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
> ((Previous.getPrecedence() != prec::Assignment &&
>   (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 
> 0 ||
>
> Regardless of the choice of BreakBeforeBinaryOperators?
>
> BTW I checked such a change passes all the other unit tests. (Beyonce rule!!)

If you think this would be the better fix, AFAICS it does what we need. What's 
the Beyonce rule, by the way?


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

https://reviews.llvm.org/D136154

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

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp:106
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note 
{{candidate function}}
 return 2;

ychen wrote:
> usaxena95 wrote:
> > Thanks for working on this.
> > 
> > I wanted to bring up related: 
> > https://github.com/llvm/llvm-project/issues/56154
> > Eg.: Removing this `const` still removes the ambiguity but it shouldn't.
> > Since you have more context, does this look related to you ?
> Removing `const` makes the partial ordering compare constraints where 
> `AtLeast2 && true` subsumes `AtLeast2` so it is not ambiguous 
> anymore. Similar reasoning could be made for the original test case of 
> https://github.com/llvm/llvm-project/issues/56154. 56154 exposes an issue in 
> constraints partial ordering which is not related to this patch.
Thanks for the clarification. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128750

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

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, I think the new diagnostic wording is an improvement. Can you please add 
a release note to mention the diagnostic wording was patched up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135920

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> Beyonce rule

"If they liked it they should have put a test on it"


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D136154#3868151 , @hel-ableton 
wrote:

> If you think this would be the better fix, AFAICS it does what we need. 
> What's the Beyonce rule, by the way?

I do, but I'd like to hear @owenpan and @HazardyKnusperkeks view.. (each of us 
is better in different part of clang-format, and I value their opinion!)


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:6615
+  "  fourthArgWithLongName) {}\n"
+  "};", Style);}
 

missing newline


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

https://reviews.llvm.org/D136154

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

[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:1530
+
+def record_needs_implicit_default_constructor(self):
+"""Returns True if the cursor refers to a C++ record declaration

dblaikie wrote:
> anderslanglands wrote:
> > dblaikie wrote:
> > > royjacobson wrote:
> > > > anderslanglands wrote:
> > > > > dblaikie wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > dblaikie wrote:
> > > > > > > > anderslanglands wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > royjacobson wrote:
> > > > > > > > > > > > anderslanglands wrote:
> > > > > > > > > > > > > royjacobson wrote:
> > > > > > > > > > > > > > anderslanglands wrote:
> > > > > > > > > > > > > > > royjacobson wrote:
> > > > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > > > > I don't think we should expose any of the 
> > > > > > > > > > > > > > > > > > "needs" functions like this -- those are 
> > > > > > > > > > > > > > > > > > internal implementation details of the 
> > > > > > > > > > > > > > > > > > class and I don't think we want to calcify 
> > > > > > > > > > > > > > > > > > that into something we have to support 
> > > > > > > > > > > > > > > > > > forever. As we add members to a class, we 
> > > > > > > > > > > > > > > > > > recalculate whether the added member causes 
> > > > > > > > > > > > > > > > > > us to delete defaulted special members 
> > > > > > > > > > > > > > > > > > (among other things), and the "needs" 
> > > > > > > > > > > > > > > > > > functions are basically used when the class 
> > > > > > > > > > > > > > > > > > is completed to handle lazily created 
> > > > > > > > > > > > > > > > > > special members. I'm pretty sure that lazy 
> > > > > > > > > > > > > > > > > > creation is not mandated by the standard, 
> > > > > > > > > > > > > > > > > > which is why I think the "needs" functions 
> > > > > > > > > > > > > > > > > > are more of an implementation detail.
> > > > > > > > > > > > > > > > > CC @erichkeane and @royjacobson as folks who 
> > > > > > > > > > > > > > > > > have been in this same area of the compiler 
> > > > > > > > > > > > > > > > > to see if they agree or disagree with my 
> > > > > > > > > > > > > > > > > assessment there.
> > > > > > > > > > > > > > > > I think so. The 'needs_*' functions query 
> > > > > > > > > > > > > > > > `DeclaredSpecialMembers` and I'm pretty sure 
> > > > > > > > > > > > > > > > it's modified when we add the implicit 
> > > > > > > > > > > > > > > > definitions in the class completion code. So 
> > > > > > > > > > > > > > > > this looks a bit suspicious. Is this API 
> > > > > > > > > > > > > > > > //meant// to be used with incomplete classes?
> > > > > > > > > > > > > > > > For complete classes I think looking up the 
> > > > > > > > > > > > > > > > default/move/copy constructor and calling 
> > > > > > > > > > > > > > > > `isImplicit()` is the way to do it.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > About the 'is deleted' API - can't the same be 
> > > > > > > > > > > > > > > > done for those functions as well so we have a 
> > > > > > > > > > > > > > > > smaller API? 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > If this //is// meant to be used with incomplete 
> > > > > > > > > > > > > > > > classes for efficiency that would be another 
> > > > > > > > > > > > > > > > thing, I guess.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > So the intended use case here is I'm using 
> > > > > > > > > > > > > > > libclang to parse an existing C++ libray's 
> > > > > > > > > > > > > > > headers and generate a C interface to it. To do 
> > > > > > > > > > > > > > > that I need to know if I need to generate default 
> > > > > > > > > > > > > > > constructors etc, which the needs* methods do for 
> > > > > > > > > > > > > > > me (I believe). The alternative is I have to 
> > > > > > > > > > > > > > > check manually whether all the 
> > > > > > > > > > > > > > > constructors/assignment operators exist, then 
> > > > > > > > > > > > > > > implement the implicit declaration rules myself 
> > > > > > > > > > > > > > > correctly for each version of the standard, which 
> > > > > > > > > > > > > > > I'd rather avoid.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Would putting a note in the doc comment about the 
> > > > > > > > > > > > > > > behaviour differing when the class is being 
> > > > > > > > > > > > > > > constructed as originally suggested work for 
> > > > > > > > > > > > > > > everyone?
> > > > > > > > > > > > > > Why is the `__is_default_constructible` builtin 
> > > > > > > > > > > > > > type trait not enough? Do you have different 
> > > > > > > > > > > > > > behavior for user provided and implicit default 
> > > > > > > > > > > > > > constructors?
> > > > > > > > > > > > > > 
> > > > > > > > > > 

[PATCH] D135429: [HLSL] [DirectX backend] Move generateGlobalCtorDtorCalls into DirectX backend.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I don’t think you should be deleting all those frontend tests. Those tests 
existed before your change to remove the global constructor global variable, 
they provide valuable coverage of the frontend constructor generation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135429

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

[PATCH] D132329: [X86][RFC] Using `__bf16` for AVX512_BF16 intrinsics

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Headers/avx512bf16intrin.h:13
 
+#ifdef __SSE2__
+

LuoYuanke wrote:
> What is this macro check used for?
`__bf16` is not available without SSE2. This is to make sure no error generated 
if user include  



Comment at: clang/test/CodeGen/X86/avx512bf16-error.c:14
+__bfloat16 bar(__bfloat16 a, __bfloat16 b) {
+  return a + b;
+}

LuoYuanke wrote:
> Need test for other operations (-, *, /) as well?
I don't think so. This is to check `__bfloat16` is deprecated. We should check 
them when enabling `__bf16` on SSE2.



Comment at: llvm/include/llvm/IR/IntrinsicsX86.td:4928
   Intrinsic<[llvm_v4f32_ty],
   [llvm_v4f32_ty, llvm_v4i32_ty, llvm_v4i32_ty], [IntrNoMem]>;
   def int_x86_avx512bf16_dpbf16ps_256:

LuoYuanke wrote:
> It seems we still use i32 to represent <2 x bf16>, but we don't have a better 
> way since 1 bit mask cover a pair of bf16 elements.
I think mask is not an issue because both the passthru and dst are <4 x float>.



Comment at: llvm/lib/IR/AutoUpgrade.cpp:4095
+Intrinsic::x86_avx512bf16_mask_cvtneps2bf16_128)
+  Args[1] = Builder.CreateBitCast(
+  Args[1], FixedVectorType::get(Builder.getBFloatTy(), NumElts));

LuoYuanke wrote:
> Why there is no bitcast for the input for the other intrinsics? I expect to 
> see the bitcast from vXi16 to vXbf16.
Others don't have vXbf16 in inputs.



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:3916
+multiclass mask_move_lowering_f16_bf16 {
 let Predicates = [HasBWI] in {
+  def : Pat<(_.info512.VT (vselect VK32WM:$mask, (_.info512.VT VR512:$src1), 
(_.info512.VT VR512:$src0))),

LuoYuanke wrote:
> Not sure the indent is correct or not.
The format is chaos in td files, at least we have code using in this way :)



Comment at: llvm/test/CodeGen/X86/bfloat.ll:32
+; BF16-NEXT:shll $16, %eax
+; BF16-NEXT:vmovd %eax, %xmm1
+; BF16-NEXT:vaddss %xmm0, %xmm1, %xmm0

LuoYuanke wrote:
> It seems the difference between SSE2 and BF16 is using SSE instruction or AVX 
> instruction. What do we expect to test for BF16?
This is to make sure the scalar type works under AVX512-BF16. We may optimize 
it with `vcvtneps2bf16` in future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132329

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:54-62
+  explicit operator int8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int64_t() const { return toAPSInt().getExtValue(); }

sepavloff wrote:
> tbaeder wrote:
> > sepavloff wrote:
> > > Conversions to integers are bitcasts+truncation but the conversion to 
> > > bool is different. What semantics have these operations?
> > They are basically used for casting. I didn't mean to make the bool 
> > implementation different, just seemed to make sense to me to do it this way.
> If they are used for casting, they need to preserve value, that is conversion 
> float->int32 should transform 1.0->0x0001. The method `toAPSInt` uses 
> bitcast, so it converts 1.0->0x3f80. `APFloat::convertToInteger` should 
> be used to make value-preverving conversion but it requires knowledge of 
> rounding mode.
That's good to know, thanks.  I've already used `TowardZero` in 
`Floating::toSemantics()`, but that was more a guess.

Is the rounding mode I'm supposed to use simply 
`LangOptions::getDefaultRoundingMode()`?


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

https://reviews.llvm.org/D134859

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

[PATCH] D136017: [clang][Interp] Materializing primitive temporaries

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

tbaeder updated this revision to Diff 468895.

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

https://reviews.llvm.org/D136017

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/references.cpp

Index: clang/test/AST/Interp/references.cpp
===
--- clang/test/AST/Interp/references.cpp
+++ clang/test/AST/Interp/references.cpp
@@ -2,8 +2,6 @@
 // RUN: %clang_cc1 -verify=ref %s
 
 
-// ref-no-diagnostics
-
 constexpr int a = 10;
 constexpr const int &b = a;
 static_assert(a == b, "");
@@ -71,9 +69,22 @@
 }
 static_assert(testGetValue() == 30, "");
 
-// FIXME: ExprWithCleanups + MaterializeTemporaryExpr not implemented
-constexpr const int &MCE = 1; // expected-error{{must be initialized by a constant expression}}
+constexpr const int &MCE = 20;
+static_assert(MCE == 20, "");
+static_assert(MCE == 30, ""); // expected-error {{static assertion failed}} \
+  // expected-note {{evaluates to '20 == 30'}} \
+  // ref-error {{static assertion failed}} \
+  // ref-note {{evaluates to '20 == 30'}}
 
+constexpr int LocalMCE() {
+  const int &m = 100;
+  return m;
+}
+static_assert(LocalMCE() == 100, "");
+static_assert(LocalMCE() == 200, ""); // expected-error {{static assertion failed}} \
+  // expected-note {{evaluates to '100 == 200'}} \
+  // ref-error {{static assertion failed}} \
+  // ref-note {{evaluates to '100 == 200'}}
 
 struct S {
   int i, j;
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -48,6 +48,7 @@
 def ArgRecordDecl : ArgType { let Name = "const RecordDecl *"; }
 def ArgRecordField : ArgType { let Name = "const Record::Field *"; }
 def ArgFltSemantics : ArgType { let Name = "const llvm::fltSemantics *"; }
+def ArgLETD: ArgType { let Name = "const LifetimeExtendedTemporaryDecl *"; }
 
 //===--===//
 // Classes of types instructions operate on.
@@ -307,6 +308,10 @@
 // [Value] -> []
 def InitGlobal : AccessOpcode;
 // [Value] -> []
+def InitGlobalTemp : AccessOpcode {
+  let Args = [ArgUint32, ArgLETD];
+}
+// [Value] -> []
 def SetGlobal : AccessOpcode;
 
 // [] -> [Value]
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -578,6 +578,22 @@
   return true;
 }
 
+/// 1) Converts the value on top of the stack to an APValue
+/// 2) Sets that APValue on \Temp
+/// 3) Initialized global with index \I with that
+template ::T>
+bool InitGlobalTemp(InterpState &S, CodePtr OpPC, uint32_t I,
+const LifetimeExtendedTemporaryDecl *Temp) {
+  assert(Temp);
+  const T Value = S.Stk.peek();
+  APValue APV = Value.toAPValue();
+  APValue *Cached = Temp->getOrCreateValue(true);
+  *Cached = APV;
+
+  S.P.getGlobal(I)->deref() = S.Stk.pop();
+  return true;
+}
+
 template ::T>
 bool InitThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
   if (S.checkingPotentialConstantExpression())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -93,6 +93,8 @@
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
   bool VisitCharacterLiteral(const CharacterLiteral *E);
+  bool VisitExprWithCleanups(const ExprWithCleanups *E);
+  bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -499,6 +499,57 @@
   return this->emitConst(E, E->getValue());
 }
 
+template 
+bool ByteCodeExprGen::VisitExprWithCleanups(
+const ExprWithCleanups *E) {
+  const Expr *SubExpr = E->getSubExpr();
+
+  assert(E->getNumObjects() == 0 && "TODO: Implement cleanups");
+  return this->visit(SubExpr);
+}
+
+template 
+bool ByteCodeExprGen::VisitMaterializeTemporaryExpr(
+const MaterializeTemporaryExpr *E) {
+  StorageDuration SD = E->getStorageDuration();
+
+  // We conservatively only support these for now.
+  if (SD != SD_Static && SD != SD_Automatic)
+return false;
+
+  const Expr *SubExpr = E->getSubExpr();
+  Optional SubExprT = classify(SubExpr);
+  // FIXME: Implement this for records and array

[clang] a6b4204 - [analyzer] Fix the liveness of Symbols for values in regions referred by LazyCompoundVal

[Tomasz Kamiński via cfe-commits]

Author: Tomasz Kamiński
Date: 2022-10-19T16:06:32+02:00
New Revision: a6b42040ad30c13474c06d3e0291a4675475fec3

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

LOG: [analyzer] Fix the liveness of Symbols for values in regions referred by 
LazyCompoundVal

To illustrate our current understanding, let's start with the following program:
https://godbolt.org/z/33f6vheh1
```lang=c++
void clang_analyzer_printState();

struct C {
   int x;
   int y;
   int more_padding;
};

struct D {
   C c;
   int z;
};

C foo(D d, int new_x, int new_y) {
   d.c.x = new_x;   // B1
   assert(d.c.x < 13);  // C1

   C c = d.c;   // L

   assert(d.c.y < 10);  // C2
   assert(d.z < 5); // C3

   d.c.y = new_y;   // B2

   assert(d.c.y < 10);  // C4

   return c;  // R
}
```
In the code, we create a few bindings to subregions of root region `d` (`B1`, 
`B2`), a constrain on the values  (`C1`, `C2`, ….), and create a 
`lazyCompoundVal` for the part of the region `d` at point `L`, which is 
returned at point `R`.

Now, the question is which of these should remain live as long the return value 
of the `foo` call is live. In perfect a word we should preserve:

  # only the bindings of the subregions of `d.c`, which were created before the 
copy at `L`. In our example, this includes `B1`, and not `B2`.  In other words, 
`new_x` should be live but `new_y` shouldn’t.

  # constraints on the values of `d.c`, that are reachable through `c`. This 
can be created both before the point of making the copy (`L`) or after. In our 
case, that would be `C1` and `C2`. But not `C3` (`d.z` value is not reachable 
through `c`) and `C4` (the original value of`d.c.y` was overridden at `B2` 
after the creation of `c`).

The current code in the `RegionStore` covers the use case (1), by using the 
`getInterestingValues()` to extract bindings to parts of the referred region 
present in the store at the point of copy. This also partially covers point 
(2), in case when constraints are applied to a location that has binding at the 
point of the copy (in our case `d.c.x` in `C1` that has value `new_x`), but it 
fails to preserve the constraints that require creating a new symbol for 
location (`d.c.y` in `C2`).

We introduce the concept of //lazily copied// locations (regions) to the 
`SymbolReaper`, i.e. for which a program can access the value stored at that 
location, but not its address. These locations are constructed as a set of 
regions referred to by `lazyCompoundVal`. A //readable// location (region) is a 
location that //live// or //lazily copied// . And symbols that refer to values 
in regions are alive if the region is //readable//.

For simplicity, we follow the current approach to live regions and mark the 
base region as //lazily copied//, and consider any subregions as //readable//. 
This makes some symbols falsy live (`d.z` in our example) and keeps the 
corresponding constraints alive.

The rename `Regions` to `LiveRegions` inside  `RegionStore` is NFC change, that 
was done to make it clear, what is difference between regions stored in this 
two sets.

Regression Test: https://reviews.llvm.org/D134941
Co-authored-by: Balazs Benics 

Reviewed By: martong, xazax.hun

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
clang/test/Analysis/trivial-copy-struct.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index e46f97b7fafcd..b7ce6ebe98786 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -582,7 +582,12 @@ class SymbolReaper {
   SymbolMapTy TheLiving;
   SymbolSetTy MetadataInUse;
 
-  RegionSetTy RegionRoots;
+  RegionSetTy LiveRegionRoots;
+  // The lazily copied regions are locations for which a program
+  // can access the value stored at that location, but not its address.
+  // These regions are constructed as a set of regions referred to by
+  // lazyCompoundVal.
+  RegionSetTy LazilyCopiedRegionRoots;
 
   const StackFrameContext *LCtx;
   const Stmt *Loc;
@@ -628,8 +633,8 @@ class SymbolReaper {
 
   using region_iterator = RegionSetTy::const_iterator;
 
-  region_iterator region_begin() const { return RegionRoots.begin(); }
-  region_iterator region_end() const { return RegionRoots.end(); }
+  region_iterator region_begin() const { return LiveRegionRoots.begin(); }
+  region_iterator region_end() const { return LiveRegionRoots.end(); }
 
   /// Returns

[PATCH] D134947: [analyzer] Fix the liveness of Symbols for values in regions referred by LazyCompoundVal

[Tomasz Kamiński via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa6b42040ad30: [analyzer] Fix the liveness of Symbols for 
values in regions referred by… (authored by tomasz-kaminski-sonarsource).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134947

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
  clang/test/Analysis/trivial-copy-struct.cpp

Index: clang/test/Analysis/trivial-copy-struct.cpp
===
--- clang/test/Analysis/trivial-copy-struct.cpp
+++ clang/test/Analysis/trivial-copy-struct.cpp
@@ -49,11 +49,53 @@
   if (c.value == 42)
 return;
   clang_analyzer_value(c.value);
-  // expected-warning@-1 {{32s:{ [-2147483648, 2147483647] }}}
-  // The symbol was garbage collected too early, hence we lose the constraints.
+  // expected-warning@-1 {{32s:{ [-2147483648, 41], [43, 2147483647] }}}
+  // Before symbol was garbage collected too early, and we lost the constraints.
   if (c.value != 42)
 return;
 
-  // Dead code should be unreachable
-  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  clang_analyzer_warnIfReached(); // no-warning: Dead code.
+};
+
+void ptr1(List* n) {
+  List* n2 = new List(*n); // ctor
+  if (!n->next) {
+if (n2->next) {
+  clang_analyzer_warnIfReached(); // unreachable
+}
+  }
+  delete n2;
+}
+
+void ptr2(List* n) {
+  List* n2 = new List(); // ctor
+  *n2 = *n; // assignment
+  if (!n->next) {
+if (n2->next) {
+  clang_analyzer_warnIfReached(); // unreachable
+}
+  }
+  delete n2;
+}
+
+struct Wrapper {
+  List head;
+  int count;
+};
+
+void nestedLazyCompoundVal(List* n) {
+  Wrapper* w = 0;
+  {
+ Wrapper lw;
+ lw.head = *n;
+ w = new Wrapper(lw);
+  }
+  if (!n->next) {
+if (w->head.next) {
+  // FIXME: Unreachable, w->head is a copy of *n, therefore
+  // w->head.next and n->next are equal
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+  }
+  delete w;
 }
Index: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -411,10 +411,14 @@
 }
 
 void SymbolReaper::markLive(const MemRegion *region) {
-  RegionRoots.insert(region->getBaseRegion());
+  LiveRegionRoots.insert(region->getBaseRegion());
   markElementIndicesLive(region);
 }
 
+void SymbolReaper::markLazilyCopied(const clang::ento::MemRegion *region) {
+  LazilyCopiedRegionRoots.insert(region->getBaseRegion());
+}
+
 void SymbolReaper::markElementIndicesLive(const MemRegion *region) {
   for (auto SR = dyn_cast(region); SR;
SR = dyn_cast(SR->getSuperRegion())) {
@@ -437,8 +441,7 @@
   // is not used later in the path, we can diagnose a leak of a value within
   // that field earlier than, say, the variable that contains the field dies.
   MR = MR->getBaseRegion();
-
-  if (RegionRoots.count(MR))
+  if (LiveRegionRoots.count(MR))
 return true;
 
   if (const auto *SR = dyn_cast(MR))
@@ -454,6 +457,15 @@
   return isa(MR);
 }
 
+bool SymbolReaper::isLazilyCopiedRegion(const MemRegion *MR) const {
+  // TODO: See comment in isLiveRegion.
+  return LazilyCopiedRegionRoots.count(MR->getBaseRegion());
+}
+
+bool SymbolReaper::isReadableRegion(const MemRegion *MR) {
+  return isLiveRegion(MR) || isLazilyCopiedRegion(MR);
+}
+
 bool SymbolReaper::isLive(SymbolRef sym) {
   if (TheLiving.count(sym)) {
 markDependentsLive(sym);
@@ -464,7 +476,7 @@
 
   switch (sym->getKind()) {
   case SymExpr::SymbolRegionValueKind:
-KnownLive = isLiveRegion(cast(sym)->getRegion());
+KnownLive = isReadableRegion(cast(sym)->getRegion());
 break;
   case SymExpr::SymbolConjuredKind:
 KnownLive = false;
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2838,6 +2838,10 @@
   // Is it a LazyCompoundVal?  All referenced regions are live as well.
   if (Optional LCS =
   V.getAs()) {
+// TODO: Make regions referred to by `lazyCompoundVals` that are bound to
+// subregions of the `LCS.getRegion()` also lazily copied.
+if (const MemRegion *R = LCS->getRegion())
+  SymReaper.markLazilyCopied(R);
 
 const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
 
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Sy

[clang] 6194229 - [analyzer] Make directly bounded LazyCompoundVal as lazily copied

[Tomasz Kamiński via cfe-commits]

Author: Tomasz Kamiński
Date: 2022-10-19T16:06:32+02:00
New Revision: 6194229c6287fa5d8a9a30aa489bcbb8a9754ae0

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

LOG: [analyzer] Make directly bounded LazyCompoundVal as lazily copied

Previously, `LazyCompoundVal` bindings to subregions referred by
`LazyCopoundVals`, were not marked as //lazily copied//.

This change returns `LazyCompoundVals` from `getInterestingValues()`,
so their regions can be marked as //lazily copied// in 
`RemoveDeadBindingsWorker::VisitBinding()`.

Depends on D134947

Authored by: Tomasz Kamiński 

Reviewed By: martong

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/trivial-copy-struct.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp 
b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 7e4e00454e96..8af351f64246 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -608,6 +608,10 @@ class RegionStoreManager : public StoreManager {
   /// The precise value of "interesting" is determined for the purposes of
   /// RegionStore's internal analysis. It must always contain all regions and
   /// symbols, but may omit constants and other kinds of SVal.
+  ///
+  /// In contrast to compound values, LazyCompoundVals are also added
+  /// to the 'interesting values' list in addition to the child interesting
+  /// values.
   const SValListTy &getInterestingValues(nonloc::LazyCompoundVal LCV);
 
   //===--===//
@@ -1032,12 +1036,11 @@ void InvalidateRegionsWorker::VisitBinding(SVal V) {
   if (Optional LCS =
   V.getAs()) {
 
-const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
-
-for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(),
-E = Vals.end();
- I != E; ++I)
-  VisitBinding(*I);
+// `getInterestingValues()` returns SVals contained within 
LazyCompoundVals,
+// so there is no need to visit them.
+for (SVal V : RM.getInterestingValues(*LCS))
+  if (!isa(V))
+VisitBinding(V);
 
 return;
   }
@@ -1290,15 +1293,10 @@ void 
RegionStoreManager::populateWorkList(InvalidateRegionsWorker &W,
 if (Optional LCS =
 V.getAs()) {
 
-  const SValListTy &Vals = getInterestingValues(*LCS);
-
-  for (SValListTy::const_iterator I = Vals.begin(),
-  E = Vals.end(); I != E; ++I) {
-// Note: the last argument is false here because these are
-// non-top-level regions.
-if (const MemRegion *R = (*I).getAsRegion())
+  for (SVal S : getInterestingValues(*LCS))
+if (const MemRegion *R = S.getAsRegion())
   W.AddToWorkList(R);
-  }
+
   continue;
 }
 
@@ -2283,11 +2281,9 @@ 
RegionStoreManager::getInterestingValues(nonloc::LazyCompoundVal LCV) {
 if (V.isUnknownOrUndef() || V.isConstant())
   continue;
 
-if (Optional InnerLCV =
-V.getAs()) {
+if (auto InnerLCV = V.getAs()) {
   const SValListTy &InnerList = getInterestingValues(*InnerLCV);
   List.insert(List.end(), InnerList.begin(), InnerList.end());
-  continue;
 }
 
 List.push_back(V);
@@ -2835,20 +2831,17 @@ void RemoveDeadBindingsWorker::VisitCluster(const 
MemRegion *baseR,
 }
 
 void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
-  // Is it a LazyCompoundVal?  All referenced regions are live as well.
-  if (Optional LCS =
-  V.getAs()) {
-// TODO: Make regions referred to by `lazyCompoundVals` that are bound to
-// subregions of the `LCS.getRegion()` also lazily copied.
-if (const MemRegion *R = LCS->getRegion())
-  SymReaper.markLazilyCopied(R);
-
-const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
-
-for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(),
-E = Vals.end();
- I != E; ++I)
-  VisitBinding(*I);
+  // Is it a LazyCompoundVal? All referenced regions are live as well.
+  // The LazyCompoundVal itself is not live but should be readable.
+  if (auto LCS = V.getAs()) {
+SymReaper.markLazilyCopied(LCS->getRegion());
+
+for (SVal V : RM.getInterestingValues(*LCS)) {
+  if (auto DepLCS = V.getAs())
+SymReaper.markLazilyCopied(DepLCS->getRegion());
+  else
+VisitBinding(V);
+}
 
 return;
   }

diff  --git a/clang/test/Analysis/trivial-copy-struct.cpp 
b/clang/test/Analysis/trivial-copy-struct.cpp
index b1f4cb3fbfdb..b2da777541ab 100644
-

[PATCH] D135136: [analyzer] Make directly bounded LazyCompoundVal as lazily copied

[Tomasz Kamiński via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6194229c6287: [analyzer] Make directly bounded 
LazyCompoundVal as lazily copied (authored by tomasz-kaminski-sonarsource).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135136

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/trivial-copy-struct.cpp

Index: clang/test/Analysis/trivial-copy-struct.cpp
===
--- clang/test/Analysis/trivial-copy-struct.cpp
+++ clang/test/Analysis/trivial-copy-struct.cpp
@@ -92,9 +92,9 @@
   }
   if (!n->next) {
 if (w->head.next) {
-  // FIXME: Unreachable, w->head is a copy of *n, therefore
+  // Unreachable, w->head is a copy of *n, therefore
   // w->head.next and n->next are equal
-  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  clang_analyzer_warnIfReached(); // no-warning: unreachable
 }
   }
   delete w;
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -608,6 +608,10 @@
   /// The precise value of "interesting" is determined for the purposes of
   /// RegionStore's internal analysis. It must always contain all regions and
   /// symbols, but may omit constants and other kinds of SVal.
+  ///
+  /// In contrast to compound values, LazyCompoundVals are also added
+  /// to the 'interesting values' list in addition to the child interesting
+  /// values.
   const SValListTy &getInterestingValues(nonloc::LazyCompoundVal LCV);
 
   //===--===//
@@ -1032,12 +1036,11 @@
   if (Optional LCS =
   V.getAs()) {
 
-const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
-
-for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(),
-E = Vals.end();
- I != E; ++I)
-  VisitBinding(*I);
+// `getInterestingValues()` returns SVals contained within LazyCompoundVals,
+// so there is no need to visit them.
+for (SVal V : RM.getInterestingValues(*LCS))
+  if (!isa(V))
+VisitBinding(V);
 
 return;
   }
@@ -1290,15 +1293,10 @@
 if (Optional LCS =
 V.getAs()) {
 
-  const SValListTy &Vals = getInterestingValues(*LCS);
-
-  for (SValListTy::const_iterator I = Vals.begin(),
-  E = Vals.end(); I != E; ++I) {
-// Note: the last argument is false here because these are
-// non-top-level regions.
-if (const MemRegion *R = (*I).getAsRegion())
+  for (SVal S : getInterestingValues(*LCS))
+if (const MemRegion *R = S.getAsRegion())
   W.AddToWorkList(R);
-  }
+
   continue;
 }
 
@@ -2283,11 +2281,9 @@
 if (V.isUnknownOrUndef() || V.isConstant())
   continue;
 
-if (Optional InnerLCV =
-V.getAs()) {
+if (auto InnerLCV = V.getAs()) {
   const SValListTy &InnerList = getInterestingValues(*InnerLCV);
   List.insert(List.end(), InnerList.begin(), InnerList.end());
-  continue;
 }
 
 List.push_back(V);
@@ -2835,20 +2831,17 @@
 }
 
 void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
-  // Is it a LazyCompoundVal?  All referenced regions are live as well.
-  if (Optional LCS =
-  V.getAs()) {
-// TODO: Make regions referred to by `lazyCompoundVals` that are bound to
-// subregions of the `LCS.getRegion()` also lazily copied.
-if (const MemRegion *R = LCS->getRegion())
-  SymReaper.markLazilyCopied(R);
-
-const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
-
-for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(),
-E = Vals.end();
- I != E; ++I)
-  VisitBinding(*I);
+  // Is it a LazyCompoundVal? All referenced regions are live as well.
+  // The LazyCompoundVal itself is not live but should be readable.
+  if (auto LCS = V.getAs()) {
+SymReaper.markLazilyCopied(LCS->getRegion());
+
+for (SVal V : RM.getInterestingValues(*LCS)) {
+  if (auto DepLCS = V.getAs())
+SymReaper.markLazilyCopied(DepLCS->getRegion());
+  else
+VisitBinding(V);
+}
 
 return;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135513: [clang][Interp] Check instance pointers before calling functions on them

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

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

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

https://reviews.llvm.org/D135513

Files:
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -156,11 +156,14 @@
 
   constexpr int foo() { // ref-error {{never produces a constant expression}}
 S *s = nullptr;
-return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+return s->get12(); // ref-note 2{{member call on dereferenced null pointer}} \
+   // expected-note {{member call on dereferenced null pointer}}
+
   }
-  // FIXME: The new interpreter doesn't reject this currently.
   static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
-  // ref-note {{in call to 'foo()'}}
+  // ref-note {{in call to 'foo()'}} \
+  // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'foo()'}}
 };
 
 struct FourBoolPairs {
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -168,7 +168,6 @@
   let Args = [ArgFunction];
   let Types = [AllTypeClass];
   let ChangesPC = 1;
-  let HasCustomEval = 1;
   let HasGroup = 1;
 }
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -114,6 +114,9 @@
 
 template  inline bool IsTrue(const T &V) { return !V.isZero(); }
 
+/// Interpreter entry point.
+bool Interpret(InterpState &S, APValue &Result);
+
 //===--===//
 // Add, Sub, Mul
 //===--===//
@@ -1108,6 +,35 @@
   return true;
 }
 
+template ::T>
+static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
+  auto NewFrame = std::make_unique(S, Func, PC);
+  if (Func->hasThisPointer()) {
+if (!CheckInvoke(S, PC, NewFrame->getThis())) {
+  return false;
+}
+// TODO: CheckCallable
+  }
+
+  InterpFrame *FrameBefore = S.Current;
+  S.Current = NewFrame.get();
+
+  APValue CallResult;
+  // Note that we cannot assert(CallResult.hasValue()) here since
+  // Ret() above only sets the APValue if the curent frame doesn't
+  // have a caller set.
+  if (Interpret(S, CallResult)) {
+NewFrame.release(); // Frame was delete'd already.
+assert(S.Current == FrameBefore);
+return true;
+  }
+
+  // Interpreting the function failed somehow. Reset to
+  // previous state.
+  S.Current = FrameBefore;
+  return false;
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
@@ -1121,9 +1153,6 @@
   }
 }
 
-/// Interpreter entry point.
-bool Interpret(InterpState &S, APValue &Result);
-
 } // namespace interp
 } // namespace clang
 
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -53,16 +53,6 @@
   return true;
 }
 
-template ::T>
-static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
-  S.Current = new InterpFrame(S, const_cast(Func), PC);
-  APValue CallResult;
-  // Note that we cannot assert(CallResult.hasValue()) here since
-  // Ret() above only sets the APValue if the curent frame doesn't
-  // have a caller set.
-  return Interpret(S, CallResult);
-}
-
 static bool CallVoid(InterpState &S, CodePtr &PC, const Function *Func) {
   APValue VoidResult;
   S.Current = new InterpFrame(S, const_cast(Func), PC);
Index: clang/lib/AST/Interp/Function.cpp
===
--- clang/lib/AST/Interp/Function.cpp
+++ clang/lib/AST/Interp/Function.cpp
@@ -30,11 +30,12 @@
 }
 
 SourceInfo Function::getSource(CodePtr PC) const {
+  assert(PC >= getCodeBegin() && "PC does not belong to this function");
+  assert(PC <= getCodeEnd() && "PC Does not belong to this function");
   unsigned Offset = PC - getCodeBegin();
   using Elem = std::pair;
   auto It = llvm::lower_bound(SrcMap, Elem{Offset, {}}, llvm::less_first());
-  if (It == SrcMap.end() || It->first != Offset)
-llvm::report_fatal_error("missing source location");
+  assert(It != SrcMap.end());
   return 

[PATCH] D132329: [X86][RFC] Using `__bf16` for AVX512_BF16 intrinsics

[LuoYuanke via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132329

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton updated this revision to Diff 468907.
hel-ableton added a comment.

Fixed missing newline.


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

https://reviews.llvm.org/D136154

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6594,6 +6594,25 @@
"(someOtherLongishConditionPart1 || "
"someOtherEvenLongerNestedConditionPart2);",
Style));
+
+  Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  verifyFormat(
+  "struct Derived {\n"
+  "  Derived(\n"
+  "int firstArgWithLongName,\n"
+  "int secondArgWithLongName,\n"
+  "int thirdArgWithLongName,\n"
+  "int fourthArgWithLongName)\n"
+  "  : Base(\n"
+  "  firstArgWithLongName,\n"
+  "  secondArgWithLongName,\n"
+  "  thirdArgWithLongName,\n"
+  "  fourthArgWithLongName) {}\n"
+  "};", Style);
 }
 
 TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -800,8 +800,9 @@
  FormatStyle::BCIS_AfterColon) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;
-  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_CtorInitializerColon)) &&
+} else if (Previous.is(TT_CtorInitializerColon)) {
+  CurrentState.LastSpace = State.Column;
+} else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
  ((Previous.getPrecedence() != prec::Assignment &&
(Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 ||
 Previous.NextOperator)) ||
@@ -809,8 +810,10 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;
+}
   } else if (Previous.is(TT_InheritanceColon)) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6594,6 +6594,25 @@
"(someOtherLongishConditionPart1 || "
"someOtherEvenLongerNestedConditionPart2);",
Style));
+
+  Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  verifyFormat(
+  "struct Derived {\n"
+  "  Derived(\n"
+  "int firstArgWithLongName,\n"
+  "int secondArgWithLongName,\n"
+  "int thirdArgWithLongName,\n"
+  "int fourthArgWithLongName)\n"
+  "  : Base(\n"
+  "  firstArgWithLongName,\n"
+  "  secondArgWithLongName,\n"
+  "  thirdArgWithLongName,\n"
+  "  fourthArgWithLongName) {}\n"
+  "};", Style);
 }
 
 TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -800,8 +800,9 @@
  FormatStyle::BCIS_AfterColon) {
 CurrentState.Indent = State.Column;
 CurrentState.LastSpace = State.Column;
-  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_CtorInitializerColon)) &&
+} else if (Previous.is(TT_CtorInitializerColon)) {
+  CurrentState.LastSpace = State.Column;
+} else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
  ((Previous.getPrecedence() != prec::Assignment &&
(Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 ||
 Previous.NextOperator)) ||
@@ -809,8 +810,10 @@
 // Indent relative to the RHS of the expre

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Henrik Lafrenz via Phabricator via cfe-commits]

hel-ableton marked an inline comment as done.
hel-ableton added a comment.

> I do, but I'd like to hear @owenpan and @HazardyKnusperkeks view.. (each of 
> us is better in different part of clang-format, and I value their opinion!)

Fair enough.


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

https://reviews.llvm.org/D136154

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

[PATCH] D132329: [X86][RFC] Using `__bf16` for AVX512_BF16 intrinsics

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Add a short description to clang ReleaseNotes about the new bf/bh types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132329

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

[PATCH] D135429: [HLSL] [DirectX backend] Move generateGlobalCtorDtorCalls into DirectX backend.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In addition to restoring the test cases that you deleted, I think you should 
also reduce and simplify the test cases you're adding. These new tests are very 
large blobs of IR generated by clang. They include a lot of extra instructions 
that aren't needed to exercise the code you wrote in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135429

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

[PATCH] D136259: Fix crash in constraining partial specialization on nested template.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136259

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5804,10 +5804,10 @@
   }
 
   bool TraverseTemplateName(TemplateName Template) {
-if (auto *TTP =
-dyn_cast(Template.getAsTemplateDecl()))
-  if (TTP->getDepth() == Depth)
-Used[TTP->getIndex()] = true;
+if (TemplateDecl *TD = Template.getAsTemplateDecl())
+  if (auto *TTP = dyn_cast(TD))
+if (TTP->getDepth() == Depth)
+  Used[TTP->getIndex()] = true;
 RecursiveASTVisitor::
 TraverseTemplateName(Template);
 return true;


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5804,10 +5804,10 @@
   }
 
   bool TraverseTemplateName(TemplateName Template) {
-if (auto *TTP =
-dyn_cast(Template.getAsTemplateDecl()))
-  if (TTP->getDepth() == Depth)
-Used[TTP->getIndex()] = true;
+if (TemplateDecl *TD = Template.getAsTemplateDecl())
+  if (auto *TTP = dyn_cast(TD))
+if (TTP->getDepth() == Depth)
+  Used[TTP->getIndex()] = true;
 RecursiveASTVisitor::
 TraverseTemplateName(Template);
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136187: [clang][AIX] Omitting Explicit Debugger Tuning Option

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 updated this revision to Diff 468914.
qiongsiwu1 added a comment.

Address the review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136187

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -118,8 +118,12 @@
 
 // On the AIX, -g defaults to -gdbx and limited debug info.
 // RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff 2>&1 \
-// RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
+// RUN: | FileCheck -check-prefix=G_LIMITED %s
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=G_LIMITED %s
+// RUN: %clang -### -c -g -gdbx %s -target powerpc-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
+// RUN: %clang -### -c -g -gdbx %s -target powerpc64-ibm-aix-xcoff 2>&1 \
 // RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
 
 // For DBX, -g defaults to -gstrict-dwarf.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4187,8 +4187,10 @@
   }
 
   // If a debugger tuning argument appeared, remember it.
+  bool HasDebuggerTuning = false;
   if (const Arg *A =
   Args.getLastArg(options::OPT_gTune_Group, options::OPT_ggdbN_Group)) 
{
+HasDebuggerTuning = true;
 if (checkDebugInfoOption(A, Args, D, TC)) {
   if (A->getOption().matches(options::OPT_glldb))
 DebuggerTuning = llvm::DebuggerKind::LLDB;
@@ -4364,8 +4366,12 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // On AIX, the debugger tuning option can be omitted if it is not explicitly
+  // set.
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, EffectiveDWARFVersion,
-  DebuggerTuning);
+  T.isOSAIX() && !HasDebuggerTuning
+  ? llvm::DebuggerKind::Default
+  : DebuggerTuning);
 
   // -fdebug-macro turns on macro debug info generation.
   if (Args.hasFlag(options::OPT_fdebug_macro, options::OPT_fno_debug_macro,


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -118,8 +118,12 @@
 
 // On the AIX, -g defaults to -gdbx and limited debug info.
 // RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff 2>&1 \
-// RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
+// RUN: | FileCheck -check-prefix=G_LIMITED %s
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=G_LIMITED %s
+// RUN: %clang -### -c -g -gdbx %s -target powerpc-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
+// RUN: %clang -### -c -g -gdbx %s -target powerpc64-ibm-aix-xcoff 2>&1 \
 // RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
 
 // For DBX, -g defaults to -gstrict-dwarf.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4187,8 +4187,10 @@
   }
 
   // If a debugger tuning argument appeared, remember it.
+  bool HasDebuggerTuning = false;
   if (const Arg *A =
   Args.getLastArg(options::OPT_gTune_Group, options::OPT_ggdbN_Group)) {
+HasDebuggerTuning = true;
 if (checkDebugInfoOption(A, Args, D, TC)) {
   if (A->getOption().matches(options::OPT_glldb))
 DebuggerTuning = llvm::DebuggerKind::LLDB;
@@ -4364,8 +4366,12 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // On AIX, the debugger tuning option can be omitted if it is not explicitly
+  // set.
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, EffectiveDWARFVersion,
-  DebuggerTuning);
+  T.isOSAIX() && !HasDebuggerTuning
+  ? llvm::DebuggerKind::Default
+  : DebuggerTuning);
 
   // -fdebug-macro turns on macro debug info generation.
   if (Args.hasFlag(options::OPT_fdebug_macro, options::OPT_fno_debug_macro,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135429: [HLSL] [DirectX backend] Move generateGlobalCtorDtorCalls into DirectX backend.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:193
-
-void CGHLSLRuntime::generateGlobalCtorDtorCalls() {
-  llvm::Module &M = CGM.getModule();

I think you've been a bit too aggressive about what code you're moving. The 
code that generates the global constructor calls should not be removed.

We should generate correct IR from clang, and part of correct IR for HLSL is 
that the global constructors are called from the entries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135429

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:814
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
+Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) {
   CurrentState.LastSpace = State.Column;

ok so now you don't actually have a test for this, so I don't know if this is 
needed any more.



Comment at: clang/unittests/Format/FormatTest.cpp:6602
+  Style.ContinuationIndentWidth = 2;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  verifyFormat(

I don't think you need this line


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

https://reviews.llvm.org/D136154

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Looking at the documentation, I think BreakBeforeBinaryOperators  is unrelated 
to your test case. I would remove any references of it from your fix.

F24968180: image.png 


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

https://reviews.llvm.org/D136154

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

[PATCH] D132329: [X86][RFC] Using `__bf16` for AVX512_BF16 intrinsics

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon accepted this revision.
RKSimon added a comment.

LGTM -  cheers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132329

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

[PATCH] D135657: add time traces for AST serialization

[Trass3r via Phabricator via cfe-commits]

Trass3r added a comment.

`Andreas Hollandt `


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135657

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Going back to the original bug that caused this.. if you leave in your 
BOS_NonAssignment I think you'll reintroduce the original bug..  which possibly 
should have been called

"incorrectly adds extra continuation indent spaces with 
BreakBeforeBinaryOperators set to All *(and NonAssignement)*"

i.e.

  bool BreakBeforeBinaryOperators(
  bool someVeryVeryLongConditionThatBarelyFitsOnALine,
  bool someOtherLongishConditionPart1,
  bool someOtherEvenLongerNestedConditionPart2) {
  return someVeryVeryLongConditionThatBarelyFitsOnALine
  && (someOtherLongishConditionPart1
  || someOtherEvenLongerNestedConditionPart2);
  }

will bcome

  bool BreakBeforeBinaryOperators(
  bool someVeryVeryLongConditionThatBarelyFitsOnALine,
  bool someOtherLongishConditionPart1,
  bool someOtherEvenLongerNestedConditionPart2) {
  return someVeryVeryLongConditionThatBarelyFitsOnALine
  && (someOtherLongishConditionPart1
 || someOtherEvenLongerNestedConditionPart2);
   
  }

 


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

https://reviews.llvm.org/D136154

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

[PATCH] D136259: Fix crash in constraining partial specialization on nested template.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 468925.
usaxena95 added a comment.

Added test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136259

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaTemplate/concepts-GH53354.cpp


Index: clang/test/SemaTemplate/concepts-GH53354.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/concepts-GH53354.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+template  class>
+struct S
+{};
+
+template 
+concept C1 = requires
+{
+  typename S;
+};
+
+template 
+requires C1
+struct A {};
+
+template 
+requires C1 and true
+struct A {};
\ No newline at end of file
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5804,10 +5804,10 @@
   }
 
   bool TraverseTemplateName(TemplateName Template) {
-if (auto *TTP =
-dyn_cast(Template.getAsTemplateDecl()))
-  if (TTP->getDepth() == Depth)
-Used[TTP->getIndex()] = true;
+if (TemplateDecl *TD = Template.getAsTemplateDecl())
+  if (auto *TTP = dyn_cast(TD))
+if (TTP->getDepth() == Depth)
+  Used[TTP->getIndex()] = true;
 RecursiveASTVisitor::
 TraverseTemplateName(Template);
 return true;


Index: clang/test/SemaTemplate/concepts-GH53354.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/concepts-GH53354.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+template  class>
+struct S
+{};
+
+template 
+concept C1 = requires
+{
+  typename S;
+};
+
+template 
+requires C1
+struct A {};
+
+template 
+requires C1 and true
+struct A {};
\ No newline at end of file
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5804,10 +5804,10 @@
   }
 
   bool TraverseTemplateName(TemplateName Template) {
-if (auto *TTP =
-dyn_cast(Template.getAsTemplateDecl()))
-  if (TTP->getDepth() == Depth)
-Used[TTP->getIndex()] = true;
+if (TemplateDecl *TD = Template.getAsTemplateDecl())
+  if (auto *TTP = dyn_cast(TD))
+if (TTP->getDepth() == Depth)
+  Used[TTP->getIndex()] = true;
 RecursiveASTVisitor::
 TraverseTemplateName(Template);
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.

[Zhiyao Ma via Phabricator via cfe-commits]

ZhiyaoMa98 updated this revision to Diff 468928.
ZhiyaoMa98 edited the summary of this revision.

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

https://reviews.llvm.org/D136203

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-execute-only.c
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll

Index: llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=thumbv7em-arm-none-eabi -relocation-model=static | FileCheck %s -check-prefixes=CHECK,STATIC
+; RUN: llc < %s -mtriple=thumbv7em-arm-none-eabi -relocation-model=rwpi | FileCheck %s -check-prefixes=CHECK,RWPI
+
+define void @fn() #0 {
+entry:
+; CHECK-LABEL: fn:
+; CHECK:   ldr [[REG:r[0-9]+]], .LCPI0_0
+; CHECK-NEXT:  blx [[REG]]
+; CHECK:   .LCPI0_0:
+; CHECK-NEXT:  .long   bar
+  call void @bar()
+  ret void
+}
+
+define void @execute_only_fn() #1 {
+; STATIC-LABEL: execute_only_fn:
+; STATIC:   movw[[REG0:r[0-9]+]], :lower16:bar
+; STATIC-NEXT:  movt[[REG0]], :upper16:bar
+; STATIC-NEXT:  blx [[REG0]]
+; STATIC-NOT:   .LCPI0_0:
+
+; RWPI-LABEL: execute_only_fn:
+; RWPI:   movw[[REG0:r[0-9]+]], :lower16:bar
+; RWPI-NEXT:  movt[[REG0]], :upper16:bar
+; RWPI-NEXT:  blx [[REG0]]
+; RWPI-NOT:   .LCPI0_0:
+entry:
+  call void @bar()
+  ret void
+}
+
+attributes #0 = { noinline optnone "target-features"="+thumb-mode,+long-calls" }
+attributes #1 = { noinline optnone "target-features"="+execute-only,+thumb-mode,+long-calls" }
+
+declare dso_local void @bar()
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -2630,11 +2630,11 @@
 
   const TargetMachine &TM = getTargetMachine();
   const Module *Mod = MF.getFunction().getParent();
-  const GlobalValue *GV = nullptr;
+  const GlobalValue *GVal = nullptr;
   if (GlobalAddressSDNode *G = dyn_cast(Callee))
-GV = G->getGlobal();
+GVal = G->getGlobal();
   bool isStub =
-  !TM.shouldAssumeDSOLocal(*Mod, GV) && Subtarget->isTargetMachO();
+  !TM.shouldAssumeDSOLocal(*Mod, GVal) && Subtarget->isTargetMachO();
 
   bool isARMFunc = !Subtarget->isThumb() || (isStub && !Subtarget->isMClass());
   bool isLocalARMFunc = false;
@@ -2647,36 +2647,59 @@
 // those, the target's already in a register, so we don't need to do
 // anything extra.
 if (isa(Callee)) {
-  // Create a constant pool entry for the callee address
-  unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
-  ARMConstantPoolValue *CPV =
-ARMConstantPoolConstant::Create(GV, ARMPCLabelIndex, ARMCP::CPValue, 0);
-
-  // Get the address of the callee into a register
-  SDValue CPAddr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
-  CPAddr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, CPAddr);
-  Callee = DAG.getLoad(
-  PtrVt, dl, DAG.getEntryNode(), CPAddr,
-  MachinePointerInfo::getConstantPool(DAG.getMachineFunction()));
+  SDValue Addr;
+
+  // When generating execute-only code we use movw movt pair.
+  // Currently execute-only is only available for architectures that
+  // support movw movt, so we are safe to assume that.
+  if (Subtarget->genExecuteOnly()) {
+SDValue GA = DAG.getTargetGlobalAddress(GVal, dl, PtrVt);
+++NumMovwMovt;
+Callee = DAG.getNode(ARMISD::Wrapper, dl, PtrVt,
+ DAG.getTargetGlobalAddress(GVal, dl, PtrVt));
+  } else {
+// Create a constant pool entry for the callee address
+unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
+ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create(
+GVal, ARMPCLabelIndex, ARMCP::CPValue, 0);
+
+// Get the address of the callee into a register
+Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
+Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr);
+Callee = DAG.getLoad(
+PtrVt, dl, DAG.getEntryNode(), Addr,
+MachinePointerInfo::getConstantPool(DAG.getMachineFunction()));
+  }
 } else if (ExternalSymbolSDNode *S=dyn_cast(Callee)) {
   const char *Sym = S->getSymbol();
-
-  // Create a constant pool entry for the callee address
-  unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
-  ARMConstantPoolValue *CPV =
-ARMConstantPoolSymbol::Create(*DAG.getContext(), Sym,
-  ARMPCLabelIndex, 0);
-  // Get the address of the callee into a register
-  SDValue CPAddr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
-  CPAddr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, CPAddr);
-  Callee = DAG

[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.

[Zhiyao Ma via Phabricator via cfe-commits]

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

I have updated the diff to avoid the extra indirection. I am thinking about 
adding a new option, say `-mgot-calls` to allow code generation with the extra 
indirection. Is it sensible and shall I create another diff to discuss that?


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

https://reviews.llvm.org/D136203

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

[PATCH] D136259: Fix crash in constraining partial specialization on nested template.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5807
   bool TraverseTemplateName(TemplateName Template) {
-if (auto *TTP =
-dyn_cast(Template.getAsTemplateDecl()))
-  if (TTP->getDepth() == Depth)
-Used[TTP->getIndex()] = true;
+if (TemplateDecl *TD = Template.getAsTemplateDecl())
+  if (auto *TTP = dyn_cast(TD))

NIT: maybe keep a single if statement and use `dyn_cast_or_null`?



Comment at: clang/test/SemaTemplate/concepts-GH53354.cpp:19
+template 
+requires C1 and true
+struct A {};

why not `&& true`!?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136259

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

[clang] d146a52 - Move HLSL builtins into hlsl namespace

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-10-19T10:59:31-05:00
New Revision: d146a5241c5039fd25e8bf5a4ae1200ad201d06c

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

LOG: Move HLSL builtins into hlsl namespace

Should have done this from the start. Since all the injected AST types
are in the hlsl namespace we should also put the header-defined types
and functions in there too.

This updates the basic_types test to run once with the namespaced types
and once without, and adds using declarations or namespaces calls in
other tests.

Reviewed By: python3kgae

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

Added: 


Modified: 
clang/lib/Headers/hlsl/hlsl_basic_types.h
clang/lib/Headers/hlsl/hlsl_intrinsics.h
clang/test/CodeGenHLSL/basic_types.hlsl
clang/test/CodeGenHLSL/builtins/abs.hlsl
clang/test/CodeGenHLSL/builtins/ceil.hlsl
clang/test/CodeGenHLSL/builtins/sqrt.hlsl
clang/test/SemaHLSL/Wave.hlsl
clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl

Removed: 




diff  --git a/clang/lib/Headers/hlsl/hlsl_basic_types.h 
b/clang/lib/Headers/hlsl/hlsl_basic_types.h
index e68715f1a6a45..9ea605cfa840a 100644
--- a/clang/lib/Headers/hlsl/hlsl_basic_types.h
+++ b/clang/lib/Headers/hlsl/hlsl_basic_types.h
@@ -9,6 +9,7 @@
 #ifndef _HLSL_HLSL_BASIC_TYPES_H_
 #define _HLSL_HLSL_BASIC_TYPES_H_
 
+namespace hlsl {
 // built-in scalar data types:
 
 #ifdef __HLSL_ENABLE_16_BIT
@@ -61,4 +62,6 @@ typedef vector double2;
 typedef vector double3;
 typedef vector double4;
 
+} // namespace hlsl
+
 #endif //_HLSL_HLSL_BASIC_TYPES_H_

diff  --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h 
b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index 43f8dfee7a5f1..e0099d435308b 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,8 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+namespace hlsl {
+
 __attribute__((availability(shadermodel, introduced = 6.0)))
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) 
uint
 WaveActiveCountBits(bool bBit);
@@ -99,4 +101,6 @@ double3 ceil(double3);
 __attribute__((clang_builtin_alias(__builtin_elementwise_ceil)))
 double4 ceil(double4);
 
+} // namespace hlsl
+
 #endif //_HLSL_HLSL_INTRINSICS_H_

diff  --git a/clang/test/CodeGenHLSL/basic_types.hlsl 
b/clang/test/CodeGenHLSL/basic_types.hlsl
index d26d94910c2d8..15c963dfa666f 100644
--- a/clang/test/CodeGenHLSL/basic_types.hlsl
+++ b/clang/test/CodeGenHLSL/basic_types.hlsl
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
 // RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
 // RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -o - -DNAMESPACED| FileCheck %s
 
 
 // CHECK:"?uint16_t_Val@@3GA" = global i16 0, align 2
@@ -36,7 +39,11 @@
 // CHECK:"?double3_Val@@3T?$__vector@N$02@__clang@@A" = global <3 x double> 
zeroinitializer, align 32
 // CHECK:"?double4_Val@@3T?$__vector@N$03@__clang@@A" = global <4 x double> 
zeroinitializer, align 32
 
+#ifdef NAMESPACED
+#define TYPE_DECL(T)  hlsl::T T##_Val
+#else
 #define TYPE_DECL(T)  T T##_Val
+#endif
 
 #ifdef __HLSL_ENABLE_16_BIT
 TYPE_DECL(uint16_t);

diff  --git a/clang/test/CodeGenHLSL/builtins/abs.hlsl 
b/clang/test/CodeGenHLSL/builtins/abs.hlsl
index 30b0bcb8837ca..e014680ff2c1c 100644
--- a/clang/test/CodeGenHLSL/builtins/abs.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -5,6 +5,7 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::abs;
 
 // CHECK: define noundef signext i16 @
 // FIXME: int16_t is promoted to i32 now. Change to abs.i16 once it is fixed.

diff  --git a/clang/test/CodeGenHLSL/builtins/ceil.hlsl 
b/clang/test/CodeGenHLSL/builtins/ceil.hlsl
index 69f2b82730673..5969028d56d1a 100644
--- a/clang/test/CodeGenHLSL/builtins/ceil.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/ceil.hlsl
@@ -5,6 +5,8 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::ceil;
+
 // CHECK: define noundef half @
 // CHECK: call half @llvm.ceil.f16(
 // NO_HALF: define noundef float @"?test_ceil_half@@YA$halff@$halff@@Z"(

diff  --git a/clang/test/CodeGenHLSL/builtins/sqrt.hlsl 
b/clang/test/CodeGenHLSL/builtins/sqrt.hlsl
index c083ce1175e59..776269395a662 100644
--- a/clang/test/CodeGenHLSL/builtins/sqrt.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/sqrt.hlsl
@@

[PATCH] D135973: Move HLSL builtins into hlsl namespace

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd146a5241c50: Move HLSL builtins into hlsl namespace 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135973

Files:
  clang/lib/Headers/hlsl/hlsl_basic_types.h
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/test/CodeGenHLSL/basic_types.hlsl
  clang/test/CodeGenHLSL/builtins/abs.hlsl
  clang/test/CodeGenHLSL/builtins/ceil.hlsl
  clang/test/CodeGenHLSL/builtins/sqrt.hlsl
  clang/test/SemaHLSL/Wave.hlsl
  clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl

Index: clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
===
--- clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
+++ clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
@@ -5,5 +5,5 @@
 // expected-warning@#site {{'WaveActiveCountBits' is only available on HLSL ShaderModel 6.0 or newer}}
 // expected-note@hlsl/hlsl_intrinsics.h:* {{'WaveActiveCountBits' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
 // expected-note@#site {{enclose 'WaveActiveCountBits' in a __builtin_available check to silence this warning}}
-return WaveActiveCountBits(b); // #site
+return hlsl::WaveActiveCountBits(b); // #site
 }
Index: clang/test/SemaHLSL/Wave.hlsl
===
--- clang/test/SemaHLSL/Wave.hlsl
+++ clang/test/SemaHLSL/Wave.hlsl
@@ -4,5 +4,5 @@
 
 // expected-no-diagnostics
 unsigned foo(bool b) {
-return WaveActiveCountBits(b);
+return hlsl::WaveActiveCountBits(b);
 }
Index: clang/test/CodeGenHLSL/builtins/sqrt.hlsl
===
--- clang/test/CodeGenHLSL/builtins/sqrt.hlsl
+++ clang/test/CodeGenHLSL/builtins/sqrt.hlsl
@@ -5,6 +5,8 @@
 // RUN:   dxil-pc-shadermodel6.2-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::sqrt;
+
 double sqrt_d(double x)
 {
   return sqrt(x);
Index: clang/test/CodeGenHLSL/builtins/ceil.hlsl
===
--- clang/test/CodeGenHLSL/builtins/ceil.hlsl
+++ clang/test/CodeGenHLSL/builtins/ceil.hlsl
@@ -5,6 +5,8 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::ceil;
+
 // CHECK: define noundef half @
 // CHECK: call half @llvm.ceil.f16(
 // NO_HALF: define noundef float @"?test_ceil_half@@YA$halff@$halff@@Z"(
Index: clang/test/CodeGenHLSL/builtins/abs.hlsl
===
--- clang/test/CodeGenHLSL/builtins/abs.hlsl
+++ clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -5,6 +5,7 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::abs;
 
 // CHECK: define noundef signext i16 @
 // FIXME: int16_t is promoted to i32 now. Change to abs.i16 once it is fixed.
Index: clang/test/CodeGenHLSL/basic_types.hlsl
===
--- clang/test/CodeGenHLSL/basic_types.hlsl
+++ clang/test/CodeGenHLSL/basic_types.hlsl
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
 // RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
 // RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -o - -DNAMESPACED| FileCheck %s
 
 
 // CHECK:"?uint16_t_Val@@3GA" = global i16 0, align 2
@@ -36,7 +39,11 @@
 // CHECK:"?double3_Val@@3T?$__vector@N$02@__clang@@A" = global <3 x double> zeroinitializer, align 32
 // CHECK:"?double4_Val@@3T?$__vector@N$03@__clang@@A" = global <4 x double> zeroinitializer, align 32
 
+#ifdef NAMESPACED
+#define TYPE_DECL(T)  hlsl::T T##_Val
+#else
 #define TYPE_DECL(T)  T T##_Val
+#endif
 
 #ifdef __HLSL_ENABLE_16_BIT
 TYPE_DECL(uint16_t);
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,8 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+namespace hlsl {
+
 __attribute__((availability(shadermodel, introduced = 6.0)))
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
@@ -99,4 +101,6 @@
 __attribute__((clang_builtin_alias(__builtin_elementwise_ceil)))
 double4 ceil(double4);
 
+} // namespace hlsl
+
 #endif //_HLSL_HLSL_INTRINSICS_H_
Index: clang/li

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:54-62
+  explicit operator int8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int64_t() const { return toAPSInt().getExtValue(); }

tbaeder wrote:
> sepavloff wrote:
> > tbaeder wrote:
> > > sepavloff wrote:
> > > > Conversions to integers are bitcasts+truncation but the conversion to 
> > > > bool is different. What semantics have these operations?
> > > They are basically used for casting. I didn't mean to make the bool 
> > > implementation different, just seemed to make sense to me to do it this 
> > > way.
> > If they are used for casting, they need to preserve value, that is 
> > conversion float->int32 should transform 1.0->0x0001. The method 
> > `toAPSInt` uses bitcast, so it converts 1.0->0x3f80. 
> > `APFloat::convertToInteger` should be used to make value-preverving 
> > conversion but it requires knowledge of rounding mode.
> That's good to know, thanks.  I've already used `TowardZero` in 
> `Floating::toSemantics()`, but that was more a guess.
> 
> Is the rounding mode I'm supposed to use simply 
> `LangOptions::getDefaultRoundingMode()`?
Rounding mode is stored in AST node, a call to `Expr::getFPFeaturesInEffect` 
may be used to extract various FP features in the form of `FPOptions`. 
`LangOptions::getDefaultRoundingMode()` returns the default rounding mode, 
which may be changed using `#pragma STDC FENV_ROUND`.


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

https://reviews.llvm.org/D134859

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

[PATCH] D135513: [clang][Interp] Check instance pointers before calling functions on them

[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


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

https://reviews.llvm.org/D135513

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

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

HazardyKnusperkeks added a comment.

In D136154#3868036 , @hel-ableton 
wrote:

> In D136154#3867935 , 
> @MyDeveloperDay wrote:
>
>> You've dropped the test on your most recent updated
>
> Oh, that's why it appeared from the diff. Apologies again, I'm really 
> unfamiliar with your process. I guess it puzzles me why it's described in 
> https://llvm.org/docs/Phabricator.html#phabricator-request-review-web that I 
> should make any commits at all, when it's just diffs that I'm supposed to 
> submit. Anyway, I'll try to bring it back.

How you get your diff doesn't matter. I make the real commits (which I will 
commit, if approved), and then just do `git diff -U999 HEAD^1 > file`.

In D136154#3868070 , @hel-ableton 
wrote:

>> The problem as I see it was that the original bug, highly constrained the 
>> cases where "CurrentState.LastSpace = State.Column;" to one particular style 
>> (which if it happens to be your style great but not if its not.
>
> You mean the original bugfix was already unnecessarily constrained, and now 
> my proposed fix is only opening it up for one my case? That may be so. In any 
> case this might really not be the ideal fix, and I'm open to any other, 
> better way of fixing it.

I too think one should split the conditions, as good as one can overview them. 
And add tests for each case. It seems you still have a running clang-format 6, 
then you are in the perfect place to regression test against that without any 
additional overhead.

Please mark all comments as done, if they are done.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:803
 CurrentState.LastSpace = State.Column;
-  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_CtorInitializerColon)) &&
+} else if (Previous.is(TT_CtorInitializerColon)) {
+  CurrentState.LastSpace = State.Column;

Before it was followed with an `&&`, I don't know which one was for this case, 
and which one for the other, or for both. This is a bit hard, but maybe this is 
just to few checks?



Comment at: clang/unittests/Format/FormatTest.cpp:6599-6601
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BinPackParameters = false;
+  Style.ContinuationIndentWidth = 2;

hel-ableton wrote:
> HazardyKnusperkeks wrote:
> > Do you need this for the observed effect? From the description I don't see 
> > that.
> I think it's necessary for the test to work, otherwise the value would fall 
> back to 4, I think? Would you prefer the test to be re-written with the 
> default value, instead? 
At least the indentation width shouldn't matter and changing that here can give 
the impression it does.


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

https://reviews.llvm.org/D136154

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

[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> I am thinking about adding a new option, say -mgot-calls to allow code 
> generation with the extra indirection. Is it sensible and shall I create 
> another diff to discuss that?

That probably makes sense, yes.




Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:2655
+  // support movw movt, so we are safe to assume that.
+  if (Subtarget->genExecuteOnly()) {
+SDValue GA = DAG.getTargetGlobalAddress(GVal, dl, PtrVt);

Can we directly check that movw/movt is available?  I think that's what we do 
in other places?  (Then just assert we aren't execute-only in the non-movw 
path.)


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

https://reviews.llvm.org/D136203

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

[clang] 6149589 - [OMPIRBuilder] Support depend clause for task

[Prabhdeep Singh Soni via cfe-commits]

Author: Prabhdeep Singh Soni
Date: 2022-10-19T13:11:43-04:00
New Revision: 614958912784a13737720de39b2da40fe6f26e75

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

LOG: [OMPIRBuilder] Support depend clause for task

This patch adds support for the `depend` clause for the `task`
construct.

Reviewed By: jdoerfert

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 84536363d6053..75709740e39ce 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4377,39 +4377,26 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFunction &CGF, 
SourceLocation Loc,
   return Result;
 }
 
-namespace {
-/// Dependence kind for RTL.
-enum RTLDependenceKindTy {
-  DepIn = 0x01,
-  DepInOut = 0x3,
-  DepMutexInOutSet = 0x4,
-  DepInOutSet = 0x8,
-  DepOmpAllMem = 0x80,
-};
-/// Fields ids in kmp_depend_info record.
-enum RTLDependInfoFieldsTy { BaseAddr, Len, Flags };
-} // namespace
-
 /// Translates internal dependency kind into the runtime kind.
 static RTLDependenceKindTy translateDependencyKind(OpenMPDependClauseKind K) {
   RTLDependenceKindTy DepKind;
   switch (K) {
   case OMPC_DEPEND_in:
-DepKind = DepIn;
+DepKind = RTLDependenceKindTy::DepIn;
 break;
   // Out and InOut dependencies must use the same code.
   case OMPC_DEPEND_out:
   case OMPC_DEPEND_inout:
-DepKind = DepInOut;
+DepKind = RTLDependenceKindTy::DepInOut;
 break;
   case OMPC_DEPEND_mutexinoutset:
-DepKind = DepMutexInOutSet;
+DepKind = RTLDependenceKindTy::DepMutexInOutSet;
 break;
   case OMPC_DEPEND_inoutset:
-DepKind = DepInOutSet;
+DepKind = RTLDependenceKindTy::DepInOutSet;
 break;
   case OMPC_DEPEND_outallmemory:
-DepKind = DepOmpAllMem;
+DepKind = RTLDependenceKindTy::DepOmpAllMem;
 break;
   case OMPC_DEPEND_source:
   case OMPC_DEPEND_sink:
@@ -4457,7 +,9 @@ CGOpenMPRuntime::getDepobjElements(CodeGenFunction &CGF, 
LValue DepobjLVal,
   DepObjAddr, KmpDependInfoTy, Base.getBaseInfo(), Base.getTBAAInfo());
   // NumDeps = deps[i].base_addr;
   LValue BaseAddrLVal = CGF.EmitLValueForField(
-  NumDepsBase, *std::next(KmpDependInfoRD->field_begin(), BaseAddr));
+  NumDepsBase,
+  *std::next(KmpDependInfoRD->field_begin(),
+ static_cast(RTLDependInfoFields::BaseAddr)));
   llvm::Value *NumDeps = CGF.EmitLoadOfScalar(BaseAddrLVal, Loc);
   return std::make_pair(NumDeps, Base);
 }
@@ -4503,18 +4492,24 @@ static void emitDependData(CodeGenFunction &CGF, 
QualType &KmpDependInfoTy,
 }
 // deps[i].base_addr = &;
 LValue BaseAddrLVal = CGF.EmitLValueForField(
-Base, *std::next(KmpDependInfoRD->field_begin(), BaseAddr));
+Base,
+*std::next(KmpDependInfoRD->field_begin(),
+   static_cast(RTLDependInfoFields::BaseAddr)));
 CGF.EmitStoreOfScalar(Addr, BaseAddrLVal);
 // deps[i].len = sizeof();
 LValue LenLVal = CGF.EmitLValueForField(
-Base, *std::next(KmpDependInfoRD->field_begin(), Len));
+Base, *std::next(KmpDependInfoRD->field_begin(),
+ static_cast(RTLDependInfoFields::Len)));
 CGF.EmitStoreOfScalar(Size, LenLVal);
 // deps[i].flags = ;
 RTLDependenceKindTy DepKind = translateDependencyKind(Data.DepKind);
 LValue FlagsLVal = CGF.EmitLValueForField(
-Base, *std::next(KmpDependInfoRD->field_begin(), Flags));
-CGF.EmitStoreOfScalar(llvm::ConstantInt::get(LLVMFlagsTy, DepKind),
-  FlagsLVal);
+Base,
+*std::next(KmpDependInfoRD->field_begin(),
+   static_cast(RTLDependInfoFields::Flags)));
+CGF.EmitStoreOfScalar(
+llvm::ConstantInt::get(LLVMFlagsTy, static_cast(DepKind)),
+FlagsLVal);
 if (unsigned *P = Pos.dyn_cast()) {
   ++(*P);
 } else {
@@ -4790,7 +4785,9 @@ Address CGOpenMPRuntime::emitDepobjDependClause(
   LValue Base = CGF.MakeAddrLValue(DependenciesArray, KmpDependInfoTy);
   // deps[i].base_addr = NumDependencies;
   LValue BaseAddrLVal = CGF.EmitLValueForField(
-  Base, *std::next(KmpDependInfoRD->field_begin(), BaseAddr));
+  Base,
+  *std::next(KmpDependInfoRD->field_begin(),
+ static_cast(RTLDependInfoFields::BaseAddr)));
   CGF.EmitStoreOfScalar(NumDepsVal, BaseAddrLVal);
   llvm::PointerUnion Pos;
   unsigned Idx = 

[PATCH] D135695: [OMPIRBuilder] Support depend clause for task construct

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG614958912784: [OMPIRBuilder] Support depend clause for task 
(authored by psoni2628, committed by Prabhdeep Singh Soni (A) 
).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135695

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -5092,6 +5092,81 @@
   EXPECT_FALSE(verifyModule(*M, &errs()));
 }
 
+TEST_F(OpenMPIRBuilderTest, CreateTaskDepend) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+  auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP) {};
+  BasicBlock *AllocaBB = Builder.GetInsertBlock();
+  BasicBlock *BodyBB = splitBB(Builder, /*CreateBranch=*/true, "alloca.split");
+  OpenMPIRBuilder::LocationDescription Loc(
+  InsertPointTy(BodyBB, BodyBB->getFirstInsertionPt()), DL);
+  AllocaInst *InDep = Builder.CreateAlloca(Type::getInt32Ty(M->getContext()));
+  OpenMPIRBuilder::DependData DDIn(RTLDependenceKindTy::DepIn,
+   Type::getInt32Ty(M->getContext()), InDep);
+  SmallVector DDS;
+  DDS.push_back(&DDIn);
+  Builder.restoreIP(OMPBuilder.createTask(
+  Loc, InsertPointTy(AllocaBB, AllocaBB->getFirstInsertionPt()), BodyGenCB,
+  /*Tied=*/false, /*Final*/ nullptr, /*IfCondition*/ nullptr, DDS));
+  OMPBuilder.finalize();
+  Builder.CreateRetVoid();
+
+  // Check for the `NumDeps` argument
+  CallInst *TaskAllocCall = dyn_cast(
+  OMPBuilder
+  .getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_omp_task_with_deps)
+  ->user_back());
+  ASSERT_NE(TaskAllocCall, nullptr);
+  ConstantInt *NumDeps = dyn_cast(TaskAllocCall->getArgOperand(3));
+  ASSERT_NE(NumDeps, nullptr);
+  EXPECT_EQ(NumDeps->getZExtValue(), 1U);
+
+  // Check for the `DepInfo` array argument
+  BitCastInst *DepArrayPtr =
+  dyn_cast(TaskAllocCall->getOperand(4));
+  ASSERT_NE(DepArrayPtr, nullptr);
+  AllocaInst *DepArray = dyn_cast(DepArrayPtr->getOperand(0));
+  ASSERT_NE(DepArray, nullptr);
+  Value::user_iterator DepArrayI = DepArray->user_begin();
+  EXPECT_EQ(*DepArrayI, DepArrayPtr);
+  ++DepArrayI;
+  Value::user_iterator DepInfoI = DepArrayI->user_begin();
+  // Check for the `DependKind` flag in the `DepInfo` array
+  Value *Flag = findStoredValue(*DepInfoI);
+  ASSERT_NE(Flag, nullptr);
+  ConstantInt *FlagInt = dyn_cast(Flag);
+  ASSERT_NE(FlagInt, nullptr);
+  EXPECT_EQ(FlagInt->getZExtValue(),
+static_cast(RTLDependenceKindTy::DepIn));
+  ++DepInfoI;
+  // Check for the size in the `DepInfo` array
+  Value *Size = findStoredValue(*DepInfoI);
+  ASSERT_NE(Size, nullptr);
+  ConstantInt *SizeInt = dyn_cast(Size);
+  ASSERT_NE(SizeInt, nullptr);
+  EXPECT_EQ(SizeInt->getZExtValue(), 4U);
+  ++DepInfoI;
+  // Check for the variable address in the `DepInfo` array
+  Value *AddrStored = findStoredValue(*DepInfoI);
+  ASSERT_NE(AddrStored, nullptr);
+  PtrToIntInst *AddrInt = dyn_cast(AddrStored);
+  ASSERT_NE(AddrInt, nullptr);
+  Value *Addr = AddrInt->getPointerOperand();
+  EXPECT_EQ(Addr, InDep);
+
+  ConstantInt *NumDepsNoAlias =
+  dyn_cast(TaskAllocCall->getArgOperand(5));
+  ASSERT_NE(NumDepsNoAlias, nullptr);
+  EXPECT_EQ(NumDepsNoAlias->getZExtValue(), 0U);
+  EXPECT_EQ(TaskAllocCall->getOperand(6),
+ConstantPointerNull::get(Type::getInt8PtrTy(M->getContext(;
+
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
 TEST_F(OpenMPIRBuilderTest, CreateTaskFinal) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
   OpenMPIRBuilder OMPBuilder(*M);
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1290,7 +1290,8 @@
 OpenMPIRBuilder::InsertPointTy
 OpenMPIRBuilder::createTask(const LocationDescription &Loc,
 InsertPointTy AllocaIP, BodyGenCallbackTy BodyGenCB,
-bool Tied, Value *Final, Value *IfCondition) {
+bool Tied, Value *Final, Value *IfCondition,
+ArrayRef Dependencies) {
   if (!updateToLocation(Loc))
 return InsertPointTy();
 
@@ -1322,8 +1323,8 @@
   OI.EntryBB = TaskAllocaBB;
   OI.OuterAllocaBB 

[PATCH] D135011: Add builtin_elementwise_sin and builtin_elementwise_cos

[Joshua Batista via Phabricator via cfe-commits]

bob80905 added a comment.

Ping, could someone please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135011

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

[PATCH] D136239: [testcase] [OpenMP] Fix the testcase error of check-all when DCLANG_DEFAULT_OPENMP_RUNTIME is not libomp

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136239

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

[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:1530
+
+def record_needs_implicit_default_constructor(self):
+"""Returns True if the cursor refers to a C++ record declaration

aaron.ballman wrote:
> dblaikie wrote:
> > anderslanglands wrote:
> > > dblaikie wrote:
> > > > royjacobson wrote:
> > > > > anderslanglands wrote:
> > > > > > dblaikie wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > dblaikie wrote:
> > > > > > > > > anderslanglands wrote:
> > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > royjacobson wrote:
> > > > > > > > > > > > > anderslanglands wrote:
> > > > > > > > > > > > > > royjacobson wrote:
> > > > > > > > > > > > > > > anderslanglands wrote:
> > > > > > > > > > > > > > > > royjacobson wrote:
> > > > > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > > > > > I don't think we should expose any of the 
> > > > > > > > > > > > > > > > > > > "needs" functions like this -- those are 
> > > > > > > > > > > > > > > > > > > internal implementation details of the 
> > > > > > > > > > > > > > > > > > > class and I don't think we want to 
> > > > > > > > > > > > > > > > > > > calcify that into something we have to 
> > > > > > > > > > > > > > > > > > > support forever. As we add members to a 
> > > > > > > > > > > > > > > > > > > class, we recalculate whether the added 
> > > > > > > > > > > > > > > > > > > member causes us to delete defaulted 
> > > > > > > > > > > > > > > > > > > special members (among other things), and 
> > > > > > > > > > > > > > > > > > > the "needs" functions are basically used 
> > > > > > > > > > > > > > > > > > > when the class is completed to handle 
> > > > > > > > > > > > > > > > > > > lazily created special members. I'm 
> > > > > > > > > > > > > > > > > > > pretty sure that lazy creation is not 
> > > > > > > > > > > > > > > > > > > mandated by the standard, which is why I 
> > > > > > > > > > > > > > > > > > > think the "needs" functions are more of 
> > > > > > > > > > > > > > > > > > > an implementation detail.
> > > > > > > > > > > > > > > > > > CC @erichkeane and @royjacobson as folks 
> > > > > > > > > > > > > > > > > > who have been in this same area of the 
> > > > > > > > > > > > > > > > > > compiler to see if they agree or disagree 
> > > > > > > > > > > > > > > > > > with my assessment there.
> > > > > > > > > > > > > > > > > I think so. The 'needs_*' functions query 
> > > > > > > > > > > > > > > > > `DeclaredSpecialMembers` and I'm pretty sure 
> > > > > > > > > > > > > > > > > it's modified when we add the implicit 
> > > > > > > > > > > > > > > > > definitions in the class completion code. So 
> > > > > > > > > > > > > > > > > this looks a bit suspicious. Is this API 
> > > > > > > > > > > > > > > > > //meant// to be used with incomplete classes?
> > > > > > > > > > > > > > > > > For complete classes I think looking up the 
> > > > > > > > > > > > > > > > > default/move/copy constructor and calling 
> > > > > > > > > > > > > > > > > `isImplicit()` is the way to do it.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > About the 'is deleted' API - can't the same 
> > > > > > > > > > > > > > > > > be done for those functions as well so we 
> > > > > > > > > > > > > > > > > have a smaller API? 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > If this //is// meant to be used with 
> > > > > > > > > > > > > > > > > incomplete classes for efficiency that would 
> > > > > > > > > > > > > > > > > be another thing, I guess.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > So the intended use case here is I'm using 
> > > > > > > > > > > > > > > > libclang to parse an existing C++ libray's 
> > > > > > > > > > > > > > > > headers and generate a C interface to it. To do 
> > > > > > > > > > > > > > > > that I need to know if I need to generate 
> > > > > > > > > > > > > > > > default constructors etc, which the needs* 
> > > > > > > > > > > > > > > > methods do for me (I believe). The alternative 
> > > > > > > > > > > > > > > > is I have to check manually whether all the 
> > > > > > > > > > > > > > > > constructors/assignment operators exist, then 
> > > > > > > > > > > > > > > > implement the implicit declaration rules myself 
> > > > > > > > > > > > > > > > correctly for each version of the standard, 
> > > > > > > > > > > > > > > > which I'd rather avoid.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Would putting a note in the doc comment about 
> > > > > > > > > > > > > > > > the behaviour differing when the class is being 
> > > > > > > > > > > > > > > > constructed as originally suggested work for 
> > > > > > > > > > > > > > > > everyone?
> > > > > > > > > > > > > > > Why is the `__is_default_constructible` builtin 
> > > > > > > > > > > > > > > type trait no

[PATCH] D134453: Disambiguate type names when printing NTTP types

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I still don't think "keep full NTTP type printing behind a policy, for those 
that want/need that" is a policy we should add. String printed names aren't 
meant to be a tool for type reflection - the AST can be queried for that 
information.




Comment at: clang/test/CodeGenCXX/debug-info-template.cpp:224
 template void f1();
-// CHECK: !DISubprogram(name: "f1", 
+// CHECK: !DISubprogram(name: "f1",
 // CHECK-SAME: templateParams: ![[RAW_FUNC_QUAL_ARGS:[0-9]*]],

Looks like some unintended whitespace changes? (removing trailing whitespace 
from these lines) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453

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

[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.

[Zhiyao Ma via Phabricator via cfe-commits]

ZhiyaoMa98 updated this revision to Diff 468957.

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

https://reviews.llvm.org/D136203

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-execute-only.c
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll

Index: llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb2/thumb2-execute-only-long-calls.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=thumbv7em-arm-none-eabi -relocation-model=static | FileCheck %s -check-prefixes=CHECK,STATIC
+; RUN: llc < %s -mtriple=thumbv7em-arm-none-eabi -relocation-model=rwpi | FileCheck %s -check-prefixes=CHECK,RWPI
+
+define void @fn() #0 {
+entry:
+; CHECK-LABEL: fn:
+; CHECK:   ldr [[REG:r[0-9]+]], .LCPI0_0
+; CHECK-NEXT:  blx [[REG]]
+; CHECK:   .LCPI0_0:
+; CHECK-NEXT:  .long   bar
+  call void @bar()
+  ret void
+}
+
+define void @execute_only_fn() #1 {
+; STATIC-LABEL: execute_only_fn:
+; STATIC:   movw[[REG0:r[0-9]+]], :lower16:bar
+; STATIC-NEXT:  movt[[REG0]], :upper16:bar
+; STATIC-NEXT:  blx [[REG0]]
+; STATIC-NOT:   .LCPI0_0:
+
+; RWPI-LABEL: execute_only_fn:
+; RWPI:   movw[[REG0:r[0-9]+]], :lower16:bar
+; RWPI-NEXT:  movt[[REG0]], :upper16:bar
+; RWPI-NEXT:  blx [[REG0]]
+; RWPI-NOT:   .LCPI0_0:
+entry:
+  call void @bar()
+  ret void
+}
+
+attributes #0 = { noinline optnone "target-features"="+thumb-mode,+long-calls" }
+attributes #1 = { noinline optnone "target-features"="+execute-only,+thumb-mode,+long-calls" }
+
+declare dso_local void @bar()
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -2630,11 +2630,11 @@
 
   const TargetMachine &TM = getTargetMachine();
   const Module *Mod = MF.getFunction().getParent();
-  const GlobalValue *GV = nullptr;
+  const GlobalValue *GVal = nullptr;
   if (GlobalAddressSDNode *G = dyn_cast(Callee))
-GV = G->getGlobal();
+GVal = G->getGlobal();
   bool isStub =
-  !TM.shouldAssumeDSOLocal(*Mod, GV) && Subtarget->isTargetMachO();
+  !TM.shouldAssumeDSOLocal(*Mod, GVal) && Subtarget->isTargetMachO();
 
   bool isARMFunc = !Subtarget->isThumb() || (isStub && !Subtarget->isMClass());
   bool isLocalARMFunc = false;
@@ -2647,36 +2647,63 @@
 // those, the target's already in a register, so we don't need to do
 // anything extra.
 if (isa(Callee)) {
-  // Create a constant pool entry for the callee address
-  unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
-  ARMConstantPoolValue *CPV =
-ARMConstantPoolConstant::Create(GV, ARMPCLabelIndex, ARMCP::CPValue, 0);
-
-  // Get the address of the callee into a register
-  SDValue CPAddr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
-  CPAddr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, CPAddr);
-  Callee = DAG.getLoad(
-  PtrVt, dl, DAG.getEntryNode(), CPAddr,
-  MachinePointerInfo::getConstantPool(DAG.getMachineFunction()));
+  SDValue Addr;
+
+  // When generating execute-only code we use movw movt pair.
+  // Currently execute-only is only available for architectures that
+  // support movw movt, so we are safe to assume that.
+  if (Subtarget->genExecuteOnly()) {
+assert(Subtarget->useMovt() &&
+   "long-calls with execute-only requires movt and movw!");
+SDValue GA = DAG.getTargetGlobalAddress(GVal, dl, PtrVt);
+++NumMovwMovt;
+Callee = DAG.getNode(ARMISD::Wrapper, dl, PtrVt,
+ DAG.getTargetGlobalAddress(GVal, dl, PtrVt));
+  } else {
+// Create a constant pool entry for the callee address
+unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
+ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create(
+GVal, ARMPCLabelIndex, ARMCP::CPValue, 0);
+
+// Get the address of the callee into a register
+Addr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
+Addr = DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, Addr);
+Callee = DAG.getLoad(
+PtrVt, dl, DAG.getEntryNode(), Addr,
+MachinePointerInfo::getConstantPool(DAG.getMachineFunction()));
+  }
 } else if (ExternalSymbolSDNode *S=dyn_cast(Callee)) {
   const char *Sym = S->getSymbol();
-
-  // Create a constant pool entry for the callee address
-  unsigned ARMPCLabelIndex = AFI->createPICLabelUId();
-  ARMConstantPoolValue *CPV =
-ARMConstantPoolSymbol::Create(*DAG.getContext(), Sym,
-  ARMPCLabelIndex, 0);
-  // Get the address of the callee into a register
-  SDValue CPAddr = DAG.getTargetConstantPool(CPV, PtrVt, Align(4));
-  CPAddr = DAG.ge

[PATCH] D136203: [ARM] Support -mexecute-only with -mlong-calls.

[Zhiyao Ma via Phabricator via cfe-commits]

ZhiyaoMa98 added a comment.

> Then just assert we aren't execute-only in the non-movw path.

When we are not execute-only, existing code handles it by using constant pools 
and we are all good.

In the case where we are execute-only and long-calls at the same time, we 
assert that we have `movt` like in other places in the same source file.


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

https://reviews.llvm.org/D136203

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

[PATCH] D134453: Disambiguate type names when printing NTTP types

[Nenad Mikša via Phabricator via cfe-commits]

DoDoENT added a comment.

In D134453#3868789 , @dblaikie wrote:

> I still don't think "keep full NTTP type printing behind a policy, for those 
> that want/need that" is a policy we should add. String printed names aren't 
> meant to be a tool for type reflection - the AST can be queried for that 
> information.

I agree on that matter.

However, I'm building my clang with this policy enabled by default to provide 
my developers with GCC/MSVC-like verbosity in diagnostic messages. They prefer 
it that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I'm OK with sticking with the existing `-fmodule-file` if that works for 
everyone. Yeah, it's short and ambiguous in a space with many concepts of what 
a "module file" is, but also the fact that it's a `-f` flag might help 
disambiguate it a bit - it's probably not the way anyone would think/expect to 
be passing source files, those just get passed without flags on the command 
line. And any use of it will show the .pcm extension or whatever that should 
make it clear enough what's going on.


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

https://reviews.llvm.org/D134267

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

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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