date:20221015

[PATCH] D135858: [clang][Interp] Support pointer arithmethic in binary operators

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 468058.

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

https://reviews.llvm.org/D135858

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/lib/AST/Interp/Pointer.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -37,6 +37,52 @@
 static_assert(getElement(1) == 4, "");
 static_assert(getElement(5) == 36, "");
 
+constexpr int data[] = {5, 4, 3, 2, 1};
+constexpr int getElement(const int *Arr, int index) {
+  return *(Arr + index);
+}
+
+static_assert(getElement(data, 1) == 4, "");
+static_assert(getElement(data, 4) == 1, "");
+
+constexpr int getElementFromEnd(const int *Arr, int size, int index) {
+  return *(Arr + size - index - 1);
+}
+static_assert(getElementFromEnd(data, 5, 0) == 1, "");
+static_assert(getElementFromEnd(data, 5, 4) == 5, "");
+
+
+constexpr static int arr[2] = {1,2};
+constexpr static int arr2[2] = {3,4};
+constexpr int *p1 = nullptr;
+constexpr int *p2 = p1 + 1; // expected-error {{must be initialized by a constant expression}} \
+// expected-note {{cannot perform pointer arithmetic on null pointer}} \
+// ref-error {{must be initialized by a constant expression}} \
+// ref-note {{cannot perform pointer arithmetic on null pointer}}
+constexpr int *p3 = p1 + 0;
+constexpr int *p4 = p1 - 0;
+constexpr int *p5 =  0 + p1;
+constexpr int *p6 =  0 - p1; // expected-error {{invalid operands to binary expression}} \
+ // ref-error {{invalid operands to binary expression}}
+
+constexpr int const * ap1 = &arr[0];
+constexpr int const * ap2 = ap1 + 3; // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{cannot refer to element 3 of array of 2}} \
+ // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{cannot refer to element 3 of array of 2}}
+
+constexpr auto ap3 = arr - 1; // expected-error {{must be initialized by a constant expression}} \
+  // expected-note {{cannot refer to element -1}} \
+  // ref-error {{must be initialized by a constant expression}} \
+  // ref-note {{cannot refer to element -1}}
+constexpr int  k1 = &arr[1] - &arr[0];
+static_assert(k1 == 1, "");
+static_assert((&arr[0] - &arr[1]) == -1, "");
+
+// FIXME: These two should work.
+static_assert((arr + 0) == arr, ""); // expected-error {{static assertion failed}}
+static_assert(&arr[0] == arr, ""); // expected-error {{static assertion failed}}
+
 
 template
 constexpr T getElementOf(T* array, int i) {
@@ -52,7 +98,6 @@
 static_assert(getElementOfArray(foo[2], 3) == &m, "");
 
 
-constexpr int data[] = {5, 4, 3, 2, 1};
 static_assert(data[0] == 4, ""); // expected-error{{failed}} \
  // expected-note{{5 == 4}} \
  // ref-error{{failed}} \
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -201,5 +201,5 @@
 }
 
 bool Pointer::hasSameArray(const Pointer &A, const Pointer &B) {
-  return A.Base == B.Base && A.getFieldDesc()->IsArray;
+  return hasSameBase(A, B) && A.Base == B.Base && A.getFieldDesc()->IsArray;
 }
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -411,6 +411,12 @@
   let HasGroup = 1;
 }
 
+// Pointer, Pointer] - [Integral]
+def SubPtr : Opcode {
+  let Types = [IntegerTypeClass];
+  let HasGroup = 1;
+}
+
 //===--===//
 // Binary operators.
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -876,6 +876,14 @@
   // Fetch the pointer and the offset.
   const T &Offset = S.Stk.pop();
   const Pointer &Ptr = S.Stk.pop();
+
+  // Pointer arithmethic with nullptr and 0 is valid and just
+  // results in nullptr again.
+  if (Offset.isZero() && Ptr.isZero()) {
+S.Stk.push(Ptr);
+return true;
+  }
+
   if (!CheckNull(S, OpPC, Ptr, CSK_ArrayIndex))
 return false;
   if (!CheckRange(S, OpPC, Ptr, CSK_ArrayToPointer))
@@ -946,6 +954,18 @@
   return OffsetHelper(S, OpPC);
 }
 
+/// 1) Pops a Pointer from the stack.

[PATCH] D132975: [CMake] Add clang-bolt target

2022-10-15 Thread Amir Ayupov via Phabricator via cfe-commits

Amir added a comment.

In D132975#3763264 , @phosek wrote:

> This was already on my list of build system features I'd like to implement 
> and I'm glad someone else is already looking into it, thank you! I have two 
> high level comments about your approach.
>
> The first one is related to the use of Clang build as the training data. I 
> think that Clang build is both unnecessarily heavyweight, but also not 
> particularly representative of typical workloads (most Clang users don't use 
> it to build Clang). Ideally, we would give vendors the flexibility to supply 
> their own training data. I'd prefer reusing the existing perf-training 
>  
> setup to do so. In fact, I'd imagine most vendors would likely use the same 
> training data for both PGO and BOLT and that use case should be supported.

Do you happen to know any existing perf-training sets? Or is there a simple way 
to create one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132975

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

[PATCH] D71507: [perf-training] Make training data location configurable

2022-10-15 Thread Amir Ayupov via Phabricator via cfe-commits

Amir added a comment.
Herald added a subscriber: wenlei.
Herald added a project: All.

Hi @smeenai,

Sorry for asking on this diff, but... Do you have any pointers for training 
sets?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71507

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

[PATCH] D136031: [DirectX backend] support ConstantBuffer to DXILResource.h

2022-10-15 Thread Xiang Li via Phabricator via cfe-commits

python3kgae updated this revision to Diff 468054.
python3kgae added a comment.

Rebase to fix stack patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136031

Files:
  llvm/lib/Target/DirectX/CBufferDataLayout.cpp
  llvm/lib/Target/DirectX/CBufferDataLayout.h
  llvm/lib/Target/DirectX/CMakeLists.txt
  llvm/lib/Target/DirectX/DXILResource.cpp
  llvm/lib/Target/DirectX/DXILResource.h
  llvm/test/CodeGen/DirectX/cbuf.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_0.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_1.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_2.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_3.ll

Index: llvm/test/CodeGen/DirectX/legacy_cb_layout_3.ll
===
--- /dev/null
+++ llvm/test/CodeGen/DirectX/legacy_cb_layout_3.ll
@@ -0,0 +1,81 @@
+; RUN: opt -S -dxil-metadata-emit < %s | FileCheck %s --check-prefix=DXILMD
+
+target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64"
+target triple = "dxil-unknown-shadermodel6.7-library"
+
+; cbuffer D
+; {
+;
+;   struct D
+;   {
+;
+;   int D0;   ; Offset:0
+;   struct struct.B
+;   {
+;
+;   double B0;; Offset:   16
+;   float3 B1;; Offset:   32
+;   float B2; ; Offset:   44
+;   double3 B3;   ; Offset:   48
+;   half B4;  ; Offset:   72
+;   double2 B5;   ; Offset:   80
+;   float B6; ; Offset:   96
+;   half3 B7; ; Offset:  100
+;   half3 B8; ; Offset:  106
+;   
+;   } D1; ; Offset:   16
+;
+;   half D2;  ; Offset:  112
+;   struct struct.C
+;   {
+;
+;   struct struct.A
+;   {
+;
+;   float A0; ; Offset:  128
+;   double A1;; Offset:  136
+;   float A2; ; Offset:  144
+;   half A3;  ; Offset:  148
+;   int16_t A4;   ; Offset:  150
+;   int64_t A5;   ; Offset:  152
+;   int A6;   ; Offset:  160
+;   
+;   } C0; ; Offset:  128
+;
+;   float C1[1];  ; Offset:  176
+;   struct struct.B
+;   {
+;
+;   double B0;; Offset:  192
+;   float3 B1;; Offset:  208
+;   float B2; ; Offset:  220
+;   double3 B3;   ; Offset:  224
+;   half B4;  ; Offset:  248
+;   double2 B5;   ; Offset:  256
+;   float B6; ; Offset:  272
+;   half3 B7; ; Offset:  276
+;   half3 B8; ; Offset:  282
+;   
+;   } C2[2];; ; Offset:  192
+;
+;   half C3;  ; Offset:  384
+;   
+;   } D3; ; Offset:  128
+;
+;   double D4;; Offset:  392
+;   
+;   } D;  ; Offset:0 Size:   400
+
+
+; Make sure the size is 400
+; DXILMD:!{i32 0, ptr @D.cb., !"", i32 0, i32 1, i32 1, i32 400}
+
+
+%struct.B = type <{ double, <3 x float>, float, <3 x double>, half, <2 x double>, float, <3 x half>, <3 x half> }>
+%struct.C = type <{ %struct.A, [1 x float], [2 x %struct.B], half }>
+%struct.A = type <{ float, double, float, half, i16, i64, i32 }>
+
+@D.cb. = external local_unnamed_addr constant { i32, %struct.B, half, %struct.C, double }
+
+!hlsl.cbufs = !{!0}
+!0 = !{ptr @D.cb., !"D.cb.ty", i32 0, i32 13, i32 1, i32 0}
Index: llvm/test/CodeGen/DirectX/legacy_cb_layout_2.ll
===
--- /dev/null
+++ llvm/test/CodeGen/DirectX/legacy_cb_layout_2.ll
@@ -0,0 +1,51 @@
+; RUN: opt -S -dxil-metadata-emit < %s | FileCheck %s --check-prefix=DXILMD
+
+target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64"
+target triple = "dxil-unknown-shadermodel6.7-library"
+
+; cbuffer B
+; {
+;
+;   struct B
+;   {
+;
+;   double B0[2]; ; Offset:0
+;

[PATCH] D130951: [HLSL] CodeGen hlsl resource binding.

2022-10-15 Thread Xiang Li via Phabricator via cfe-commits

python3kgae updated this revision to Diff 468053.
python3kgae added a comment.

Rebase for D136031 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130951

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/test/CodeGenHLSL/builtins/RWBuffer-annotations.hlsl
  clang/test/CodeGenHLSL/cbuf.hlsl
  llvm/include/llvm/Frontend/HLSL/HLSLResource.h
  llvm/lib/Frontend/HLSL/HLSLResource.cpp
  llvm/lib/Target/DirectX/DXILResource.cpp
  llvm/lib/Target/DirectX/DXILResource.h
  llvm/test/CodeGen/DirectX/UAVMetadata.ll

Index: llvm/test/CodeGen/DirectX/UAVMetadata.ll
===
--- llvm/test/CodeGen/DirectX/UAVMetadata.ll
+++ llvm/test/CodeGen/DirectX/UAVMetadata.ll
@@ -13,15 +13,15 @@
 ; PRINT-NEXT:; Name Type  Format Dim  ID  HLSL Bind  Count
 ; PRINT-NEXT:; -- -- --- --- --- -- --
 ; PRINT-NEXT:;   UAV f16 buf  U0 u0 1
-; PRINT-NEXT:;   UAV f32 buf  U1 u0 1
-; PRINT-NEXT:;   UAV f64 buf  U2 u0 1
-; PRINT-NEXT:;   UAV  i1 buf  U3 u0 2
-; PRINT-NEXT:;   UAVbyte r/w  U4 u0 1
-; PRINT-NEXT:;   UAV  struct r/w  U5 u0 1
-; PRINT-NEXT:;   UAV i32 buf  U6 u0 1
-; PRINT-NEXT:;   UAV  struct r/w  U7 u0 1
-; PRINT-NEXT:;   UAVbyte r/w  U8 u0 1
-; PRINT-NEXT:;   UAV u64 buf  U9 u0 1
+; PRINT-NEXT:;   UAV f32 buf  U1 u1 1
+; PRINT-NEXT:;   UAV f64 buf  U2 u2 1
+; PRINT-NEXT:;   UAV  i1 buf  U3 u3 2
+; PRINT-NEXT:;   UAVbyte r/w  U4 u5 1
+; PRINT-NEXT:;   UAV  struct r/w  U5 u6 1
+; PRINT-NEXT:;   UAV i32 buf  U6 u7 1
+; PRINT-NEXT:;   UAV  struct r/w  U7 u8 1
+; PRINT-NEXT:;   UAVbyte r/w  U8 u9 1
+; PRINT-NEXT:;   UAV u64 buf  U9 u10,space2 1
 
 @Zero = local_unnamed_addr global %"class.hlsl::RWBuffer" zeroinitializer, align 4
 @One = local_unnamed_addr global %"class.hlsl::RWBuffer" zeroinitializer, align 4
@@ -37,16 +37,16 @@
 
 !hlsl.uavs = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9}
 
-!0 = !{ptr @Zero, !"RWBuffer", i32 0}
-!1 = !{ptr @One, !"Buffer>", i32 1}
-!2 = !{ptr @Two, !"Buffer", i32 2}
-!3 = !{ptr @Three, !"Buffer", i32 3}
-!4 = !{ptr @Four, !"ByteAddressBuffer", i32 4}
-!5 = !{ptr @Five, !"StructuredBuffer", i32 5}
-!6 = !{ptr @Six, !"RasterizerOrderedBuffer", i32 6}
-!7 = !{ptr @Seven, !"RasterizerOrderedStructuredBuffer", i32 7}
-!8 = !{ptr @Eight, !"RasterizerOrderedByteAddressBuffer", i32 8}
-!9 = !{ptr @Nine, !"RWBuffer", i32 9}
+!0 = !{ptr @Zero, !"RWBuffer", i32 0, i32 10, i32 0, i32 0}
+!1 = !{ptr @One, !"Buffer>", i32 1, i32 10, i32 1, i32 0}
+!2 = !{ptr @Two, !"Buffer", i32 2, i32 10, i32 2, i32 0}
+!3 = !{ptr @Three, !"Buffer", i32 3, i32 10, i32 3, i32 0}
+!4 = !{ptr @Four, !"ByteAddressBuffer", i32 4, i32 11, i32 5, i32 0}
+!5 = !{ptr @Five, !"StructuredBuffer", i32 5, i32 12, i32 6, i32 0}
+!6 = !{ptr @Six, !"RasterizerOrderedBuffer", i32 6, i32 10, i32 7, i32 0}
+!7 = !{ptr @Seven, !"RasterizerOrderedStructuredBuffer", i32 7, i32 12, i32 8, i32 0}
+!8 = !{ptr @Eight, !"RasterizerOrderedByteAddressBuffer", i32 8, i32 11, i32 9, i32 0}
+!9 = !{ptr @Nine, !"RWBuffer", i32 9, i32 10, i32 10, i32 2}
 
 ; CHECK: !dx.resources = !{[[ResList:[!][0-9]+]]}
 
@@ -57,21 +57,21 @@
 ; CHECK-SAME: [[Eight:[!][0-9]+]], [[Nine:[!][0-9]+]]}
 ; CHECK: [[Zero]] = !{i32 0, ptr @Zero, !"", i32 0, i32 0, i32 1, i32 10, i1 false, i1 false, i1 false, [[Half:[!][0-9]+]]}
 ; CHECK: [[Half]] = !{i32 0, i32 8}
-; CHECK: [[One]] = !{i32 1, ptr @One, !"", i32 0, i32 0, i32 1, i32 10, i1 false, i1 false, i1 false, [[Float:[!][0-9]+]]}
+; CHECK: [[One]] = !{i32 1, p

[PATCH] D136033: Add an option to specify a workspece-level config file

2022-10-15 Thread Qingyuan Zheng via Phabricator via cfe-commits

daiyousei-qz created this revision.
daiyousei-qz added reviewers: nridge, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
daiyousei-qz requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang-tools-extra.

Add an option "--workspace-config=" to specify a workspace config file, 
which stands between the in-tree .clangd and the global user config file and 
offers per-workspace customization.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136033

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -491,8 +491,9 @@
 "enable-config",
 cat(Misc),
 desc(
-"Read user and project configuration from YAML files.\n"
+"Read user/workspace/project configuration from YAML files.\n"
 "Project config is from a .clangd file in the project directory.\n"
+"Workspace config is specified by the --workspace-config option.\n"
 "User config is from clangd/config.yaml in the following 
directories:\n"
 "\tWindows: %USERPROFILE%\\AppData\\Local\n"
 "\tMac OS: ~/Library/Preferences/\n"
@@ -501,6 +502,13 @@
 init(true),
 };
 
+opt WorkspaceConfig{
+"workspace-config",
+cat(Misc),
+desc("Path to a workspace configuration file."),
+init(""),
+};
+
 opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
   desc("Use files open in the editor when parsing "
"headers instead of reading from the disk"),
@@ -923,6 +931,15 @@
   if (EnableConfig) {
 ProviderStack.push_back(
 config::Provider::fromAncestorRelativeYAMLFiles(".clangd", TFS));
+if (!WorkspaceConfig.empty() && llvm::sys::fs::exists(WorkspaceConfig)) {
+  llvm::SmallString<256> AbsWorkspaceConfigPath =
+  StringRef{WorkspaceConfig.getValue()};
+  llvm::sys::fs::make_absolute("", AbsWorkspaceConfigPath);
+  ProviderStack.push_back(config::Provider::fromYAMLFile(
+  AbsWorkspaceConfigPath, /*Directory=*/"", TFS, /*Trusted=*/true));
+} else {
+  elog("Couldn't find workspace config file, not loading");
+}
 llvm::SmallString<256> UserConfig;
 if (llvm::sys::path::user_config_directory(UserConfig)) {
   llvm::sys::path::append(UserConfig, "clangd", "config.yaml");


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -491,8 +491,9 @@
 "enable-config",
 cat(Misc),
 desc(
-"Read user and project configuration from YAML files.\n"
+"Read user/workspace/project configuration from YAML files.\n"
 "Project config is from a .clangd file in the project directory.\n"
+"Workspace config is specified by the --workspace-config option.\n"
 "User config is from clangd/config.yaml in the following directories:\n"
 "\tWindows: %USERPROFILE%\\AppData\\Local\n"
 "\tMac OS: ~/Library/Preferences/\n"
@@ -501,6 +502,13 @@
 init(true),
 };
 
+opt WorkspaceConfig{
+"workspace-config",
+cat(Misc),
+desc("Path to a workspace configuration file."),
+init(""),
+};
+
 opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
   desc("Use files open in the editor when parsing "
"headers instead of reading from the disk"),
@@ -923,6 +931,15 @@
   if (EnableConfig) {
 ProviderStack.push_back(
 config::Provider::fromAncestorRelativeYAMLFiles(".clangd", TFS));
+if (!WorkspaceConfig.empty() && llvm::sys::fs::exists(WorkspaceConfig)) {
+  llvm::SmallString<256> AbsWorkspaceConfigPath =
+  StringRef{WorkspaceConfig.getValue()};
+  llvm::sys::fs::make_absolute("", AbsWorkspaceConfigPath);
+  ProviderStack.push_back(config::Provider::fromYAMLFile(
+  AbsWorkspaceConfigPath, /*Directory=*/"", TFS, /*Trusted=*/true));
+} else {
+  elog("Couldn't find workspace config file, not loading");
+}
 llvm::SmallString<256> UserConfig;
 if (llvm::sys::path::user_config_directory(UserConfig)) {
   llvm::sys::path::append(UserConfig, "clangd", "config.yaml");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 08901c8 - [clang] Use llvm::reverse (NFC)

2022-10-15 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2022-10-15T21:54:13-07:00
New Revision: 08901c8a980d98672d456558fac3b2bee990bf61

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

LOG: [clang] Use llvm::reverse (NFC)

Added: 


Modified: 
clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
clang/lib/AST/Interp/Function.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
index 088474b9b2980..7792707e50250 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
@@ -204,11 +204,11 @@ template  class SimpleArray {
   }
 
   llvm::iterator_range reverse() {
-return llvm::make_range(rbegin(), rend());
+return llvm::reverse(*this);
   }
 
   llvm::iterator_range reverse() const {
-return llvm::make_range(rbegin(), rend());
+return llvm::reverse(*this);
   }
 
 private:

diff  --git a/clang/lib/AST/Interp/Function.h b/clang/lib/AST/Interp/Function.h
index 67a4e27b3753f..f73a6d3f3c9ce 100644
--- a/clang/lib/AST/Interp/Function.h
+++ b/clang/lib/AST/Interp/Function.h
@@ -111,7 +111,7 @@ class Function final {
   using arg_reverse_iterator =
   SmallVectorImpl::const_reverse_iterator;
   llvm::iterator_range args_reverse() const {
-return llvm::make_range(ParamTypes.rbegin(), ParamTypes.rend());
+return llvm::reverse(ParamTypes);
   }
 
   /// Returns a specific scope.

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index faac52de70649..2dfd815c5c93e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1657,7 +1657,7 @@ void RenderARMABI(const Driver &D, const llvm::Triple 
&Triple,
 
 void AddUnalignedAccessWarning(ArgStringList &CmdArgs) {
   auto StrictAlignIter =
-  std::find_if(CmdArgs.rbegin(), CmdArgs.rend(), [](StringRef Arg) {
+  llvm::find_if(llvm::reverse(CmdArgs), [](StringRef Arg) {
 return Arg == "+strict-align" || Arg == "-strict-align";
   });
   if (StrictAlignIter != CmdArgs.rend() &&

diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 4d6b82e63f6a4..6996f590a7cf6 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -3097,9 +3097,8 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
 if (getAnalyzerOptions().ShouldDisplayNotesAsEvents) {
   // For path diagnostic consumers that don't support extra notes,
   // we may optionally convert those to path notes.
-  for (auto I = report->getNotes().rbegin(),
-   E = report->getNotes().rend(); I != E; ++I) {
-PathDiagnosticNotePiece *Piece = I->get();
+  for (const auto &I : llvm::reverse(report->getNotes())) {
+PathDiagnosticNotePiece *Piece = I.get();
 auto ConvertedPiece = std::make_shared(
   Piece->getLocation(), Piece->getString());
 for (const auto &R: Piece->getRanges())
@@ -3108,9 +3107,8 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
 Pieces.push_front(std::move(ConvertedPiece));
   }
 } else {
-  for (auto I = report->getNotes().rbegin(),
-   E = report->getNotes().rend(); I != E; ++I)
-Pieces.push_front(*I);
+  for (const auto &I : llvm::reverse(report->getNotes()))
+Pieces.push_front(I);
 }
 
 for (const auto &I : report->getFixits())



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

[clang] 765af67 - [clang] Remove redundaunt typename (NFC)

2022-10-15 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2022-10-15T21:07:02-07:00
New Revision: 765af675bce580cd6473ce7bfb9255f22db14414

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

LOG: [clang] Remove redundaunt typename (NFC)

Added: 


Modified: 
clang/include/clang/Sema/ParsedAttr.h
clang/lib/ASTMatchers/ASTMatchFinder.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index 1f5237dc4a4d2..8ed89d3f10592 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -1173,21 +1173,21 @@ inline const StreamingDiagnostic &operator<<(const 
StreamingDiagnostic &DB,
 /// it explicit is hard. This constructor causes ambiguity with
 /// DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, SourceRange R).
 /// We use SFINAE to disable any conversion and remove any ambiguity.
-template ::value, int> = 0>
+template <
+typename ACI,
+std::enable_if_t::value, int> = 0>
 inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
-   const ACI &CI) {
+ const ACI &CI) {
   DB.AddTaggedVal(reinterpret_cast(CI.getAttrName()),
   DiagnosticsEngine::ak_identifierinfo);
   return DB;
 }
 
-template ::value, int> = 0>
+template <
+typename ACI,
+std::enable_if_t::value, int> = 0>
 inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
-   const ACI* CI) {
+ const ACI *CI) {
   DB.AddTaggedVal(reinterpret_cast(CI->getAttrName()),
   DiagnosticsEngine::ak_identifierinfo);
   return DB;

diff  --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp 
b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index ac8e4eccad8eb..45eabfeb37319 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -779,7 +779,7 @@ class MatchASTVisitor : public 
RecursiveASTVisitor,
 
 #define IMPL(Index)
\
   template  
\
-  typename std::enable_if_t<   
\
+  std::enable_if_t<
\
   llvm::is_one_of::value> 
\
   SetCallbackAndRawNode(const MatchCallback *CB, const NodeType &N) {  
\
 assertEmpty(); 
\
@@ -788,8 +788,8 @@ class MatchASTVisitor : public 
RecursiveASTVisitor,
   }
\

\
   template 
\
-  typename std::enable_if_t<   
\
-  llvm::is_one_of::value, const T *> 
\
+  std::enable_if_t::value,   
\
+   const T *>  
\
   getNode() const {
\
 assertHoldsState();
\
 return Callback.getInt() == (Index) ? Node##Index.dyn_cast()
\



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

[PATCH] D135951: [X86][1/2] SUPPORT RAO-INT

2022-10-15 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:259
+ "Support RAO-INT instructions",
+ [FeatureSSE2]>;
 def FeatureINVPCID : SubtargetFeature<"invpcid", "HasINVPCID", "true",

craig.topper wrote:
> pengfei wrote:
> > craig.topper wrote:
> > > Why do these require SSE2?
> > We need mfence instructions for strong orders. The mfence feature relies on 
> > SSE2.
> > I see your concern, we may need split these features from SSE2. Filed an 
> > issue https://github.com/llvm/llvm-project/issues/58388
> The lowering code for atomics could check SSE2 in addition to the RAO-INT 
> feature. My primary concern was that the RAO-INT feature itself shouldn't 
> require it.
> 
> Are there going to be intrinsics for RAO-INT?
I don't think there will be a target supports RAO-INT but no SSE2. All I can 
think out is for Kernel build that disable vector registers intentionally.

> Are there going to be intrinsics for RAO-INT?

This is a good question. In fact we wander whether providing intrinsics or not. 
The current preference is not. We don't want to add new intrinsics arbitrarily, 
there're burdens to maintenance and it's always easy to adding per to actual 
requests than removing afterwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

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

[PATCH] D135951: [X86][1/2] SUPPORT RAO-INT

2022-10-15 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:259
+ "Support RAO-INT instructions",
+ [FeatureSSE2]>;
 def FeatureINVPCID : SubtargetFeature<"invpcid", "HasINVPCID", "true",

pengfei wrote:
> craig.topper wrote:
> > Why do these require SSE2?
> We need mfence instructions for strong orders. The mfence feature relies on 
> SSE2.
> I see your concern, we may need split these features from SSE2. Filed an 
> issue https://github.com/llvm/llvm-project/issues/58388
The lowering code for atomics could check SSE2 in addition to the RAO-INT 
feature. My primary concern was that the RAO-INT feature itself shouldn't 
require it.

Are there going to be intrinsics for RAO-INT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

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

[PATCH] D135951: [X86] SUPPORT RAO-INT

2022-10-15 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:259
+ "Support RAO-INT instructions",
+ [FeatureSSE2]>;
 def FeatureINVPCID : SubtargetFeature<"invpcid", "HasINVPCID", "true",

craig.topper wrote:
> Why do these require SSE2?
We need mfence instructions for strong orders. The mfence feature relies on 
SSE2.
I see your concern, we may need split these features from SSE2. Filed an issue 
https://github.com/llvm/llvm-project/issues/58388



Comment at: llvm/lib/Target/X86/X86ISelLowering.h:801
+RXOR,
+RAND,
+

craig.topper wrote:
> RKSimon wrote:
> > very pedantic, but are these likely to get confused with ROR / RAND 
> > instructions? Would it be better to use a RAO_ prefix?
> Why not AADD etc to match the instruction names?
Good ideas, thanks!



Comment at: llvm/test/CodeGen/X86/atomic-instructions-32.ll:5
+; RUN: llc < %s -O0 -verify-machineinstrs -mtriple=x86_64-unknown-unknown | 
FileCheck %s --check-prefixes=X64-NO-RAOINT
+; RUN: llc < %s -O0 -verify-machineinstrs -mtriple=x86_64-unknown-unknown 
-mattr=+raoint | FileCheck %s --check-prefixes=X64-RAO-INT
+

RKSimon wrote:
> Is the -O0 necessary?
I missed that, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

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

[PATCH] D135951: [X86] SUPPORT RAO-INT

2022-10-15 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei updated this revision to Diff 468049.
pengfei marked an inline comment as done.
pengfei added a comment.

Split atomic operations lowering into D136032 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Headers/cpuid.h
  clang/test/Driver/x86-target-features.c
  clang/test/Preprocessor/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/X86InstrInfo.td
  llvm/lib/Target/X86/X86InstrRAOINT.td
  llvm/test/MC/Disassembler/X86/rao-int.txt
  llvm/test/MC/Disassembler/X86/x86-64-rao-int.txt
  llvm/test/MC/X86/rao-int-att.s
  llvm/test/MC/X86/rao-int-intel.s
  llvm/test/MC/X86/x86-64-rao-int-att.s
  llvm/test/MC/X86/x86-64-rao-int-intel.s

Index: llvm/test/MC/X86/x86-64-rao-int-intel.s
===
--- /dev/null
+++ llvm/test/MC/X86/x86-64-rao-int-intel.s
@@ -0,0 +1,193 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel -output-asm-variant=1 --show-encoding %s | FileCheck %s
+
+// CHECK:  aadd qword ptr [rbp + 8*r14 + 268435456], r9
+// CHECK: encoding: [0x4e,0x0f,0x38,0xfc,0x8c,0xf5,0x00,0x00,0x00,0x10]
+   aadd qword ptr [rbp + 8*r14 + 268435456], r9
+
+// CHECK:  aadd qword ptr [r8 + 4*rax + 291], r9
+// CHECK: encoding: [0x4d,0x0f,0x38,0xfc,0x8c,0x80,0x23,0x01,0x00,0x00]
+   aadd qword ptr [r8 + 4*rax + 291], r9
+
+// CHECK:  aadd qword ptr [rip], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x0d,0x00,0x00,0x00,0x00]
+   aadd qword ptr [rip], r9
+
+// CHECK:  aadd qword ptr [2*rbp - 512], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x0c,0x6d,0x00,0xfe,0xff,0xff]
+   aadd qword ptr [2*rbp - 512], r9
+
+// CHECK:  aadd qword ptr [rcx + 2032], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x89,0xf0,0x07,0x00,0x00]
+   aadd qword ptr [rcx + 2032], r9
+
+// CHECK:  aadd qword ptr [rdx - 2048], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x8a,0x00,0xf8,0xff,0xff]
+   aadd qword ptr [rdx - 2048], r9
+
+// CHECK:  aadd dword ptr [esp + 8*esi + 268435456], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x9c,0xf4,0x00,0x00,0x00,0x10]
+   aadd dword ptr [esp + 8*esi + 268435456], ebx
+
+// CHECK:  aadd dword ptr [edi + 4*eax + 291], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x9c,0x87,0x23,0x01,0x00,0x00]
+   aadd dword ptr [edi + 4*eax + 291], ebx
+
+// CHECK:  aadd dword ptr [eax], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x18]
+   aadd dword ptr [eax], ebx
+
+// CHECK:  aadd dword ptr [2*ebp - 512], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x1c,0x6d,0x00,0xfe,0xff,0xff]
+   aadd dword ptr [2*ebp - 512], ebx
+
+// CHECK:  aadd dword ptr [ecx + 2032], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x99,0xf0,0x07,0x00,0x00]
+   aadd dword ptr [ecx + 2032], ebx
+
+// CHECK:  aadd dword ptr [edx - 2048], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x9a,0x00,0xf8,0xff,0xff]
+   aadd dword ptr [edx - 2048], ebx
+
+// CHECK:  aand qword ptr [rbp + 8*r14 + 268435456], r9
+// CHECK: encoding: [0x66,0x4e,0x0f,0x38,0xfc,0x8c,0xf5,0x00,0x00,0x00,0x10]
+   aand qword ptr [rbp + 8*r14 + 268435456], r9
+
+// CHECK:  aand qword ptr [r8 + 4*rax + 291], r9
+// CHECK: encoding: [0x66,0x4d,0x0f,0x38,0xfc,0x8c,0x80,0x23,0x01,0x00,0x00]
+   aand qword ptr [r8 + 4*rax + 291], r9
+
+// CHECK:  aand qword ptr [rip], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x0d,0x00,0x00,0x00,0x00]
+   aand qword ptr [rip], r9
+
+// CHECK:  aand qword ptr [2*rbp - 512], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x0c,0x6d,0x00,0xfe,0xff,0xff]
+   aand qword ptr [2*rbp - 512], r9
+
+// CHECK:  aand qword ptr [rcx + 2032], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x89,0xf0,0x07,0x00,0x00]
+   aand qword ptr [rcx + 2032], r9
+
+// CHECK:  aand qword ptr [rdx - 2048], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x8a,0x00,0xf8,0xff,0xff]
+   aand qword ptr [rdx - 2048], r9
+
+// CHECK:  aand dword ptr [esp + 8*esi + 268435456], ebx
+// CHECK: encoding: [0x67,0x66,0x0f,0x38,0xfc,0x9c,0xf4,0x00,0x00,0x00,0x10]
+   aand dword ptr [esp + 8*esi + 268435456], ebx
+
+// CHECK:  aand dword ptr [edi + 4*eax + 291], ebx
+// CHECK: encoding: [0x67,0x66,0x0f,0x38,0xfc,0x9c,0x87,0x23,0x01,0x00,0x00]
+   aand dword ptr [edi + 4*eax + 291], ebx
+
+// CHECK:  aand dword ptr [eax], ebx
+// CHECK: encoding: [0x67,0x66,0x0f,0x

[PATCH] D136031: [DirectX backend] support ConstantBuffer to DXILResource.h

2022-10-15 Thread Xiang Li via Phabricator via cfe-commits

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

arc diff `git merge-base HEAD origin` --update D136031 
 to fix the apply patch issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136031

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/test/CodeGenHLSL/builtins/RWBuffer-annotations.hlsl
  clang/test/CodeGenHLSL/cbuf.hlsl
  llvm/include/llvm/Frontend/HLSL/HLSLResource.h
  llvm/lib/Frontend/HLSL/HLSLResource.cpp
  llvm/lib/Target/DirectX/CBufferDataLayout.cpp
  llvm/lib/Target/DirectX/CBufferDataLayout.h
  llvm/lib/Target/DirectX/CMakeLists.txt
  llvm/lib/Target/DirectX/DXILResource.cpp
  llvm/lib/Target/DirectX/DXILResource.h
  llvm/test/CodeGen/DirectX/UAVMetadata.ll
  llvm/test/CodeGen/DirectX/cbuf.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_0.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_1.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_2.ll
  llvm/test/CodeGen/DirectX/legacy_cb_layout_3.ll

Index: llvm/test/CodeGen/DirectX/legacy_cb_layout_3.ll
===
--- /dev/null
+++ llvm/test/CodeGen/DirectX/legacy_cb_layout_3.ll
@@ -0,0 +1,81 @@
+; RUN: opt -S -dxil-metadata-emit < %s | FileCheck %s --check-prefix=DXILMD
+
+target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64"
+target triple = "dxil-unknown-shadermodel6.7-library"
+
+; cbuffer D
+; {
+;
+;   struct D
+;   {
+;
+;   int D0;   ; Offset:0
+;   struct struct.B
+;   {
+;
+;   double B0;; Offset:   16
+;   float3 B1;; Offset:   32
+;   float B2; ; Offset:   44
+;   double3 B3;   ; Offset:   48
+;   half B4;  ; Offset:   72
+;   double2 B5;   ; Offset:   80
+;   float B6; ; Offset:   96
+;   half3 B7; ; Offset:  100
+;   half3 B8; ; Offset:  106
+;   
+;   } D1; ; Offset:   16
+;
+;   half D2;  ; Offset:  112
+;   struct struct.C
+;   {
+;
+;   struct struct.A
+;   {
+;
+;   float A0; ; Offset:  128
+;   double A1;; Offset:  136
+;   float A2; ; Offset:  144
+;   half A3;  ; Offset:  148
+;   int16_t A4;   ; Offset:  150
+;   int64_t A5;   ; Offset:  152
+;   int A6;   ; Offset:  160
+;   
+;   } C0; ; Offset:  128
+;
+;   float C1[1];  ; Offset:  176
+;   struct struct.B
+;   {
+;
+;   double B0;; Offset:  192
+;   float3 B1;; Offset:  208
+;   float B2; ; Offset:  220
+;   double3 B3;   ; Offset:  224
+;   half B4;  ; Offset:  248
+;   double2 B5;   ; Offset:  256
+;   float B6; ; Offset:  272
+;   half3 B7; ; Offset:  276
+;   half3 B8; ; Offset:  282
+;   
+;   } C2[2];; ; Offset:  192
+;
+;   half C3;  ; Offset:  384
+;   
+;   } D3; ; Offset:  128
+;
+;   double D4;; Offset:  392
+;   
+;   } D;  ; Offset:0 Size:   400
+
+
+; Make sure the size is 400
+; DXILMD:!{i32 0, ptr @D.cb., !"", i32 0, i32 1, i32 1, i32 400}
+
+
+%struct.B = type <{ double, <3 x float>, float, <3 x double>, half, <2 x double>, float, <3 x half>, <3 x half> }>
+%struct.C = type <{ %struct.A, [1 x float], [2 x %struct.B], half }>
+%struct.A = type <{ float, double, float, half, i16, i64, i32 }>
+
+@D.cb. = external local_unnamed_addr constant { i32, %struct.B, half, %struct.C, double }
+
+!hlsl.cbufs = !{!0}
+!0 = !{ptr @D.cb., !"D.cb.ty", i32 0, i32 13, i32 1, i32 0}
Index: llvm/test/CodeGen/DirectX/legacy_cb_layout_2.ll

[PATCH] D109460: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

2022-10-15 Thread Caleb Zulawski via Phabricator via cfe-commits

calebzulawski added a comment.
Herald added a subscriber: MaskRay.
Herald added a project: All.

I'm interested in this feature--is there any reason this wasn't completed? If 
not, I might be able to take over implementing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109460

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

[clang-tools-extra] e323a90 - [clangd] Use std::decay_t (NFC)

2022-10-15 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2022-10-15T14:59:23-07:00
New Revision: e323a904dce5bf248affef811791f4f8d4d45f02

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

LOG: [clangd] Use std::decay_t (NFC)

Added: 


Modified: 
clang-tools-extra/clangd/support/Context.h
clang-tools-extra/clangd/support/Function.h

Removed: 




diff  --git a/clang-tools-extra/clangd/support/Context.h 
b/clang-tools-extra/clangd/support/Context.h
index 926add18d88aa..2f403aebd3bba 100644
--- a/clang-tools-extra/clangd/support/Context.h
+++ b/clang-tools-extra/clangd/support/Context.h
@@ -155,7 +155,7 @@ class Context {
   };
 
   template  class TypedAnyStorage : public Context::AnyStorage {
-static_assert(std::is_same::type, T>::value,
+static_assert(std::is_same, T>::value,
   "Argument to TypedAnyStorage must be decayed");
 
   public:
@@ -200,7 +200,7 @@ class [[nodiscard]] WithContext {
 class [[nodiscard]] WithContextValue {
 public:
   template 
-  WithContextValue(const Key &K, typename std::decay::type V)
+  WithContextValue(const Key &K, std::decay_t V)
   : Restore(Context::current().derive(K, std::move(V))) {}
 
   // Anonymous values can be used for the destructor side-effect.

diff  --git a/clang-tools-extra/clangd/support/Function.h 
b/clang-tools-extra/clangd/support/Function.h
index dc9216bc53753..5437729d91b62 100644
--- a/clang-tools-extra/clangd/support/Function.h
+++ b/clang-tools-extra/clangd/support/Function.h
@@ -93,7 +93,7 @@ template  class Event {
   }
 
 private:
-  static_assert(std::is_same::type, T>::value,
+  static_assert(std::is_same, T>::value,
 "use a plain type: event values are always passed by const&");
 
   std::recursive_mutex ListenersMu;



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

[clang] ccd89be - [Driver] Remove legacy cc1 alias -mlink-cuda-bitcode

2022-10-15 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2022-10-15T14:02:13-07:00
New Revision: ccd89be64567697d6d09a3a74e08eab81080d567

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

LOG: [Driver] Remove legacy cc1 alias -mlink-cuda-bitcode

r340193 added -mlink-builtin-bitcode as the canonical spelling.

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/test/CodeGenCUDA/link-device-bitcode.cu

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index ef8d6f59d0985..a4c6f205ed611 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5455,8 +5455,6 @@ def mlink_bitcode_file : Separate<["-"], 
"mlink-bitcode-file">,
 def mlink_builtin_bitcode : Separate<["-"], "mlink-builtin-bitcode">,
   HelpText<"Link and internalize needed symbols from the given bitcode file "
"before performing optimizations.">;
-def mlink_cuda_bitcode : Separate<["-"], "mlink-cuda-bitcode">,
-  Alias;
 def vectorize_loops : Flag<["-"], "vectorize-loops">,
   HelpText<"Run the Loop vectorization passes">,
   MarshallingInfoFlag>;

diff  --git a/clang/test/CodeGenCUDA/link-device-bitcode.cu 
b/clang/test/CodeGenCUDA/link-device-bitcode.cu
index 0c6993da1038e..e650d6f62c25d 100644
--- a/clang/test/CodeGenCUDA/link-device-bitcode.cu
+++ b/clang/test/CodeGenCUDA/link-device-bitcode.cu
@@ -15,12 +15,6 @@
 // RUN:-disable-llvm-passes -o - %s \
 // RUN:| FileCheck %s -check-prefix CHECK-IR
 
-// Make sure legacy flag name works
-// RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
-// RUN:-mlink-cuda-bitcode %t.bc  -emit-llvm \
-// RUN:-disable-llvm-passes -o - %s \
-// RUN:| FileCheck %s -check-prefix CHECK-IR
-//
 // Make sure we can link two bitcode files.
 // RUN: %clang_cc1 -triple nvptx-unknown-cuda -fcuda-is-device \
 // RUN:-mlink-builtin-bitcode %t.bc -mlink-builtin-bitcode %t-2.bc \



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

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary)

2022-10-15 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

> I assume with this patch landed, many such cases may change code behavior. So 
> we will need to update msan to have a tool to detect cases like this anyway.

I believe that updated msan should come first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410

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

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-10-15 Thread Nikita Popov via Phabricator via cfe-commits

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

The default switch has happened, so unblocking this.




Comment at: clang/test/CodeGenOpenCL/overload.cl:23
   generic int *local *genloc;
   generic int *global *genglob;
+  // CHECK-DAG: call spir_func void @_Z3fooPU3AS1iS0_(i32 addrspace(1)* 
noundef {{.*}}, i32 addrspace(1)* noundef {{.*}})

Maybe initialize the relevant variables instead? I'd assume just NULL would 
work here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary)

2022-10-15 Thread Vitaly Buka via Phabricator via cfe-commits

vitalybuka added a reviewer: vitalybuka.
vitalybuka added a comment.

In D134410#3817190 , @vitalybuka 
wrote:

>> Also I applied this patch with D134698  
>> and used on our large test set, expecting significant number of pre-existing 
>> reports. To my surprise I see not many of them.
>
> Something wrong with my msan experiment, I'll re-evaluate size and reports 
> tomorrow.

Finally I had a time to fix my msan experiment D134698 

It's about 5.5% .text savings for Msan, and 10% for msan with "track origins", 
which is pretty good.
For context, for msan it's usually cheaper to report uninitialized ASAP, then 
propagating and report it later. With this metadata it will happen immediately 
after load.

However cleanup looks scary. Msan reports maybe 20% of unique tests on our code 
base. Many a of them share root cause, but still many unique root causes.
On quick looks I see no false report. A lot of stuff like this 
https://stackoverflow.com/questions/60112841/copying-structs-with-uninitialized-members
 which is technically is UB.
I assume with this patch landed, many such cases may change code behavior. So 
we will need to update msan to have a tool to detect cases like this anyway.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410

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

[PATCH] D136023: [CMake] Disable BOLT instrumentation of Clang on instrumented build

2022-10-15 Thread Amir Ayupov via Phabricator via cfe-commits

Amir updated this revision to Diff 468030.
Amir added a comment.

Add BOLT-PGO cmake cache file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136023

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/BOLT-PGO.cmake


Index: clang/cmake/caches/BOLT-PGO.cmake
===
--- /dev/null
+++ clang/cmake/caches/BOLT-PGO.cmake
@@ -0,0 +1,11 @@
+set(LLVM_ENABLE_PROJECTS "bolt;clang;lld" CACHE STRING "")
+
+set(CLANG_BOOTSTRAP_TARGETS
+  stage2-clang++-bolt
+  CACHE STRING "")
+set(BOOTSTRAP_CLANG_BOOTSTRAP_TARGETS
+  clang++-bolt
+  CACHE STRING "")
+
+set(PGO_BUILD_CONFIGURATION ${CMAKE_CURRENT_LIST_DIR}/BOLT.cmake CACHE STRING 
"")
+include(${CMAKE_CURRENT_LIST_DIR}/PGO.cmake)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -874,7 +874,7 @@
   endforeach()
 endif()
 
-if (CLANG_BOLT_INSTRUMENT)
+if (CLANG_BOLT_INSTRUMENT AND NOT LLVM_BUILD_INSTRUMENTED)
   set(CLANG_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
   set(CLANGXX_PATH ${CLANG_PATH}++)
   set(CLANG_INSTRUMENTED ${CLANG_PATH}-bolt.inst)


Index: clang/cmake/caches/BOLT-PGO.cmake
===
--- /dev/null
+++ clang/cmake/caches/BOLT-PGO.cmake
@@ -0,0 +1,11 @@
+set(LLVM_ENABLE_PROJECTS "bolt;clang;lld" CACHE STRING "")
+
+set(CLANG_BOOTSTRAP_TARGETS
+  stage2-clang++-bolt
+  CACHE STRING "")
+set(BOOTSTRAP_CLANG_BOOTSTRAP_TARGETS
+  clang++-bolt
+  CACHE STRING "")
+
+set(PGO_BUILD_CONFIGURATION ${CMAKE_CURRENT_LIST_DIR}/BOLT.cmake CACHE STRING "")
+include(${CMAKE_CURRENT_LIST_DIR}/PGO.cmake)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -874,7 +874,7 @@
   endforeach()
 endif()
 
-if (CLANG_BOLT_INSTRUMENT)
+if (CLANG_BOLT_INSTRUMENT AND NOT LLVM_BUILD_INSTRUMENTED)
   set(CLANG_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
   set(CLANGXX_PATH ${CLANG_PATH}++)
   set(CLANG_INSTRUMENTED ${CLANG_PATH}-bolt.inst)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0784de2 - [clang] Fix a warning

2022-10-15 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2022-10-15T12:50:34-07:00
New Revision: 0784de20e2ad5a91f07f952ae6d9887e380bccd6

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

LOG: [clang] Fix a warning

This patch fixes:

  clang/lib/Sema/SemaLambda.cpp:922:39: warning: suggest parentheses
  around ‘&&’ within ‘||’ [-Wparentheses]

Added: 


Modified: 
clang/lib/Sema/SemaLambda.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index e0a9091c4f2b0..f2ec96955975e 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -916,11 +916,11 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer 
&Intro,
   SourceLocation EndLoc;
   SmallVector Params;
 
-  assert(ParamInfo.getDeclSpec().getStorageClassSpec() ==
- DeclSpec::SCS_unspecified ||
- ParamInfo.getDeclSpec().getStorageClassSpec() ==
- DeclSpec::SCS_static &&
- "Unexpected storage specifier");
+  assert(
+  (ParamInfo.getDeclSpec().getStorageClassSpec() ==
+   DeclSpec::SCS_unspecified ||
+   ParamInfo.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static) 
&&
+  "Unexpected storage specifier");
   bool IsLambdaStatic =
   ParamInfo.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static;
 



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

[clang] a867cb8 - [clang] Fix a warning

2022-10-15 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2022-10-15T12:48:23-07:00
New Revision: a867cb849ad0ef2e56c63ce091b74aa0497beef8

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

LOG: [clang] Fix a warning

This patch fixes:

  clang/lib/Basic/SourceManager.cpp:2131:72: warning: suggest
  parentheses around ‘&&’ within ‘||’ [-Wparentheses]

Added: 


Modified: 
clang/lib/Basic/SourceManager.cpp

Removed: 




diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index 7229561394e32..6aae581be8001 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -2126,10 +2126,11 @@ std::pair 
SourceManager::isInTheSameTranslationUnit(
   // doesn't matter as these are never mixed in macro expansion.
   unsigned LParent = I->second.ParentFID.ID;
   unsigned RParent = Parent.ID;
-  assert((LOffs.second != ROffs.second) || (LParent == 0 || RParent == 0) 
||
- isInSameSLocAddrSpace(getComposedLoc(I->second.ParentFID, 0),
-   getComposedLoc(Parent, 0), nullptr) &&
- "Mixed local/loaded FileIDs with same include location?");
+  assert(((LOffs.second != ROffs.second) ||
+  (LParent == 0 || RParent == 0) ||
+  isInSameSLocAddrSpace(getComposedLoc(I->second.ParentFID, 0),
+getComposedLoc(Parent, 0), nullptr)) &&
+ "Mixed local/loaded FileIDs with same include location?");
   IsBeforeInTUCache.setCommonLoc(LOffs.first, LOffs.second, ROffs.second,
  LParent < RParent);
   return std::make_pair(



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

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-15 Thread David Rector via Phabricator via cfe-commits

davrec added inline comments.



Comment at: clang/include/clang/AST/DeclTemplate.h:3427
 
+TemplateParameterList *getReplacedTemplateParameterList(Decl *D);
+

mizvekov wrote:
> mizvekov wrote:
> > davrec wrote:
> > > davrec wrote:
> > > > I don't object with making this public, and I can see the argument for 
> > > > making this its own function rather than a Decl method all things being 
> > > > equal, but given that we already have 
> > > > `Decl::getDescribedTemplateParams`, I think that 
> > > > # this should probably also go in Decl,
> > > > # a good comment is needed explaining the difference between the two 
> > > > (e.g. that the `D` is the template/template-like decl itself, not a 
> > > > templated decl as required by `getDescribedTemplateParams`, or 
> > > > whatever), and what happens when it is called on a non-template decl, 
> > > > and
> > > > # it probably should be named just `getTemplateParams` or something 
> > > > that suggests its difference with `getDescribedTemplateParams`.
> > > > 
> > > On second thought this should definitely be a Decl method called 
> > > `getTemplateParameters()`, since all it does is dispatches to the derived 
> > > class's implementation of that.
> > I think this function shouldn't be public in that sense, it should only be 
> > used for implementation of retrieving parameter for Subst* nodes.
> > 
> > I don't think this should try to handle any other kinds of decls which 
> > can't appear as the AssociatedDecl in a Subst node.
> > 
> > There will be Subst specific changes here in the future, but which depend 
> > on some other enablers:
> > * We need to make Subst nodes for variable templates store the 
> > SpecializationDecl instead of the TemplateDecl, but this requires changing 
> > the order between creating the specialization and performing the 
> > substitution. I will do that in a separate patch.
> > * We will stop creating Subst* nodes for things we already performed 
> > substitution with sugar. And so, we won't need to handle alias templates, 
> > conceptdecls, etc. Subst nodes will only be used for things which we track 
> > specializations for, and that need resugaring.
> > 
> > It's only made 'public' because now we are using it across far away places 
> > like `Type.cpp` and `ExprCXX.cpp` and `TemplateName.cpp`.
> > 
> > I didn't think this needs to go as a Decl member, because of limited scope 
> > and because it only ever needs to access public members.
> > I don't think this justifies either a separate header to be included only 
> > where it's needed.
> > 
> > One option is to further make the name more specific, by adding `Internal` 
> > to the name for example.
> > Any other ideas?
> I ended up simply documenting that this is an internal function, for now.
That works for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D136023: [CMake] Disable BOLT instrumentation of Clang on instrumented build

2022-10-15 Thread Amir Ayupov via Phabricator via cfe-commits

Amir created this revision.
Amir added reviewers: phosek, nikic, beanz.
Herald added a subscriber: wenlei.
Herald added a project: All.
Amir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This enables multi-stage PGO build optimized by BOLT using BOLT.cmake cache.

The issue is that `-DPGO_BUILD_CONFIGURATION` cache file is passed to both
stage2-instrumented and stage2-optimized builds (for them to be identical),
but in case of BOLT.cmake, it doesn't make sense to BOLT-instrument the
instrumented binary (it's not going to be optimized). Hence turn off 
`CLANG_BOLT_INSTRUMENT` code if `LLVM_BUILD_INSTRUMENTED` is enabled.

The final workflow that enables multi-stage InstrPGO+ThinLTO+BOLT Clang build:

  cmake /llvm -GNinja -DCMAKE_BUILD_TYPE=Release \
-DLLVM_ENABLE_PROJECTS="bolt;clang;lld" -DLLVM_ENABLE_LLD=ON \
-DBOOTSTRAP_LLVM_ENABLE_LLD=ON -DBOOTSTRAP_BOOTSTRAP_LLVM_ENABLE_LLD=ON \
-DPGO_INSTRUMENT_LTO=Thin  -DCLANG_BOOTSTRAP_TARGETS="stage2-clang++-bolt" \
-DBOOTSTRAP_CLANG_BOOTSTRAP_TARGETS=clang++-bolt \

-DPGO_BUILD_CONFIGURATION=/path/to/llvm-project/clang/cmake/caches/BOLT.cmake \
-C llvm-project/clang/cmake/caches/PGO.cmake
  ninja stage2-clang++-bolt


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136023

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -874,7 +874,7 @@
   endforeach()
 endif()
 
-if (CLANG_BOLT_INSTRUMENT)
+if (CLANG_BOLT_INSTRUMENT AND NOT LLVM_BUILD_INSTRUMENTED)
   set(CLANG_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
   set(CLANGXX_PATH ${CLANG_PATH}++)
   set(CLANG_INSTRUMENTED ${CLANG_PATH}-bolt.inst)


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -874,7 +874,7 @@
   endforeach()
 endif()
 
-if (CLANG_BOLT_INSTRUMENT)
+if (CLANG_BOLT_INSTRUMENT AND NOT LLVM_BUILD_INSTRUMENTED)
   set(CLANG_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
   set(CLANGXX_PATH ${CLANG_PATH}++)
   set(CLANG_INSTRUMENTED ${CLANG_PATH}-bolt.inst)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135951: [X86] SUPPORT RAO-INT

2022-10-15 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31799
 
+  // We can lower add/sub/or/xor/and into RAO-INT instructions when the result
+  // is unused.

This should probably be in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

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

[PATCH] D135951: [X86] SUPPORT RAO-INT

2022-10-15 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:259
+ "Support RAO-INT instructions",
+ [FeatureSSE2]>;
 def FeatureINVPCID : SubtargetFeature<"invpcid", "HasINVPCID", "true",

Why do these require SSE2?



Comment at: llvm/lib/Target/X86/X86ISelLowering.h:801
+RXOR,
+RAND,
+

RKSimon wrote:
> very pedantic, but are these likely to get confused with ROR / RAND 
> instructions? Would it be better to use a RAO_ prefix?
Why not AADD etc to match the instruction names?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

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

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-15 Thread Evgeny Shulgin via Phabricator via cfe-commits

Izaron added a comment.

We can use this file as an example: `ConstProfiler.cpp` 
https://reviews.llvm.org/P8296
The code does a really big constant evaluation.

This is the generated json **before** the patch: `before.json` 
https://reviews.llvm.org/P8294
Giving this json to https://www.speedscope.app/, we get a big empty gap where 
the constant evaluation is being done.
link to screenshot 


This is the generated json **after** the patch: `after.json` 
https://reviews.llvm.org/P8295
The empty gap is filled with four events.
link to screenshot 


You can use this command for generating the json:

  clang++ ConstProfiler.cpp -std=c++20 -c -ftime-trace 
-ftime-trace-granularity=50

P.S. I'm not sure how to write tests to cover the patch, but I'll find this out 
=)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136022

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

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-15 Thread Evgeny Shulgin via Phabricator via cfe-commits

Izaron created this revision.
Izaron added reviewers: aaron.ballman, cor3ntin, anton-afanasyev, mbs-modular, 
jloser.
Herald added a project: All.
Izaron requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add time profiler for various constexpr evaluation events
so that slow event could be visible on the visualized flame chart.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136022

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -55,6 +55,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/TypeSize.h"
 
 using namespace clang;
@@ -17538,8 +17539,14 @@
   Expr::EvalResult Eval;
   Eval.Diag = &Notes;
   ConstantExpr *CE = Candidate.getPointer();
-  bool Result = CE->EvaluateAsConstantExpr(
-  Eval, SemaRef.getASTContext(), ConstantExprKind::ImmediateInvocation);
+  bool Result;
+  {
+llvm::TimeTraceScope TimeScope("EvaluateAsConstantExpr", [&] {
+  return CE->getSourceRange().printToString(SemaRef.getSourceManager());
+});
+Result = CE->EvaluateAsConstantExpr(Eval, SemaRef.getASTContext(),
+ConstantExprKind::ImmediateInvocation);
+  }
   if (!Result || !Notes.empty()) {
 Expr *InnerExpr = CE->getSubExpr()->IgnoreImplicit();
 if (auto *FunctionalCast = dyn_cast(InnerExpr))
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -41,10 +41,11 @@
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/TimeProfiler.h"
 #include 
 #include 
 
@@ -1736,6 +1737,14 @@
 // This implements C++11 [dcl.constexpr]p3,4, as amended by DR1360.
 bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD,
 CheckConstexprKind Kind) {
+  llvm::TimeTraceScope TimeScope("CheckConstexprFunctionDefinition", [&] {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+NewFD->getNameForDiagnostic(OS, Context.getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   const CXXMethodDecl *MD = dyn_cast(NewFD);
   if (MD && MD->isInstance()) {
 // C++11 [dcl.constexpr]p4:
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -56,6 +56,7 @@
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -15247,6 +15248,13 @@
   assert(!isValueDependent() &&
  "Expression evaluator can't be called on a dependent expression.");
 
+  llvm::TimeTraceScope TimeScope("EvaluateAsInitializer", [&] {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->printQualifiedName(OS);
+return Name;
+  });
+
   // FIXME: Evaluating initializers for large array and record types can cause
   // performance problems. Only do so in C++11 for now.
   if (isPRValue() && (getType()->isArrayType() || getType()->isRecordType()) &&


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -55,6 +55,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/TypeSize.h"
 
 using namespace clang;
@@ -17538,8 +17539,14 @@
   Expr::EvalResult Eval;
   Eval.Diag = &Notes;
   ConstantExpr *CE = Candidate.getPointer();
-  bool Result = CE->EvaluateAsConstantExpr(
-  Eval, SemaRef.getASTContext(), ConstantExprKind::ImmediateInvocation);
+  bool Result;
+  {
+llvm::TimeTraceScope TimeScope("EvaluateAsConstantExpr", [&] {
+  return CE->getSourceRange().printToString(SemaRef.getSourceManager());
+});
+Result = CE->EvaluateAsConstantExpr(Eval, SemaRef.getASTContext(),
+ConstantExprKind::ImmediateInvocation);
+  }
   if (!Result || !Notes.empty()) {
 Expr *InnerExpr = CE->getSubExpr()->IgnoreImplicit();
 if (auto *FunctionalCast = dyn_cast(InnerExpr))
Index: clang/lib/Sema/SemaDeclCXX.cpp

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

2022-10-15 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > ychen wrote:
> > > > mizvekov wrote:
> > > > > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > > > > 
> > > > > The injected arguments are 'Converted' arguments, and the 
> > > > > transformation above, by unpacking the arguments, is reversing just a 
> > > > > tiny part of the conversion process.
> > > > > 
> > > > > It's not very meaningful to canonicalize the arguments as written to 
> > > > > perform a semantic comparison, as that works well just for some kinds 
> > > > > of template arguments, like types and templates, but not for other 
> > > > > kinds in which the conversion process is not trivial.
> > > > > 
> > > > > For example, I think this may fail to compare the same integers 
> > > > > written in different ways, like `2` vs `1 + 1`.
> > > > Indeed. It can happen only when comparing one partial specialization 
> > > > with another. I think the standard does not require an implementation 
> > > > to deal with this but we could use the best effort without much 
> > > > overhead. For `2` vs `1+1` or similar template arguments that are not 
> > > > dependent, we could assume the equivalence because they wouldn't be in 
> > > > the partial ordering stage if they're not equivalent. For more 
> > > > complicated cases like `J+2` vs `J+1+1` where J is NTTP, let's stop 
> > > > trying (match GCC) because the overhead is a little bit high. 
> > > But I think the 'TemplateArgs', which are the specialization arguments 
> > > and are available through `getTemplateArgs()`, are the converted 
> > > arguments you want here, ie the AsWritten arguments converted against the 
> > > template.
> > > 
> > > I don't see why you can't just use that.
> > > 
> > > How about we change:
> > > ```
> > >   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> > > return nullptr;
> > > ```
> > > 
> > > Into:
> > > 
> > > ```
> > >   {
> > > ArrayRef Args1 = P1->getTemplateArgs().asArray(), 
> > > Args2;
> > > if constexpr (IsMoreSpecialThanPrimaryCheck)
> > >   Args2 = P2->getInjectedTemplateArgs();
> > > else
> > >   Args2 = P2->getTemplateArgs().asArray();
> > > 
> > > if (Args1.size() != Args2.size())
> > >   return nullptr;
> > > 
> > > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> > >   TemplateArgument Arg2 = Args2[I];
> > >   // Unlike the specialization arguments, the injected arguments are 
> > > not
> > >   // always canonical.
> > >   if constexpr (IsMoreSpecialThanPrimaryCheck)
> > > Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> > > 
> > >   // We use profile, instead of structural comparison of the 
> > > arguments,
> > >   // because canonicalization can't do the right thing for dependent
> > >   // expressions.
> > >   llvm::FoldingSetNodeID IDA, IDB;
> > >   Args1[I].Profile(IDA, S.Context);
> > >   Arg2.Profile(IDB, S.Context);
> > >   if (IDA != IDB)
> > > return nullptr;
> > > }
> > >   }
> > > ```
> > > 
> > > That should work, right?
> > Actually, you can even further simplify this.
> > 
> > You can't have two different specializations with the same specialization 
> > arguments. These arguments are used as the key to unique them anyway.
> > 
> > So simplify my above suggestion to:
> > ```
> >   if constexpr (IsMoreSpecialThanPrimaryCheck) {
> > ArrayRef Args1 = P1->getTemplateArgs().asArray(),
> >Args2 = P2->getInjectedTemplateArgs();
> > if (Args1.size() != Args2.size())
> >   return nullptr;
> > 
> > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> >   // We use profile, instead of structural comparison of the arguments,
> >   // because canonicalization can't do the right thing for dependent
> >   // expressions.
> >   llvm::FoldingSetNodeID IDA, IDB;
> >   Args1[I].Profile(IDA, S.Context);
> >   // Unlike the specialization arguments, the injected arguments are not
> >   // always canonical.
> >   S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB, 
> > S.Context);
> >   if (IDA != IDB)
> > return nullptr;
> > }
> >   }
> > ```
> Yet another improvement here is that you can do this check once, when we 
> create the specialization, and then simply store a 
> `hasSameArgumentsAsPrimaryInjected` flag.
> 
> When we create the specialization, we already profile the specialization 
> arguments anyway, so it's another computation you can avoid repeating.
> 
> Could even avoid repeating the profiling of the injected arguments if t

[PATCH] D135871: [clang-format][NFC] Handle language specific stuff at the top...

2022-10-15 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3121
+  return 1;
+if (Right.is(tok::period))
+  return 500;

owenpan wrote:
> Otherwise, it wouldn't be NFC.
It was Left == :: || Right == .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135871

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

[PATCH] D135951: [X86] SUPPORT RAO-INT

2022-10-15 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.h:801
+RXOR,
+RAND,
+

very pedantic, but are these likely to get confused with ROR / RAND 
instructions? Would it be better to use a RAO_ prefix?



Comment at: llvm/test/CodeGen/X86/atomic-instructions-32.ll:5
+; RUN: llc < %s -O0 -verify-machineinstrs -mtriple=x86_64-unknown-unknown | 
FileCheck %s --check-prefixes=X64-NO-RAOINT
+; RUN: llc < %s -O0 -verify-machineinstrs -mtriple=x86_64-unknown-unknown 
-mattr=+raoint | FileCheck %s --check-prefixes=X64-RAO-INT
+

Is the -O0 necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

___
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

2022-10-15 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> mizvekov wrote:
> > ychen wrote:
> > > mizvekov wrote:
> > > > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > > > 
> > > > The injected arguments are 'Converted' arguments, and the 
> > > > transformation above, by unpacking the arguments, is reversing just a 
> > > > tiny part of the conversion process.
> > > > 
> > > > It's not very meaningful to canonicalize the arguments as written to 
> > > > perform a semantic comparison, as that works well just for some kinds 
> > > > of template arguments, like types and templates, but not for other 
> > > > kinds in which the conversion process is not trivial.
> > > > 
> > > > For example, I think this may fail to compare the same integers written 
> > > > in different ways, like `2` vs `1 + 1`.
> > > Indeed. It can happen only when comparing one partial specialization with 
> > > another. I think the standard does not require an implementation to deal 
> > > with this but we could use the best effort without much overhead. For `2` 
> > > vs `1+1` or similar template arguments that are not dependent, we could 
> > > assume the equivalence because they wouldn't be in the partial ordering 
> > > stage if they're not equivalent. For more complicated cases like `J+2` vs 
> > > `J+1+1` where J is NTTP, let's stop trying (match GCC) because the 
> > > overhead is a little bit high. 
> > But I think the 'TemplateArgs', which are the specialization arguments and 
> > are available through `getTemplateArgs()`, are the converted arguments you 
> > want here, ie the AsWritten arguments converted against the template.
> > 
> > I don't see why you can't just use that.
> > 
> > How about we change:
> > ```
> >   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> > return nullptr;
> > ```
> > 
> > Into:
> > 
> > ```
> >   {
> > ArrayRef Args1 = P1->getTemplateArgs().asArray(), 
> > Args2;
> > if constexpr (IsMoreSpecialThanPrimaryCheck)
> >   Args2 = P2->getInjectedTemplateArgs();
> > else
> >   Args2 = P2->getTemplateArgs().asArray();
> > 
> > if (Args1.size() != Args2.size())
> >   return nullptr;
> > 
> > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> >   TemplateArgument Arg2 = Args2[I];
> >   // Unlike the specialization arguments, the injected arguments are not
> >   // always canonical.
> >   if constexpr (IsMoreSpecialThanPrimaryCheck)
> > Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> > 
> >   // We use profile, instead of structural comparison of the arguments,
> >   // because canonicalization can't do the right thing for dependent
> >   // expressions.
> >   llvm::FoldingSetNodeID IDA, IDB;
> >   Args1[I].Profile(IDA, S.Context);
> >   Arg2.Profile(IDB, S.Context);
> >   if (IDA != IDB)
> > return nullptr;
> > }
> >   }
> > ```
> > 
> > That should work, right?
> Actually, you can even further simplify this.
> 
> You can't have two different specializations with the same specialization 
> arguments. These arguments are used as the key to unique them anyway.
> 
> So simplify my above suggestion to:
> ```
>   if constexpr (IsMoreSpecialThanPrimaryCheck) {
> ArrayRef Args1 = P1->getTemplateArgs().asArray(),
>Args2 = P2->getInjectedTemplateArgs();
> if (Args1.size() != Args2.size())
>   return nullptr;
> 
> for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
>   // We use profile, instead of structural comparison of the arguments,
>   // because canonicalization can't do the right thing for dependent
>   // expressions.
>   llvm::FoldingSetNodeID IDA, IDB;
>   Args1[I].Profile(IDA, S.Context);
>   // Unlike the specialization arguments, the injected arguments are not
>   // always canonical.
>   S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB, 
> S.Context);
>   if (IDA != IDB)
> return nullptr;
> }
>   }
> ```
Yet another improvement here is that you can do this check once, when we create 
the specialization, and then simply store a `hasSameArgumentsAsPrimaryInjected` 
flag.

When we create the specialization, we already profile the specialization 
arguments anyway, so it's another computation you can avoid repeating.

Could even avoid repeating the profiling of the injected arguments if there was 
justified runtime overhead there, which I doubt, trading that off with 
increased memory use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128750

_

[PATCH] D136019: [clang][lex] Avoid `DirectoryLookup` copies

2022-10-15 Thread Juergen Ributzka via Phabricator via cfe-commits

ributzka added a comment.

We could invoke clang with the `-stats` option and compare the result against 
the expected number of stat calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136019

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

[PATCH] D136019: [clang][lex] Avoid `DirectoryLookup` copies

2022-10-15 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added a comment.

The change looks good to me. And based on the scope of D121685 
 it should be sufficient. Do you have any 
ideas for testing? If it would require significant testing infrastructure work, 
we can do that separately. But if you have some simple testing ideas in mind, 
that would be worth adding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136019

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

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-15 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.



Comment at: clang/include/clang/AST/DeclTemplate.h:3427
 
+TemplateParameterList *getReplacedTemplateParameterList(Decl *D);
+

mizvekov wrote:
> davrec wrote:
> > davrec wrote:
> > > I don't object with making this public, and I can see the argument for 
> > > making this its own function rather than a Decl method all things being 
> > > equal, but given that we already have `Decl::getDescribedTemplateParams`, 
> > > I think that 
> > > # this should probably also go in Decl,
> > > # a good comment is needed explaining the difference between the two 
> > > (e.g. that the `D` is the template/template-like decl itself, not a 
> > > templated decl as required by `getDescribedTemplateParams`, or whatever), 
> > > and what happens when it is called on a non-template decl, and
> > > # it probably should be named just `getTemplateParams` or something that 
> > > suggests its difference with `getDescribedTemplateParams`.
> > > 
> > On second thought this should definitely be a Decl method called 
> > `getTemplateParameters()`, since all it does is dispatches to the derived 
> > class's implementation of that.
> I think this function shouldn't be public in that sense, it should only be 
> used for implementation of retrieving parameter for Subst* nodes.
> 
> I don't think this should try to handle any other kinds of decls which can't 
> appear as the AssociatedDecl in a Subst node.
> 
> There will be Subst specific changes here in the future, but which depend on 
> some other enablers:
> * We need to make Subst nodes for variable templates store the 
> SpecializationDecl instead of the TemplateDecl, but this requires changing 
> the order between creating the specialization and performing the 
> substitution. I will do that in a separate patch.
> * We will stop creating Subst* nodes for things we already performed 
> substitution with sugar. And so, we won't need to handle alias templates, 
> conceptdecls, etc. Subst nodes will only be used for things which we track 
> specializations for, and that need resugaring.
> 
> It's only made 'public' because now we are using it across far away places 
> like `Type.cpp` and `ExprCXX.cpp` and `TemplateName.cpp`.
> 
> I didn't think this needs to go as a Decl member, because of limited scope 
> and because it only ever needs to access public members.
> I don't think this justifies either a separate header to be included only 
> where it's needed.
> 
> One option is to further make the name more specific, by adding `Internal` to 
> the name for example.
> Any other ideas?
I ended up simply documenting that this is an internal function, for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D136019: [clang][lex] Avoid `DirectoryLookup` copies

2022-10-15 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 created this revision.
jansvoboda11 added reviewers: ributzka, friss, vsapsai, arphaman.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes a performance regression introduced in D121685 
 that was caused by copying `DirectoryLookup`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136019

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -304,7 +304,7 @@
 
   // Look through the various header search paths to load any available module
   // maps, searching for a module map that describes this module.
-  for (DirectoryLookup Dir : search_dir_range()) {
+  for (DirectoryLookup &Dir : search_dir_range()) {
 if (Dir.isFramework()) {
   // Search for or infer a module map for a framework. Here we use
   // SearchName rather than ModuleName, to permit finding private modules


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -304,7 +304,7 @@
 
   // Look through the various header search paths to load any available module
   // maps, searching for a module map that describes this module.
-  for (DirectoryLookup Dir : search_dir_range()) {
+  for (DirectoryLookup &Dir : search_dir_range()) {
 if (Dir.isFramework()) {
   // Search for or infer a module map for a framework. Here we use
   // SearchName rather than ModuleName, to permit finding private modules
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135951: [X86][WIP] SUPPORT RAO-INT

2022-10-15 Thread Phoebe Wang via Phabricator via cfe-commits

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

Add atomic operations lowering for RAO-INT instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135951

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Headers/cpuid.h
  clang/test/Driver/x86-target-features.c
  clang/test/Preprocessor/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/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86InstrRAOINT.td
  llvm/test/CodeGen/X86/atomic-instructions-32.ll
  llvm/test/CodeGen/X86/atomic-instructions-64.ll
  llvm/test/MC/Disassembler/X86/rao-int.txt
  llvm/test/MC/Disassembler/X86/x86-64-rao-int.txt
  llvm/test/MC/X86/rao-int-att.s
  llvm/test/MC/X86/rao-int-intel.s
  llvm/test/MC/X86/x86-64-rao-int-att.s
  llvm/test/MC/X86/x86-64-rao-int-intel.s

Index: llvm/test/MC/X86/x86-64-rao-int-intel.s
===
--- /dev/null
+++ llvm/test/MC/X86/x86-64-rao-int-intel.s
@@ -0,0 +1,193 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel -output-asm-variant=1 --show-encoding %s | FileCheck %s
+
+// CHECK:  aadd qword ptr [rbp + 8*r14 + 268435456], r9
+// CHECK: encoding: [0x4e,0x0f,0x38,0xfc,0x8c,0xf5,0x00,0x00,0x00,0x10]
+   aadd qword ptr [rbp + 8*r14 + 268435456], r9
+
+// CHECK:  aadd qword ptr [r8 + 4*rax + 291], r9
+// CHECK: encoding: [0x4d,0x0f,0x38,0xfc,0x8c,0x80,0x23,0x01,0x00,0x00]
+   aadd qword ptr [r8 + 4*rax + 291], r9
+
+// CHECK:  aadd qword ptr [rip], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x0d,0x00,0x00,0x00,0x00]
+   aadd qword ptr [rip], r9
+
+// CHECK:  aadd qword ptr [2*rbp - 512], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x0c,0x6d,0x00,0xfe,0xff,0xff]
+   aadd qword ptr [2*rbp - 512], r9
+
+// CHECK:  aadd qword ptr [rcx + 2032], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x89,0xf0,0x07,0x00,0x00]
+   aadd qword ptr [rcx + 2032], r9
+
+// CHECK:  aadd qword ptr [rdx - 2048], r9
+// CHECK: encoding: [0x4c,0x0f,0x38,0xfc,0x8a,0x00,0xf8,0xff,0xff]
+   aadd qword ptr [rdx - 2048], r9
+
+// CHECK:  aadd dword ptr [esp + 8*esi + 268435456], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x9c,0xf4,0x00,0x00,0x00,0x10]
+   aadd dword ptr [esp + 8*esi + 268435456], ebx
+
+// CHECK:  aadd dword ptr [edi + 4*eax + 291], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x9c,0x87,0x23,0x01,0x00,0x00]
+   aadd dword ptr [edi + 4*eax + 291], ebx
+
+// CHECK:  aadd dword ptr [eax], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x18]
+   aadd dword ptr [eax], ebx
+
+// CHECK:  aadd dword ptr [2*ebp - 512], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x1c,0x6d,0x00,0xfe,0xff,0xff]
+   aadd dword ptr [2*ebp - 512], ebx
+
+// CHECK:  aadd dword ptr [ecx + 2032], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x99,0xf0,0x07,0x00,0x00]
+   aadd dword ptr [ecx + 2032], ebx
+
+// CHECK:  aadd dword ptr [edx - 2048], ebx
+// CHECK: encoding: [0x67,0x0f,0x38,0xfc,0x9a,0x00,0xf8,0xff,0xff]
+   aadd dword ptr [edx - 2048], ebx
+
+// CHECK:  aand qword ptr [rbp + 8*r14 + 268435456], r9
+// CHECK: encoding: [0x66,0x4e,0x0f,0x38,0xfc,0x8c,0xf5,0x00,0x00,0x00,0x10]
+   aand qword ptr [rbp + 8*r14 + 268435456], r9
+
+// CHECK:  aand qword ptr [r8 + 4*rax + 291], r9
+// CHECK: encoding: [0x66,0x4d,0x0f,0x38,0xfc,0x8c,0x80,0x23,0x01,0x00,0x00]
+   aand qword ptr [r8 + 4*rax + 291], r9
+
+// CHECK:  aand qword ptr [rip], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x0d,0x00,0x00,0x00,0x00]
+   aand qword ptr [rip], r9
+
+// CHECK:  aand qword ptr [2*rbp - 512], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x0c,0x6d,0x00,0xfe,0xff,0xff]
+   aand qword ptr [2*rbp - 512], r9
+
+// CHECK:  aand qword ptr [rcx + 2032], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x89,0xf0,0x07,0x00,0x00]
+   aand qword ptr [rcx + 2032], r9
+
+// CHECK:  aand qword ptr [rdx - 2048], r9
+// CHECK: encoding: [0x66,0x4c,0x0f,0x38,0xfc,0x8a,0x00,0xf8,0xff,0xff]
+   aand qword ptr [rdx - 2048], r9
+
+// CHECK:  aand dword ptr [esp + 8*esi + 268435456], ebx
+// CHECK: encoding: [0x67,0x66,0x0f,0x38,0xfc,0x9c,0xf4,0x00,0x00,0x00,0x10]
+   aand dword ptr [esp + 8*esi + 268435456], ebx
+
+// CHECK:  aand dword ptr [edi + 4*eax + 291], ebx
+// CHECK: encoding: [0x67,0x66,0x0f,0x38,0xfc,0x9c,0x87,0x23,0x01,0x00,0x00]
+   aand dwor

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-15 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.

Comment at: clang/include/clang/AST/DeclTemplate.h:3427

+TemplateParameterList *getReplacedTemplateParameterList(Decl *D);
+

davrec wrote:
> davrec wrote:
> > I don't object with making this public, and I can see the argument for 
> > making this its own function rather than a Decl method all things being 
> > equal, but given that we already have `Decl::getDescribedTemplateParams`, I 
> > think that 
> > # this should probably also go in Decl,
> > # a good comment is needed explaining the difference between the two (e.g. 
> > that the `D` is the template/template-like decl itself, not a templated 
> > decl as required by `getDescribedTemplateParams`, or whatever), and what 
> > happens when it is called on a non-template decl, and
> > # it probably should be named just `getTemplateParams` or something that 
> > suggests its difference with `getDescribedTemplateParams`.
> > 
> On second thought this should definitely be a Decl method called 
> `getTemplateParameters()`, since all it does is dispatches to the derived 
> class's implementation of that.
I think this function shouldn't be public in that sense, it should only be used 
for implementation of retrieving parameter for Subst* nodes.

I don't think this should try to handle any other kinds of decls which can't 
appear as the AssociatedDecl in a Subst node.

There will be Subst specific changes here in the future, but which depend on 
some other enablers:
* We need to make Subst nodes for variable templates store the 
SpecializationDecl instead of the TemplateDecl, but this requires changing the 
order between creating the specialization and performing the substitution. I 
will do that in a separate patch.
* We will stop creating Subst* nodes for things we already performed 
substitution with sugar. And so, we won't need to handle alias templates, 
conceptdecls, etc. Subst nodes will only be used for things which we track 
specializations for, and that need resugaring.

It's only made 'public' because now we are using it across far away places like 
`Type.cpp` and `ExprCXX.cpp` and `TemplateName.cpp`.

I didn't think this needs to go as a Decl member, because of limited scope and 
because it only ever needs to access public members.
I don't think this justifies either a separate header to be included only where 
it's needed.

One option is to further make the name more specific, by adding `Internal` to 
the name for example.
Any other ideas?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-15 Thread Yi Kong via Phabricator via cfe-commits

kongyi added a comment.

In D135097#3859917 , @jrtc27 wrote:

> It also will fail if the configured default target triple is one where the 
> default program address space is non-zero, and maybe other things (do any 
> targets do pre-IR name mangling for C?)

Our internal builder is broken due to this failing test, and I took the liberty 
to fix the test in commit f118280b04 
 by 
relaxing the variable name check. Feel free to change it if you want a 
different fix.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135097

___
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

2022-10-15 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> ychen wrote:
> > mizvekov wrote:
> > > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > > 
> > > The injected arguments are 'Converted' arguments, and the transformation 
> > > above, by unpacking the arguments, is reversing just a tiny part of the 
> > > conversion process.
> > > 
> > > It's not very meaningful to canonicalize the arguments as written to 
> > > perform a semantic comparison, as that works well just for some kinds of 
> > > template arguments, like types and templates, but not for other kinds in 
> > > which the conversion process is not trivial.
> > > 
> > > For example, I think this may fail to compare the same integers written 
> > > in different ways, like `2` vs `1 + 1`.
> > Indeed. It can happen only when comparing one partial specialization with 
> > another. I think the standard does not require an implementation to deal 
> > with this but we could use the best effort without much overhead. For `2` 
> > vs `1+1` or similar template arguments that are not dependent, we could 
> > assume the equivalence because they wouldn't be in the partial ordering 
> > stage if they're not equivalent. For more complicated cases like `J+2` vs 
> > `J+1+1` where J is NTTP, let's stop trying (match GCC) because the overhead 
> > is a little bit high. 
> But I think the 'TemplateArgs', which are the specialization arguments and 
> are available through `getTemplateArgs()`, are the converted arguments you 
> want here, ie the AsWritten arguments converted against the template.
> 
> I don't see why you can't just use that.
> 
> How about we change:
> ```
>   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> return nullptr;
> ```
> 
> Into:
> 
> ```
>   {
> ArrayRef Args1 = P1->getTemplateArgs().asArray(), Args2;
> if constexpr (IsMoreSpecialThanPrimaryCheck)
>   Args2 = P2->getInjectedTemplateArgs();
> else
>   Args2 = P2->getTemplateArgs().asArray();
> 
> if (Args1.size() != Args2.size())
>   return nullptr;
> 
> for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
>   TemplateArgument Arg2 = Args2[I];
>   // Unlike the specialization arguments, the injected arguments are not
>   // always canonical.
>   if constexpr (IsMoreSpecialThanPrimaryCheck)
> Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> 
>   // We use profile, instead of structural comparison of the arguments,
>   // because canonicalization can't do the right thing for dependent
>   // expressions.
>   llvm::FoldingSetNodeID IDA, IDB;
>   Args1[I].Profile(IDA, S.Context);
>   Arg2.Profile(IDB, S.Context);
>   if (IDA != IDB)
> return nullptr;
> }
>   }
> ```
> 
> That should work, right?
Actually, you can even further simplify this.

You can't have two different specializations with the same specialization 
arguments. These arguments are used as the key to unique them anyway.

So simplify my above suggestion to:
```
  if constexpr (IsMoreSpecialThanPrimaryCheck) {
ArrayRef Args1 = P1->getTemplateArgs().asArray(),
   Args2 = P2->getInjectedTemplateArgs();
if (Args1.size() != Args2.size())
  return nullptr;

for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
  // We use profile, instead of structural comparison of the arguments,
  // because canonicalization can't do the right thing for dependent
  // expressions.
  llvm::FoldingSetNodeID IDA, IDB;
  Args1[I].Profile(IDA, S.Context);
  // Unlike the specialization arguments, the injected arguments are not
  // always canonical.
  S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB, S.Context);
  if (IDA != IDB)
return nullptr;
}
  }
```


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

[clang] f118280 - [clang] Fix func-attr.c test

2022-10-15 Thread Yi Kong via cfe-commits


Author: Yi Kong
Date: 2022-10-15T23:16:26+09:00
New Revision: f118280b0407c40b1aacf40ad6777d777e333c88

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

LOG: [clang] Fix func-attr.c test

The test was introduced by 84a9ec2ff1ee. The check is over-specific and
is broken on the Android buildbot. Fixed by relaxing the variable name
check.

Added: 


Modified: 
clang/test/CodeGen/func-attr.c

Removed: 




diff  --git a/clang/test/CodeGen/func-attr.c b/clang/test/CodeGen/func-attr.c
index 6191130589fe6..e7c60f2d7abff 100644
--- a/clang/test/CodeGen/func-attr.c
+++ b/clang/test/CodeGen/func-attr.c
@@ -8,5 +8,5 @@ float foo(float a, float b) {
   return a+b;
 }
 
-// CHECK: define{{.*}} float @foo(float noundef %a, float noundef %b) 
[[FAST_ATTRS:#[0-9]+]]
+// CHECK: define{{.*}} float @foo(float noundef %{{.*}}, float noundef 
%{{.*}}) [[FAST_ATTRS:#[0-9]+]]
 // CHECK: attributes [[FAST_ATTRS]] = { {{.*}} "approx-func-fp-math"="true" 
{{.*}} "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" {{.*}} 
"unsafe-fp-math"="true"



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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

2022-10-15 Thread Whisperity via Phabricator via cfe-commits

whisperity added a comment.

In D91000#3770071 , @whisperity wrote:

> In D91000#3197851 , @aaron.ballman 
> wrote:
>
>> In terms of whether we should expose the check in C++: I think we shouldn't. 
>> [...]
>>
>> I think we should probably also not enable the check when the user compiles 
>> in C99 or earlier mode, because there is no Annex K available to provide 
>> replacement functions.
>
> @aaron.ballman I think the current version of the check satisfies these 
> conditions. It seems the check **will** run for every TU, but in case there 
> is no alternative the check could suggest, it will do nothing:
>
>   if (!ReplacementFunctionName)
> return;
>
> Is this good enough? This seems more future-proof than juggling the 
> `LangOpts` instance in yet another member function.

@aaron.ballman @njames93 Ping!
It seems @futogergely has resigned from the company, so I'll end up flying the 
approach, but the one above is the last outstanding question.


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

https://reviews.llvm.org/D91000

___
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

2022-10-15 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

ychen wrote:
> mizvekov wrote:
> > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > 
> > The injected arguments are 'Converted' arguments, and the transformation 
> > above, by unpacking the arguments, is reversing just a tiny part of the 
> > conversion process.
> > 
> > It's not very meaningful to canonicalize the arguments as written to 
> > perform a semantic comparison, as that works well just for some kinds of 
> > template arguments, like types and templates, but not for other kinds in 
> > which the conversion process is not trivial.
> > 
> > For example, I think this may fail to compare the same integers written in 
> > different ways, like `2` vs `1 + 1`.
> Indeed. It can happen only when comparing one partial specialization with 
> another. I think the standard does not require an implementation to deal with 
> this but we could use the best effort without much overhead. For `2` vs `1+1` 
> or similar template arguments that are not dependent, we could assume the 
> equivalence because they wouldn't be in the partial ordering stage if they're 
> not equivalent. For more complicated cases like `J+2` vs `J+1+1` where J is 
> NTTP, let's stop trying (match GCC) because the overhead is a little bit 
> high. 
But I think the 'TemplateArgs', which are the specialization arguments and are 
available through `getTemplateArgs()`, are the converted arguments you want 
here, ie the AsWritten arguments converted against the template.

I don't see why you can't just use that.

How about we change:
```
  if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
return nullptr;
```

Into:

```
  {
ArrayRef Args1 = P1->getTemplateArgs().asArray(), Args2;
if constexpr (IsMoreSpecialThanPrimaryCheck)
  Args2 = P2->getInjectedTemplateArgs();
else
  Args2 = P2->getTemplateArgs().asArray();

if (Args1.size() != Args2.size())
  return nullptr;

for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
  TemplateArgument Arg2 = Args2[I];
  // Unlike the specialization arguments, the injected arguments are not
  // always canonical.
  if constexpr (IsMoreSpecialThanPrimaryCheck)
Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);

  // We use profile, instead of structural comparison of the arguments,
  // because canonicalization can't do the right thing for dependent
  // expressions.
  llvm::FoldingSetNodeID IDA, IDB;
  Args1[I].Profile(IDA, S.Context);
  Arg2.Profile(IDB, S.Context);
  if (IDA != IDB)
return nullptr;
}
  }
```

That should work, right?


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] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-15 Thread Yusuke Kadowaki via Phabricator via cfe-commits

yusuke-kadowaki updated this revision to Diff 468015.
yusuke-kadowaki marked 2 inline comments as done.
yusuke-kadowaki added a comment.

- Rebased
- Remove more braces
- Update rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,224 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing OverEmptyLines
+  Style.MaxEmptyLinesToKeep = 3;
+  Style.AlignTrailingComments.OverEmptyLines = 2;
+  // Cannot use verifyFormat here
+  // test::messUp removes all new lines which changes the logic
+  EXPECT_EQ("#include \"a.h\" // comment\n"
+"\n"
+"\n"
+"\n"
+"#include \"ab.h\"  // comment\n"
+"\n"
+"\n"
+"#include \"abcdefg.h\" // comment\n",
+format("#include \"a.h\" // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "#include \"ab.h\" // comment\n"
+   "\n"
+   "\n"
+   "#include \"abcdefg.h\" // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  // End of testing OverEmptyLines
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int ba

[PATCH] D135013: [clang][Interp] Array initialization via ImplicitValueInitExpr

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ah it's the array filler which we don't handle at all right now :(


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

https://reviews.llvm.org/D135013

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

[PATCH] D136018: [Clang] Fix crash when checking misaligned member with dependent type

2022-10-15 Thread Jun Zhang via Phabricator via cfe-commits

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

If the type is dependent, we should just discard it and not checking its
alignment as it doesn't exisit yet.
Fixes https://github.com/llvm/llvm-project/issues/58370


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136018

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/misaligned-member-with-depdent-type.cpp


Index: clang/test/SemaCXX/misaligned-member-with-depdent-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/misaligned-member-with-depdent-type.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+struct __attribute__((packed)) {
+  unsigned options;
+  template 
+  void getOptions() {
+  (T *)&options;
+  }
+} s;
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -17393,12 +17393,13 @@
   cast(E)->getOpcode() == UO_AddrOf) {
 auto *Op = cast(E)->getSubExpr()->IgnoreParens();
 if (isa(Op)) {
-  auto MA = llvm::find(MisalignedMembers, MisalignedMember(Op));
+  auto *MA = llvm::find(MisalignedMembers, MisalignedMember(Op));
+  const bool IsDiscardMisalignedPointer =
+  T->isPointerType() &&
+  (T->getPointeeType()->isIncompleteType() || T->isDependentType() ||
+   Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment);
   if (MA != MisalignedMembers.end() &&
-  (T->isIntegerType() ||
-   (T->isPointerType() && (T->getPointeeType()->isIncompleteType() ||
-   Context.getTypeAlignInChars(
-   T->getPointeeType()) <= 
MA->Alignment
+  (T->isIntegerType() || IsDiscardMisalignedPointer))
 MisalignedMembers.erase(MA);
 }
   }


Index: clang/test/SemaCXX/misaligned-member-with-depdent-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/misaligned-member-with-depdent-type.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+struct __attribute__((packed)) {
+  unsigned options;
+  template 
+  void getOptions() {
+  (T *)&options;
+  }
+} s;
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -17393,12 +17393,13 @@
   cast(E)->getOpcode() == UO_AddrOf) {
 auto *Op = cast(E)->getSubExpr()->IgnoreParens();
 if (isa(Op)) {
-  auto MA = llvm::find(MisalignedMembers, MisalignedMember(Op));
+  auto *MA = llvm::find(MisalignedMembers, MisalignedMember(Op));
+  const bool IsDiscardMisalignedPointer =
+  T->isPointerType() &&
+  (T->getPointeeType()->isIncompleteType() || T->isDependentType() ||
+   Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment);
   if (MA != MisalignedMembers.end() &&
-  (T->isIntegerType() ||
-   (T->isPointerType() && (T->getPointeeType()->isIncompleteType() ||
-   Context.getTypeAlignInChars(
-   T->getPointeeType()) <= MA->Alignment
+  (T->isIntegerType() || IsDiscardMisalignedPointer))
 MisalignedMembers.erase(MA);
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-15 Thread David Rector via Phabricator via cfe-commits

davrec added inline comments.



Comment at: clang/include/clang/AST/DeclTemplate.h:3427
 
+TemplateParameterList *getReplacedTemplateParameterList(Decl *D);
+

davrec wrote:
> I don't object with making this public, and I can see the argument for making 
> this its own function rather than a Decl method all things being equal, but 
> given that we already have `Decl::getDescribedTemplateParams`, I think that 
> # this should probably also go in Decl,
> # a good comment is needed explaining the difference between the two (e.g. 
> that the `D` is the template/template-like decl itself, not a templated decl 
> as required by `getDescribedTemplateParams`, or whatever), and what happens 
> when it is called on a non-template decl, and
> # it probably should be named just `getTemplateParams` or something that 
> suggests its difference with `getDescribedTemplateParams`.
> 
On second thought this should definitely be a Decl method called 
`getTemplateParameters()`, since all it does is dispatches to the derived 
class's implementation of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

___
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

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Utilitze the existing support for temporary declarations to support 
`MaterializeTemporaryExpr`s for primitive values.


Repository:
  rG LLVM Github Monorepo

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
@@ -568,6 +568,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
@@ -92,6 +92,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
@@ -468,6 +468,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) {
+  Storag

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-15 Thread David Rector via Phabricator via cfe-commits

davrec added a comment.

I like the late changes, just need to add comments to the public methods, and 
maybe move `getReplacedTemplateParameterList` over to `Decl`.




Comment at: clang/include/clang/AST/DeclTemplate.h:3427
 
+TemplateParameterList *getReplacedTemplateParameterList(Decl *D);
+

I don't object with making this public, and I can see the argument for making 
this its own function rather than a Decl method all things being equal, but 
given that we already have `Decl::getDescribedTemplateParams`, I think that 
# this should probably also go in Decl,
# a good comment is needed explaining the difference between the two (e.g. that 
the `D` is the template/template-like decl itself, not a templated decl as 
required by `getDescribedTemplateParams`, or whatever), and what happens when 
it is called on a non-template decl, and
# it probably should be named just `getTemplateParams` or something that 
suggests its difference with `getDescribedTemplateParams`.




Comment at: clang/include/clang/AST/ExprCXX.h:4307
 
-  NonTypeTemplateParmDecl *getParameter() const {
-return ParamAndRef.getPointer();
+  Decl *getAssociatedDecl() const { return AssociatedDeclAndRef.getPointer(); }
+

Add comment



Comment at: clang/include/clang/AST/ExprCXX.h:4321
+
+  bool isReferenceParameter() const { return AssociatedDeclAndRef.getInt(); }
 

(Not your responsibility but while you're adding comments maybe you could add 
one for this too.)



Comment at: clang/include/clang/AST/ExprCXX.h:4378-4380
+  Decl *getAssociatedDecl() const { return AssociatedDecl; }
+
+  unsigned getIndex() const { return Index; }

Add comments



Comment at: clang/include/clang/AST/TemplateName.h:153-154
+
+  Decl *getAssociatedDecl() const { return AssociatedDecl; }
+  unsigned getIndex() const { return Bits.Index; }
 

Add comments



Comment at: clang/include/clang/AST/TemplateName.h:386-387
 public:
-  TemplateTemplateParmDecl *getParameter() const { return Parameter; }
+  Decl *getAssociatedDecl() const { return AssociatedDecl; }
+  unsigned getIndex() const { return Bits.Index; }
+

Add comments



Comment at: clang/lib/AST/DeclTemplate.cpp:1610
+  default:
+llvm_unreachable("Unhandled templated declaration kind");
+  }

Now that this is public, probably should return nullptr - otherwise users need 
the same huge switch just to decide whether they can call this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D136013: [clang][Interp] Fix ignoring expression return values

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 468006.

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

https://reviews.llvm.org/D136013

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp
  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
@@ -204,6 +204,26 @@
 // expected-error {{static assertion failed}} \
 // expected-note {{evaluates to '12 == 200'}}
 
+
+struct S {
+  int a = 0;
+  constexpr int get5() const { return 5; }
+  constexpr void fo() const {
+this; // expected-warning {{expression result unused}} \
+  // ref-warning {{expression result unused}}
+this->a; // expected-warning {{expression result unused}} \
+ // ref-warning {{expression result unused}}
+get5();
+  }
+
+  constexpr int m() const {
+fo();
+return 1;
+  }
+};
+constexpr S s;
+static_assert(s.m() == 1, "");
+
 namespace MI {
   class A {
   public:
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -279,6 +279,17 @@
   static_assert((12 | true) == 13, "");
 };
 
+
+#if __cplusplus >= 201402L
+constexpr bool IgnoredUnary() {
+  bool bo = true;
+  !bo; // expected-warning {{expression result unused}} \
+   // ref-warning {{expression result unused}}
+  return bo;
+}
+static_assert(IgnoredUnary(), "");
+#endif
+
 namespace floats {
   constexpr int i = 2;
   constexpr float f = 1.0f;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -383,6 +383,9 @@
 
 template 
 bool ByteCodeExprGen::VisitMemberExpr(const MemberExpr *E) {
+  if (DiscardResult)
+return true;
+
   // 'Base.Member'
   const Expr *Base = E->getBase();
   const ValueDecl *Member = E->getMemberDecl();
@@ -1050,8 +1053,6 @@
 if (Func->isFullyCompiled() && !Func->isConstexpr())
   return false;
 
-QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
-Optional T = classify(ReturnType);
 // Put arguments on the stack.
 for (const auto *Arg : E->arguments()) {
   if (!this->visit(Arg))
@@ -1061,7 +1062,18 @@
 // In any case call the function. The return value will end up on the stack and
 // if the function has RVO, we already have the pointer on the stack to write
 // the result into.
-return this->emitCall(Func, E);
+if (!this->emitCall(Func, E))
+  return false;
+
+QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
+if (DiscardResult && !ReturnType->isVoidType()) {
+  Optional T = classify(ReturnType);
+  if (T)
+return this->emitPop(*T, E);
+  // TODO: This is a RVO function and we need to ignore the return value.
+}
+
+return true;
   } else {
 assert(false && "We don't support non-FunctionDecl callees right now.");
   }
@@ -,11 +1123,16 @@
 
 template 
 bool ByteCodeExprGen::VisitCXXThisExpr(const CXXThisExpr *E) {
+  if (DiscardResult)
+return true;
   return this->emitThis(E);
 }
 
 template 
 bool ByteCodeExprGen::VisitUnaryOperator(const UnaryOperator *E) {
+  if (DiscardResult)
+return true;
+
   const Expr *SubExpr = E->getSubExpr();
 
   switch (E->getOpcode()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135989: [clang][Sema] Use size of char for void types

2022-10-15 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

Thanks for clarifying!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135989

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

[PATCH] D136013: [clang][Interp] Fix ignoring expression return values

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 468001.
tbaeder retitled this revision from "[clang][Interp] Fix ignoring unary 
operators and This exprs" to "[clang][Interp] Fix ignoring expression return 
values".

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

https://reviews.llvm.org/D136013

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp
  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
@@ -204,6 +204,26 @@
 // expected-error {{static assertion failed}} \
 // expected-note {{evaluates to '12 == 200'}}
 
+
+struct S {
+  int a = 0;
+  constexpr int get5() const { return 5; }
+  constexpr void fo() const {
+this; // expected-warning {{expression result unused}} \
+  // ref-warning {{expression result unused}}
+this->a; // expected-warning {{expression result unused}} \
+ // ref-warning {{expression result unused}}
+get5();
+  }
+
+  constexpr int m() const {
+fo();
+return 1;
+  }
+};
+constexpr S s;
+static_assert(s.m() == 1, "");
+
 namespace MI {
   class A {
   public:
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -279,6 +279,17 @@
   static_assert((12 | true) == 13, "");
 };
 
+
+#if __cplusplus >= 201402L
+constexpr bool IgnoredUnary() {
+  bool bo = true;
+  !bo; // expected-warning {{expression result unused}} \
+   // ref-warning {{expression result unused}}
+  return bo;
+}
+static_assert(IgnoredUnary(), "");
+#endif
+
 namespace floats {
   constexpr int i = 2;
   constexpr float f = 1.0f;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -383,6 +383,9 @@
 
 template 
 bool ByteCodeExprGen::VisitMemberExpr(const MemberExpr *E) {
+  if (DiscardResult)
+return true;
+
   // 'Base.Member'
   const Expr *Base = E->getBase();
   const ValueDecl *Member = E->getMemberDecl();
@@ -1050,8 +1053,6 @@
 if (Func->isFullyCompiled() && !Func->isConstexpr())
   return false;
 
-QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
-Optional T = classify(ReturnType);
 // Put arguments on the stack.
 for (const auto *Arg : E->arguments()) {
   if (!this->visit(Arg))
@@ -1061,7 +1062,16 @@
 // In any case call the function. The return value will end up on the stack and
 // if the function has RVO, we already have the pointer on the stack to write
 // the result into.
-return this->emitCall(Func, E);
+if (!this->emitCall(Func, E))
+  return false;
+
+QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
+if (DiscardResult && !ReturnType->isVoidType()) {
+  Optional T = classify(ReturnType);
+  return this->emitPop(*T, E);
+}
+
+return true;
   } else {
 assert(false && "We don't support non-FunctionDecl callees right now.");
   }
@@ -,11 +1121,16 @@
 
 template 
 bool ByteCodeExprGen::VisitCXXThisExpr(const CXXThisExpr *E) {
+  if (DiscardResult)
+return true;
   return this->emitThis(E);
 }
 
 template 
 bool ByteCodeExprGen::VisitUnaryOperator(const UnaryOperator *E) {
+  if (DiscardResult)
+return true;
+
   const Expr *SubExpr = E->getSubExpr();
 
   switch (E->getOpcode()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136012: [clang][Interp] Fix record members of reference type

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 468000.

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

https://reviews.llvm.org/D136012

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  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
@@ -88,3 +88,29 @@
   return j;
 }
 static_assert(RefToMemberExpr() == 11, "");
+
+struct Ref {
+  int &a;
+};
+
+constexpr int RecordWithRef() {
+  int m = 100;
+  Ref r{m};
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef() == 200, "");
+
+
+struct Ref2 {
+  int &a;
+  constexpr Ref2(int &a) : a(a) {}
+};
+
+constexpr int RecordWithRef2() {
+  int m = 100;
+  Ref2 r(m);
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef2() == 200, "");
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -478,9 +478,11 @@
   return true;
 }
 
+/// 1) Pops a pointer from the stack
+/// 2) Pushes the value of the pointer's field on the stack
 template ::T>
 bool GetField(InterpState &S, CodePtr OpPC, uint32_t I) {
-  const Pointer &Obj = S.Stk.peek();
+  const Pointer &Obj = S.Stk.pop();
   if (!CheckNull(S, OpPC, Obj, CSK_Field))
   return false;
   if (!CheckRange(S, OpPC, Obj, CSK_Field))
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -104,7 +104,7 @@
   if (const FieldDecl *Member = Init->getMember()) {
 const Record::Field *F = R->getField(Member);
 
-if (Optional T = this->classify(InitExpr->getType())) {
+if (Optional T = this->classify(InitExpr)) {
   if (!this->emitThis(InitExpr))
 return false;
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -397,6 +397,8 @@
 const Record *R = getRecord(RD);
 const Record::Field *F = R->getField(FD);
 // Leave a pointer to the field on the stack.
+if (F->Decl->getType()->isReferenceType())
+  return this->emitGetField(PT_Ptr, F->Offset, E);
 return this->emitGetPtrField(F->Offset, E);
   }
 
@@ -871,7 +873,7 @@
   if (!this->emitDupPtr(Initializer))
 return false;
 
-  if (Optional T = classify(Init->getType())) {
+  if (Optional T = classify(Init)) {
 if (!this->visit(Init))
   return false;
 


Index: clang/test/AST/Interp/references.cpp
===
--- clang/test/AST/Interp/references.cpp
+++ clang/test/AST/Interp/references.cpp
@@ -88,3 +88,29 @@
   return j;
 }
 static_assert(RefToMemberExpr() == 11, "");
+
+struct Ref {
+  int &a;
+};
+
+constexpr int RecordWithRef() {
+  int m = 100;
+  Ref r{m};
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef() == 200, "");
+
+
+struct Ref2 {
+  int &a;
+  constexpr Ref2(int &a) : a(a) {}
+};
+
+constexpr int RecordWithRef2() {
+  int m = 100;
+  Ref2 r(m);
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef2() == 200, "");
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -478,9 +478,11 @@
   return true;
 }
 
+/// 1) Pops a pointer from the stack
+/// 2) Pushes the value of the pointer's field on the stack
 template ::T>
 bool GetField(InterpState &S, CodePtr OpPC, uint32_t I) {
-  const Pointer &Obj = S.Stk.peek();
+  const Pointer &Obj = S.Stk.pop();
   if (!CheckNull(S, OpPC, Obj, CSK_Field))
   return false;
   if (!CheckRange(S, OpPC, Obj, CSK_Field))
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -104,7 +104,7 @@
   if (const FieldDecl *Member = Init->getMember()) {
 const Record::Field *F = R->getField(Member);
 
-if (Optional T = this->classify(InitExpr->getType())) {
+if (Optional T = this->classify(InitExpr)) {
   if (!this->emitThis(InitExpr))
 return false;
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -397,6 +397,8 @@
 const Record *R = getRecord(RD);
 const Record::Field *F = R->getField(FD);
 // Leave a pointer to the field on the stack.
+if (F->Decl->getType()->isReference

[PATCH] D136013: [clang][Interp] Fix ignoring unary operators and This exprs

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

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

Randomly noticed this. We need to honor DiscardResult here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136013

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp
  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
@@ -204,6 +204,21 @@
 // expected-error {{static assertion failed}} \
 // expected-note {{evaluates to '12 == 200'}}
 
+
+struct S {
+  constexpr void fo() const {
+this; // expected-warning {{expression result unused}} \
+  // ref-warning {{expression result unused}}
+  }
+
+  constexpr int m() const {
+fo();
+return 1;
+  }
+};
+constexpr S s;
+static_assert(s.m() == 1, "");
+
 namespace MI {
   class A {
   public:
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -279,6 +279,17 @@
   static_assert((12 | true) == 13, "");
 };
 
+
+#if __cplusplus >= 201402L
+constexpr bool IgnoredUnary() {
+  bool bo = true;
+  !bo; // expected-warning {{expression result unused}} \
+   // ref-warning {{expression result unused}}
+  return bo;
+}
+static_assert(IgnoredUnary(), "");
+#endif
+
 namespace floats {
   constexpr int i = 2;
   constexpr float f = 1.0f;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1132,11 +1132,16 @@
 
 template 
 bool ByteCodeExprGen::VisitCXXThisExpr(const CXXThisExpr *E) {
+  if (DiscardResult)
+return true;
   return this->emitThis(E);
 }
 
 template 
 bool ByteCodeExprGen::VisitUnaryOperator(const UnaryOperator *E) {
+  if (DiscardResult)
+return true;
+
   const Expr *SubExpr = E->getSubExpr();
 
   switch (E->getOpcode()) {


Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -204,6 +204,21 @@
 // expected-error {{static assertion failed}} \
 // expected-note {{evaluates to '12 == 200'}}
 
+
+struct S {
+  constexpr void fo() const {
+this; // expected-warning {{expression result unused}} \
+  // ref-warning {{expression result unused}}
+  }
+
+  constexpr int m() const {
+fo();
+return 1;
+  }
+};
+constexpr S s;
+static_assert(s.m() == 1, "");
+
 namespace MI {
   class A {
   public:
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -279,6 +279,17 @@
   static_assert((12 | true) == 13, "");
 };
 
+
+#if __cplusplus >= 201402L
+constexpr bool IgnoredUnary() {
+  bool bo = true;
+  !bo; // expected-warning {{expression result unused}} \
+   // ref-warning {{expression result unused}}
+  return bo;
+}
+static_assert(IgnoredUnary(), "");
+#endif
+
 namespace floats {
   constexpr int i = 2;
   constexpr float f = 1.0f;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1132,11 +1132,16 @@
 
 template 
 bool ByteCodeExprGen::VisitCXXThisExpr(const CXXThisExpr *E) {
+  if (DiscardResult)
+return true;
   return this->emitThis(E);
 }
 
 template 
 bool ByteCodeExprGen::VisitUnaryOperator(const UnaryOperator *E) {
+  if (DiscardResult)
+return true;
+
   const Expr *SubExpr = E->getSubExpr();
 
   switch (E->getOpcode()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136012: [clang][Interp] Fix record members of reference type

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

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

  When assigning to them, we can't classify the expression type, because
  that doesn't contain the right information.
  
  And when reading from them, we need to do the extra deref, just like we
  do when reading from a DeclRefExpr.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136012

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -88,3 +88,29 @@
   return j;
 }
 static_assert(RefToMemberExpr() == 11, "");
+
+struct Ref {
+  int &a;
+};
+
+constexpr int RecordWithRef() {
+  int m = 100;
+  Ref r{m};
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef() == 200, "");
+
+
+struct Ref2 {
+  int &a;
+  constexpr Ref2(int &a) : a(a) {}
+};
+
+constexpr int RecordWithRef2() {
+  int m = 100;
+  Ref2 r(m);
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef2() == 200, "");
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -104,7 +104,7 @@
   if (const FieldDecl *Member = Init->getMember()) {
 const Record::Field *F = R->getField(Member);
 
-if (Optional T = this->classify(InitExpr->getType())) {
+if (Optional T = this->classify(InitExpr)) {
   if (!this->emitThis(InitExpr))
 return false;
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -397,7 +397,11 @@
 const Record *R = getRecord(RD);
 const Record::Field *F = R->getField(FD);
 // Leave a pointer to the field on the stack.
-return this->emitGetPtrField(F->Offset, E);
+if (!this->emitGetPtrField(F->Offset, E))
+  return false;
+if (F->Decl->getType()->isReferenceType())
+  return this->emitLoadPopPtr(E);
+return true;
   }
 
   return false;
@@ -871,7 +875,7 @@
   if (!this->emitDupPtr(Initializer))
 return false;
 
-  if (Optional T = classify(Init->getType())) {
+  if (Optional T = classify(Init)) {
 if (!this->visit(Init))
   return false;
 


Index: clang/test/AST/Interp/references.cpp
===
--- clang/test/AST/Interp/references.cpp
+++ clang/test/AST/Interp/references.cpp
@@ -88,3 +88,29 @@
   return j;
 }
 static_assert(RefToMemberExpr() == 11, "");
+
+struct Ref {
+  int &a;
+};
+
+constexpr int RecordWithRef() {
+  int m = 100;
+  Ref r{m};
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef() == 200, "");
+
+
+struct Ref2 {
+  int &a;
+  constexpr Ref2(int &a) : a(a) {}
+};
+
+constexpr int RecordWithRef2() {
+  int m = 100;
+  Ref2 r(m);
+  m = 200;
+  return r.a;
+}
+static_assert(RecordWithRef2() == 200, "");
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -104,7 +104,7 @@
   if (const FieldDecl *Member = Init->getMember()) {
 const Record::Field *F = R->getField(Member);
 
-if (Optional T = this->classify(InitExpr->getType())) {
+if (Optional T = this->classify(InitExpr)) {
   if (!this->emitThis(InitExpr))
 return false;
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -397,7 +397,11 @@
 const Record *R = getRecord(RD);
 const Record::Field *F = R->getField(FD);
 // Leave a pointer to the field on the stack.
-return this->emitGetPtrField(F->Offset, E);
+if (!this->emitGetPtrField(F->Offset, E))
+  return false;
+if (F->Decl->getType()->isReferenceType())
+  return this->emitLoadPopPtr(E);
+return true;
   }
 
   return false;
@@ -871,7 +875,7 @@
   if (!this->emitDupPtr(Initializer))
 return false;
 
-  if (Optional T = classify(Init->getType())) {
+  if (Optional T = classify(Init)) {
 if (!this->visit(Init))
   return false;
 
___
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

2022-10-15 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 467997.

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
@@ -119,6 +119,9 @@
 
 template  inline bool IsTrue(const T &V) { return !V.isZero(); }
 
+/// Interpreter entry point.
+bool Interpret(InterpState &S, APValue &Result);
+
 //===--===//
 // Add, Sub, Mul
 //===--===//
@@ -1123,6 +1126,36 @@
   }
 }
 
+template ::T>
+static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
+  auto *NewFrame = new InterpFrame(S, Func, PC);
+  if (Func->hasThisPointer()) {
+if (!CheckInvoke(S, PC, NewFrame->getThis())) {
+  delete NewFrame;
+  return false;
+}
+// TODO: CheckCallable
+  }
+
+  InterpFrame *FrameBefore = S.Current;
+  S.Current = NewFrame;
+
+  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)) {
+assert(S.Current == FrameBefore);
+return true;
+  }
+
+  // Interpreting the function failed somehow. Reset to
+  // previous state.
+  delete NewFrame;
+  S.Current = FrameBefore;
+  return false;
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
@@ -1136,9 +1169,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 It->second;
 }
 
Index: clang/lib/AST/Interp/EvalEmitter.cpp
==

56 matches

Mail list logo