[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

2021-03-04 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke added a comment.

LGTM too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93594

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


[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

2021-03-04 Thread Bing Yu via Phabricator via cfe-commits
yubing updated this revision to Diff 328408.
yubing added a comment.

Address pengfei's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93594

Files:
  llvm/include/llvm/CodeGen/Passes.h
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/AMX/amx-low-intrinsics-no-amx-bitcast.ll
  llvm/test/CodeGen/X86/AMX/amx-low-intrinsics.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -513,7 +513,8 @@
   "expand-reductions","indirectbr-expand",
   "generic-to-nvvm",  "expandmemcmp",
   "loop-reduce",  "lower-amx-type",
-  "polyhedral-info",  "replace-with-veclib"};
+  "lower-amx-intrinsics" ,"polyhedral-info",
+  "replace-with-veclib"};
   for (const auto  : PassNamePrefix)
 if (Pass.startswith(P))
   return true;
Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -24,11 +24,12 @@
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   Expand Atomic instructions
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
 ; CHECK-NEXT:   Module Verifier
-; CHECK-NEXT:   Dominator Tree Construction
 ; CHECK-NEXT:   Basic Alias Analysis (stateless AA impl)
-; CHECK-NEXT:   Natural Loop Information
 ; CHECK-NEXT:   Canonicalize natural loops
 ; CHECK-NEXT:   Scalar Evolution Analysis
 ; CHECK-NEXT:   Loop Pass Manager
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -18,6 +18,9 @@
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   Expand Atomic instructions
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
 ; CHECK-NEXT:   Module Verifier
 ; CHECK-NEXT:   Lower Garbage Collection Instructions
Index: llvm/test/CodeGen/X86/AMX/amx-type.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-type.ll
+++ llvm/test/CodeGen/X86/AMX/amx-type.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -lower-amx-type %s -S | FileCheck %s
+; RUN: opt --codegen-opt-level=2 -mtriple=x86_64 -lower-amx-type %s -S | FileCheck %s
 
 %struct.__tile_str = type { i16, i16, <256 x i32> }
 
Index: llvm/test/CodeGen/X86/AMX/amx-low-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/AMX/amx-low-intrinsics.ll
@@ -0,0 +1,237 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -mtriple=x86_64 -lower-amx-intrinsics %s -S | FileCheck %s
+
+define dso_local void @test_amx_load_non_O0(i16 signext %row, i16 signext %col, i8 *%ptr, i64 %stride, <256 x i32>* %vptr) {
+; CHECK-LABEL: @test_amx_load_non_O0(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TMP0:%.*]] = lshr i16 [[COL:%.*]], 2
+; CHECK-NEXT:[[TMP1:%.*]] = lshr i64 [[STRIDE:%.*]], 2
+; CHECK-NEXT:br label [[TILELOAD_SCALARIZE_ROWS_HEADER:%.*]]
+; CHECK:   tileload.scalarize.rows.header:
+; CHECK-NEXT:[[TILELOAD_SCALARIZE_ROWS_IV:%.*]] = phi i16 [ 0, [[ENTRY:%.*]] ], [ [[TILELOAD_SCALARIZE_ROWS_STEP:%.*]], [[TILELOAD_SCALARIZE_ROWS_LATCH:%.*]] ]
+; CHECK-NEXT:[[VEC_PHI_ROW:%.*]] = phi <256 x i32> [ zeroinitializer, [[ENTRY]] ], [ [[TMP11:%.*]], [[TILELOAD_SCALARIZE_ROWS_LATCH]] ]
+; CHECK-NEXT:br label [[TILELOAD_SCALARIZE_ROWS_BODY:%.*]]
+; CHECK:   tileload.scalarize.rows.body:
+; CHECK-NEXT:br label [[TILELOAD_SCALARIZE_COLS_HEADER:%.*]]
+; CHECK:   tileload.scalarize.cols.header:
+; CHECK-NEXT:[[TILELOAD_SCALARIZE_COLS_IV:%.*]] = phi i16 [ 0, [[TILELOAD_SCALARIZE_ROWS_BODY]] ], [ [[TILELOAD_SCALARIZE_COLS_STEP:%.*]], [[TILELOAD_SCALARIZE_COLS_LATCH:%.*]] ]
+; CHECK-NEXT:[[VEC_PHI:%.*]] = phi <256 x i32> [ [[VEC_PHI_ROW]], [[TILELOAD_SCALARIZE_ROWS_BODY]] ], [ [[TMP11]], [[TILELOAD_SCALARIZE_COLS_LATCH]] ]
+; CHECK-NEXT:br label [[TILELOAD_SCALARIZE_COLS_BODY:%.*]]
+; CHECK:   

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

2021-03-04 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng updated this revision to Diff 328407.
kito-cheng added a comment.

Reupload


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-gcc
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/rv64imac/lp64/crt0.o
  clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/bin/riscv64-unknown-elf-gcc
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad/bin/riscv64-unknown-elf-gcc
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad/riscv64-unknown-elf/lib/crt0.o
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -153,6 +153,24 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
+// Test for multi-lib config from GCC.
+// RUN: %clang %s \
+// RUN:   -target riscv64-unknown-elf \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv64_elf_sdk \
+// RUN:   --print-multi-lib \
+// RUN:   | FileCheck -check-prefix=C-RV64-GCC-MULTI-LIB %s
+// C-RV64-GCC-MULTI-LIB: rv32iac/ilp32;@march=rv32iac@mabi=ilp32
+// C-RV64-GCC-MULTI-LIB-NEXT: rv32imafc/ilp32f;@march=rv32iac@mabi=ilp32f
+// C-RV64-GCC-MULTI-LIB-NEXT: rv64imafdc/lp64d;@march=rv64imafdc@mabi=lp64d
+// C-RV64-GCC-MULTI-LIB-NOT:  {{^.+$}}
+
+// RUN: %clang %s \
+// RUN:   -target riscv64-unknown-elf \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv64_elf_sdk_bad \
+// RUN:   --print-multi-lib 2>&1 \
+// RUN:   | FileCheck -check-prefix=C-RV64-GCC-MULTI-LIB-BAD %s
+// C-RV64-GCC-MULTI-LIB-BAD: warning: xxx option unrecognized in multi-lib configuration when parsing config from GCC, falling back to built-in multi-lib configuration. [-Wmultilib-fallback]
+
 // RUN: %clang -target riscv64 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;
Index: clang/test/Driver/riscv32-toolchain.c

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

2021-03-04 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng updated this revision to Diff 328406.
kito-cheng marked 2 inline comments as done.
kito-cheng added a comment.

- Fix build issue.
- Address Jim Lin's and Zakk's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -526,6 +526,7 @@
 * 64-bit ARM (AArch64)
 * AMDGPU
 * SPIR
+* RISC-V
 
 ``_Float16`` will be supported on more targets as they define ABIs for it.
 
@@ -535,13 +536,18 @@
 
 The ``__bf16`` type is only available when supported in hardware.
 
-``__fp16`` is a storage and interchange format only.  This means that values of
+``__fp16`` is a storage and interchange format only on most targets.  This 
means that values of
 ``__fp16`` are immediately promoted to (at least) ``float`` when used in 
arithmetic
 operations, so that e.g. the result of adding two ``__fp16`` values has type 
``float``.
 The behavior of ``__fp16`` is specified by the ARM C Language Extensions 
(`ACLE 
`_).
 Clang uses the ``binary16`` format from IEEE 754-2008 for ``__fp16``, not the 
ARM
 alternative format.
 
+On RISC-V, the evaluation format of ``__fp16`` will depend on the presence of
+the ``Zfh``  (half-precision) extension. ``__fp16`` will perform arithmetic
+without promotion when the Zfh extension is enabled, and promoted to float
+otherwise.
+
 ``_Float16`` is an interchange floating-point type.  This means that, just 
like arithmetic on
 ``float`` or ``double``, arithmetic on ``_Float16`` operands is formally 
performed in the
 ``_Float16`` type, so that e.g. the result of adding two ``_Float16`` values 
has type


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -526,6 +526,7 @@
 * 64-bit ARM (AArch64)
 * AMDGPU
 * SPIR
+* RISC-V
 
 ``_Float16`` will be supported on more targets as they define ABIs for it.
 
@@ -535,13 +536,18 @@
 
 The ``__bf16`` type is only available when supported in hardware.
 
-``__fp16`` is a storage and interchange format only.  This means that values of
+``__fp16`` is a storage and interchange format only on most targets.  This means that values of
 ``__fp16`` are immediately promoted to (at least) ``float`` when used in arithmetic
 operations, so that e.g. the result of adding two ``__fp16`` values has type ``float``.
 The behavior of ``__fp16`` is specified by the ARM C Language Extensions (`ACLE `_).
 Clang uses the ``binary16`` format from IEEE 754-2008 for ``__fp16``, not the ARM
 alternative format.
 
+On RISC-V, the evaluation format of ``__fp16`` will depend on the presence of
+the ``Zfh``  (half-precision) extension. ``__fp16`` will perform arithmetic
+without promotion when the Zfh extension is enabled, and promoted to float
+otherwise.
+
 ``_Float16`` is an interchange floating-point type.  This means that, just like arithmetic on
 ``float`` or ``double``, arithmetic on ``_Float16`` operands is formally performed in the
 ``_Float16`` type, so that e.g. the result of adding two ``_Float16`` values has type
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D98010: [XCOFF] [DWARF] set default DWARF version to 3

2021-03-04 Thread ChenZheng via Phabricator via cfe-commits
shchenz created this revision.
shchenz added reviewers: jsji, hubert.reinterpretcast, jasonliu, PowerPC.
shchenz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We support DWARF for XCOFF in D95518 .

In this patch, we set the default DWARF version to 3 because the latest AIX OS 
supports DWARF version 3.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98010

Files:
  clang/lib/Driver/ToolChains/AIX.h
  clang/test/CodeGen/dwarf-version.c


Index: clang/test/CodeGen/dwarf-version.c
===
--- clang/test/CodeGen/dwarf-version.c
+++ clang/test/CodeGen/dwarf-version.c
@@ -32,6 +32,17 @@
 // Explicitly request both.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -gcodeview -S -emit-llvm 
-o - %s \
 // RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+// RUN: %clang -target powerpc-ibm-aix-xcoff -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=VER3
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-2 -S -emit-llvm -o - %s | 
\
+// RUN:   FileCheck %s --check-prefix=VER2
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-3 -S -emit-llvm -o - %s | 
\
+// RUN:   FileCheck %s --check-prefix=VER3
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-4 -S -emit-llvm -o - %s | 
\
+// RUN:   FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-5 -S -emit-llvm -o - %s | 
\
+// RUN:   FileCheck %s --check-prefix=VER5
+
 int main (void) {
   return 0;
 }
Index: clang/lib/Driver/ToolChains/AIX.h
===
--- clang/lib/Driver/ToolChains/AIX.h
+++ clang/lib/Driver/ToolChains/AIX.h
@@ -74,6 +74,9 @@
 
   RuntimeLibType GetDefaultRuntimeLibType() const override;
 
+  // Set default DWARF version to 3 for now as latest AIX OS supports version 
3.
+  unsigned GetDefaultDwarfVersion() const override { return 3; }
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;


Index: clang/test/CodeGen/dwarf-version.c
===
--- clang/test/CodeGen/dwarf-version.c
+++ clang/test/CodeGen/dwarf-version.c
@@ -32,6 +32,17 @@
 // Explicitly request both.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -gcodeview -S -emit-llvm -o - %s \
 // RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+// RUN: %clang -target powerpc-ibm-aix-xcoff -g -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=VER3
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-2 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=VER2
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-3 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=VER3
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-4 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-5 -S -emit-llvm -o - %s | \
+// RUN:   FileCheck %s --check-prefix=VER5
+
 int main (void) {
   return 0;
 }
Index: clang/lib/Driver/ToolChains/AIX.h
===
--- clang/lib/Driver/ToolChains/AIX.h
+++ clang/lib/Driver/ToolChains/AIX.h
@@ -74,6 +74,9 @@
 
   RuntimeLibType GetDefaultRuntimeLibType() const override;
 
+  // Set default DWARF version to 3 for now as latest AIX OS supports version 3.
+  unsigned GetDefaultDwarfVersion() const override { return 3; }
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

2021-03-04 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie updated this revision to Diff 328396.
OikawaKirie retitled this revision from "[clang] Allow clang-check to customize 
output file name" to "[clang] Allow clang-check to customize analyzer output 
file or dir name".
OikawaKirie edited the summary of this revision.
OikawaKirie added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, dkrupp, donat.nagy, 
Szelethus, a.sidorin, baloghadamsoftware.

1. Update option name from `-output` to `-analyzer-output-path`. As it can 
either be a file name or a directory name, the word `path` may not be accurate. 
Please give me your suggestions on this option name.
2. Add more verifications on the output files, rather than verifying the value 
of the `-o` option. For simplicity, the regression test case only checks the 
plist format.


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

https://reviews.llvm.org/D97265

Files:
  clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
  clang/tools/clang-check/ClangCheck.cpp


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,7 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
-
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
   cl::cat(ClangCheckCategory));
@@ -206,7 +209,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo 
-ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-check -p "%t" "%t/test.cpp" -analyze 
-analyzer-output-path=qwerty -extra-arg=-v -extra-arg=-Xclang 
-extra-arg=-verify 2>&1 | cat - qwerty | FileCheck %s
+// FIXME: Make the above easier.
+
+// CHECK: Invocation
+// CHECK: {{qwerty}}
+// CHECK: DOCTYPE plist
+// CHECK: Division by zero
+int f() {
+  return 1 / 0; // expected-warning {{Division by zero}}
+}


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,7 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
-
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
   cl::cat(ClangCheckCategory));
@@ -206,7 +209,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

2021-03-04 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng marked 5 inline comments as done.
kito-cheng added a comment.

Here is another solution for flexible multi-lib configuration I consider before:

- Add an option called `-fmultilib-config=`
- Using same or similar syntax with GCC's `--with-multilib-generator`

But the problem is it's really too long for describe a multi-lib config.

e.g: current default bare-metal multi-lib configuration:
`--with-multilib-generator="rv32i-ilp32--c;rv32im-ilp32--c;rv32iac-ilp32--;rv32imac-ilp32--;rv32imafc-ilp32f-rv32imafdc-;rv64imc-lp64--;rv64imfc-lp64f--;rv64imac-lp64--;rv64imafdc-lp64d--"`




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1625
+   const std::string ,
+   DetectedMultilibs ) {
+  llvm::StringSet<> AllABI;

khchen wrote:
> bool -> static bool?
> const std::string  -> StringRef ?
> 
> The function name seem like it's not a target-specific function, but it's 
> only support RISC-V now. Maybe we could rename it or make an assertion in 
> function to check the target should be RISC-V?
> 
> Maybe we could rename it or make an assertion in function to check the target 
> should be RISC-V?

Good point, renamed.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1642
+  std::string CurrentArchOpt = Twine("march=", MArch).str();
+  std::string CurrentMCmodelOpt = llvm::StringSwitch(CodeModel)
+  .Case("medium", "mcmodel=medany")

khchen wrote:
> Could they be declared as StringRef and use llvm::StringSwitch?
I would prefer keep `const char *` to prevent create a temporal object for 
`StringRef` here.

And I also found another place are used same way:
```
$ grep "StringSwitch(BufStr)
clang/lib/Driver/ToolChains/Gnu.cpp:  std::string CurrentMCmodelOpt = 
llvm::StringSwitch(CodeModel)
clang/lib/Driver/ToolChains/Clang.cpp:MipsTargetFeature = 
llvm::StringSwitch(Value)
clang/lib/Driver/ToolChains/Cuda.cpp:  OOpt = llvm::StringSwitch(A->getValue())
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:  OOpt = 
llvm::StringSwitch(A->getValue())
clang/lib/Driver/ToolChains/Arch/Mips.cpp:ABIName = 
llvm::StringSwitch(CPUName)
clang/lib/Driver/ToolChains/Arch/Mips.cpp:CPUName = 
llvm::StringSwitch(ABIName)
clang/lib/Driver/ToolChains/Arch/PPC.cpp:return llvm::StringSwitch(CPUName)
clang/lib/Driver/ToolChains/Arch/PPC.cpp:  return llvm::StringSwitch(Name)
clang/lib/Driver/ToolChains/Arch/Sparc.cpp:return llvm::StringSwitch(Name)
clang/lib/Driver/ToolChains/Arch/Sparc.cpp:return llvm::StringSwitch(Name)
clang/lib/Driver/ToolChains/Darwin.cpp:  return llvm::StringSwitch(Arch)
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:StringSwitch(T.first->getValueAsString("AccessQualifier"))
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:  OS << 
StringSwitch(
llvm/include/llvm/Support/FormatProviders.h:Stream << StringSwitch(Style)
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:  StringSwitch(BroadcastString)
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:const char *Repl = 
StringSwitch(Name)
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:  const char *Repl = 
StringSwitch(Op.getToken())
llvm/lib/Support/Host.cpp:  return StringSwitch(StringRef(CPUStart, CPULen))
llvm/lib/Support/Host.cpp:return StringSwitch(Part)
llvm/lib/Support/Host.cpp:return StringSwitch(Part)
llvm/lib/Support/Host.cpp:return StringSwitch(Part)
llvm/lib/Support/Host.cpp:return StringSwitch(Part)
llvm/lib/Support/Host.cpp:return StringSwitch(Part)
llvm/lib/Support/Host.cpp:return StringSwitch(Part)
llvm/unittests/Support/Host.cpp:  StringRef MCPU = StringSwitch(CPU)
```



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1650
+
+  if (File) {
+SmallVector Lines;

khchen wrote:
> ```
> if (!File)
>   return;
> ```
Good suggestion, thanks! 



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1703
+}
+  }
+  Ms.emplace_back(Multilib);

Jim wrote:
> Do you have plan to support other kind of options to build multilib?
Currently, no, but I think could be consider in future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

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


[clang] 931a3aa - [Driver][test] Fix ClangDriverTest

2021-03-04 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2021-03-04T22:44:37-08:00
New Revision: 931a3aa96726ec6d2052e24e9966fe32d98ddd3e

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

LOG: [Driver][test] Fix ClangDriverTest

Added: 


Modified: 
clang/lib/Driver/Distro.cpp
clang/unittests/Driver/DistroTest.cpp

Removed: 




diff  --git a/clang/lib/Driver/Distro.cpp b/clang/lib/Driver/Distro.cpp
index 785394eefc97..9fc790f17d6c 100644
--- a/clang/lib/Driver/Distro.cpp
+++ b/clang/lib/Driver/Distro.cpp
@@ -36,10 +36,10 @@ static Distro::DistroType 
DetectOsRelease(llvm::vfs::FileSystem ) {
   for (StringRef Line : Lines)
 if (Version == Distro::UnknownDistro && Line.startswith("ID="))
   Version = llvm::StringSwitch(Line.substr(3))
+.Case("alpine", Distro::AlpineLinux)
 .Case("fedora", Distro::Fedora)
 .Case("gentoo", Distro::Gentoo)
 .Case("arch", Distro::ArchLinux)
-.Case("exherbo", Distro::Exherbo)
 // On SLES, /etc/os-release was introduced in SLES 11.
 .Case("sles", Distro::OpenSUSE)
 .Case("opensuse", Distro::OpenSUSE)

diff  --git a/clang/unittests/Driver/DistroTest.cpp 
b/clang/unittests/Driver/DistroTest.cpp
index 7c00a8f66be8..b6e4ca9be857 100644
--- a/clang/unittests/Driver/DistroTest.cpp
+++ b/clang/unittests/Driver/DistroTest.cpp
@@ -275,12 +275,10 @@ TEST(DistroTest, DetectDebian) {
 
 TEST(DistroTest, DetectExherbo) {
   llvm::vfs::InMemoryFileSystem ExherboFileSystem;
-  ExherboFileSystem.addFile("/etc/exherbo-release", 0, // (ASCII art)
- llvm::MemoryBuffer::getMemBuffer(""));
   ExherboFileSystem.addFile("/etc/os-release", 0,
   llvm::MemoryBuffer::getMemBuffer("NAME=\"Exherbo\"\n"
"PRETTY_NAME=\"Exherbo Linux\"\n"
-   "ID=\"exherbo\"\n"
+   "ID=exherbo\n"
"ANSI_COLOR=\"0;32\"\n"

"HOME_URL=\"https://www.exherbo.org/\"\n;

"SUPPORT_URL=\"irc://irc.freenode.net/#exherbo\"\n"



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


[clang] 087e7ab - [Driver] Switch Exherbo/Alpine/Arch Linux to /etc/os-release

2021-03-04 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2021-03-04T22:31:05-08:00
New Revision: 087e7ab459e74ad3b33701296ade581cb4c3ba6c

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

LOG: [Driver] Switch Exherbo/Alpine/Arch Linux to /etc/os-release

Added: 


Modified: 
clang/lib/Driver/Distro.cpp

Removed: 




diff  --git a/clang/lib/Driver/Distro.cpp b/clang/lib/Driver/Distro.cpp
index ee4fe841e7ee..785394eefc97 100644
--- a/clang/lib/Driver/Distro.cpp
+++ b/clang/lib/Driver/Distro.cpp
@@ -39,6 +39,7 @@ static Distro::DistroType 
DetectOsRelease(llvm::vfs::FileSystem ) {
 .Case("fedora", Distro::Fedora)
 .Case("gentoo", Distro::Gentoo)
 .Case("arch", Distro::ArchLinux)
+.Case("exherbo", Distro::Exherbo)
 // On SLES, /etc/os-release was introduced in SLES 11.
 .Case("sles", Distro::OpenSUSE)
 .Case("opensuse", Distro::OpenSUSE)
@@ -188,15 +189,6 @@ static Distro::DistroType 
DetectDistro(llvm::vfs::FileSystem ) {
   }
 
   // ...and others.
-  if (VFS.exists("/etc/exherbo-release"))
-return Distro::Exherbo;
-
-  if (VFS.exists("/etc/alpine-release"))
-return Distro::AlpineLinux;
-
-  if (VFS.exists("/etc/arch-release"))
-return Distro::ArchLinux;
-
   if (VFS.exists("/etc/gentoo-release"))
 return Distro::Gentoo;
 



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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Siva Chandra via Phabricator via cfe-commits
sivachandra added a comment.

In D97736#2605535 , @phosek wrote:

> Have you considered using an input linker script? We could generate `libc.so` 
> that could look something like:
>
>   INPUT(libllvmlibc.a /lib/libc.so)
>
> We would need to pass `--sysroot` to the linker for this to work. The driver 
> could remain completely agnostic of whether you're using LLVM libc or not.

Yes, that was also considered. Those downstream users who have the flexibility 
to do it that way should be able to do it that way. However, not all downstream 
users or normal clang users will have that liberty [1]. Another point to note 
is that we will have to do this with all libc components like `libc.so`, 
`libm.so` etc.

[1] I think all of this can be done. For example, we can set all this up when 
building a distribution. However, I am not sure this is worth it when we know 
this is a transient phase. Soon, when LLVM libc is complete enough, a more 
appropriate option would be the one which allows choosing a libc as Eric 
pointed out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

Have you considered using an input linker script? We could generate `libc.so` 
that could look something like:

  INPUT(libllvmlibc.a /lib/libc.so)

We would need to pass `--sysroot` to the linker for this to work. The driver 
could remain completely agnostic of whether you're using LLVM libc or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

2021-03-04 Thread Tim Wojtulewicz via Phabricator via cfe-commits
timwoj added a comment.

In D94500#2605515 , 
@HazardyKnusperkeks wrote:

> In D94500#2604754 , @timwoj wrote:
>
>> Rebase onto master again
>
> `master` or `main`? Please rebase on `main`. That is where the pushes land. 
> (And as far as I can see `master` hasn't been updated in quite some time.

Yes, `main`, sorry. It was late in my work day and I was fried already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

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


[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

2021-03-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D94500#2604754 , @timwoj wrote:

> Rebase onto master again

`master` or `main`? Please rebase on `main`. That is where the pushes land. 
(And as far as I can see `master` hasn't been updated in quite some time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

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


[clang] bc172e5 - [clang][StaticAnalyzer] Compilation fix.

2021-03-04 Thread Michael Kruse via cfe-commits

Author: Michael Kruse
Date: 2021-03-04T23:23:58-06:00
New Revision: bc172e532a89754d47fef1306064a26a4dc0a76b

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

LOG: [clang][StaticAnalyzer] Compilation fix.

An enum was unhandled after landing of D94973. Add the new
OMPCanonicalLoopClass to the list of unhandled cases.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 465af24b2899..a388fc9e6e26 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1238,6 +1238,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
 case Stmt::SEHExceptStmtClass:
 case Stmt::SEHLeaveStmtClass:
 case Stmt::SEHFinallyStmtClass:
+case Stmt::OMPCanonicalLoopClass:
 case Stmt::OMPParallelDirectiveClass:
 case Stmt::OMPSimdDirectiveClass:
 case Stmt::OMPForDirectiveClass:



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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Siva Chandra via Phabricator via cfe-commits
sivachandra updated this revision to Diff 328380.
sivachandra added a comment.

Remove empty '//' in lit test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -232,7 +232,27 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
-//
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,23 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+  }
+  WithLLVMLibc.push_back(Arg.data());
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -232,7 +232,27 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
-//
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: 

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Siva Chandra via Phabricator via cfe-commits
sivachandra added a comment.

In D97736#2605432 , @echristo wrote:

> In addition to the bikeshed below, I'm not a huge fan of this in general. I 
> think we should assume that the libc we link is complete and thus it would 
> just be named "libc." and in a sysroot somewhere. Another option is 
> perhaps looking at the rtlib option, but I'd want to see a bit of a writeup 
> there so we can see what we'd be doing.

I missed responding to the "writeup" part earlier. But, I am not sure what I 
should be writing about. I feel that there is some confusion that this option 
can be used to switch between libcs. As I said in the earlier reply, the idea 
is not to provide a way to switch between libcs. Rather, the goal is to provide 
a way to use LLVM libc along with the system libc with the effect that LLVM 
libc symbols will be preferred by the linker over the system libc symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Siva Chandra via Phabricator via cfe-commits
sivachandra added a comment.

In D97736#2605432 , @echristo wrote:

> In addition to the bikeshed below, I'm not a huge fan of this in general. I 
> think we should assume that the libc we link is complete and thus it would 
> just be named "libc." and in a sysroot somewhere. Another option is 
> perhaps looking at the rtlib option, but I'd want to see a bit of a writeup 
> there so we can see what we'd be doing.

By not giving it a `libc.` name, we are actually implying that, "it is not 
the full libc that you expect." It is expected that it will be used along with 
another full libc. When LLVM libc is complete enough, then yes we should be 
giving it the conventional name of `libc.` and the experimental option can 
be thrown out.

In D97736#2605159 , @MaskRay wrote:

> If the end goal is to provide a complete libc, and currently the usage is 
> expected to shadow system libc (this has to be very careful, as I don't know 
> how shadowing some important components like pthread shall work), perhaps the 
> name should convey this point?

Yes. So, we have not done it yet, but the plan is to be able to build LLVM in 
two different modes:

1. The default mode: This mode only builds and packages the ABI independent and 
ABI agnostic parts of LLVM libc. The binaries produced from this mode are to be 
used with the `-experimental-llvm-libc`. The plan is also to make default mode 
work under `LLVM_RUNTIMES_BUILD`.
2. The full libc mode: In this mode, LLVM libc will be built as if it is a full 
libc. A special CMake option needs to be set to build in this mode. The 
binaries produced in this mode will include everything, including ABI sensitive 
pieces. They will be tested on bots but will not be practically useful for many 
months as we are still building out the libc.




Comment at: clang/include/clang/Driver/Options.td:3626
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;

echristo wrote:
> Bikeshedding: Not a huge fan of this name. I'd rather a more general option 
> if we feel the need to select libc from an option rather than a sysroot. 
The idea is not to provide an option to "choose" a libc. The idea is that this 
option will help one use what LLVM libc provides while getting the rest from 
the system libc. I would think that the option to "choose" a libc is not 
related to this as LLVM libc is not complete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In addition to the bikeshed below, I'm not a huge fan of this in general. I 
think we should assume that the libc we link is complete and thus it would just 
be named "libc." and in a sysroot somewhere. Another option is perhaps 
looking at the rtlib option, but I'd want to see a bit of a writeup there so we 
can see what we'd be doing.

Thoughts? Other options?

Thanks!

-eric




Comment at: clang/include/clang/Driver/Options.td:3626
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;

Bikeshedding: Not a huge fan of this name. I'd rather a more general option if 
we feel the need to select libc from an option rather than a sysroot. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment.

I am a little confusing about the problem. The example in the link tells the 
align of the `promise` instead of the `frame`. The address of `promise` and 
`frame` is not same. It looks like you're trying to do:

  +   +---+
  |   |   |
  +---+  frame|
  | pedding   |   |
  +   +---+
  ^
  |
  |
  |
  |
  |
  +
  
The address of frame matches the offset of promise.

However, what we should do is:

  +   +---+
  |   |   +--+|
  +---+frame  | promise  ||
  | pedding   |   <--+|
  +   +---+
  ^   |
  |   |
  |   |
  |   |
  |   +
  |   This is what we really want
  +
  
The address of frame matches the offset of promise.

If I get the problem problems, I think we can handle this problem in the middle 
end if the information for the promise remains.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16756
 };
-} // namespace
 

Why we remove the anonymous namespace here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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


[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

2021-03-04 Thread Zakk Chen via Phabricator via cfe-commits
khchen added a comment.

I didn't check why I got error in  
Failed Tests (1):

  Clang :: Driver/riscv64-toolchain.c

Could you please double check it?
Thanks!




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1599
+static std::string getGCCPath(const Driver , const ArgList ) {
+  llvm::StringRef GCCBasePath;
+  std::string GCCPath;

move into else



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1604
+  const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain);
+  if (A) {
+GCCPath = findGCCPath(D, A->getValue());

if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain))



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1625
+   const std::string ,
+   DetectedMultilibs ) {
+  llvm::StringSet<> AllABI;

bool -> static bool?
const std::string  -> StringRef ?

The function name seem like it's not a target-specific function, but it's only 
support RISC-V now. Maybe we could rename it or make an assertion in function 
to check the target should be RISC-V?




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1642
+  std::string CurrentArchOpt = Twine("march=", MArch).str();
+  std::string CurrentMCmodelOpt = llvm::StringSwitch(CodeModel)
+  .Case("medium", "mcmodel=medany")

Could they be declared as StringRef and use llvm::StringSwitch?



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1650
+
+  if (File) {
+SmallVector Lines;

```
if (!File)
  return;
```



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1736
+
+  int rc = llvm::sys::ExecuteAndWait(GCCPath, GCCArgs, None, Redirects);
+

rc->RC



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1744
+  // which multi-lib should be used.
+  return ScanGCCMultilibConfig(D, TargetTriple, Path, Args, MultilibOutput,
+   Result);

ScanGCCMultilibConfig->scanGCCMultilibConfig


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

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


[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

2021-03-04 Thread Jim Lin via Phabricator via cfe-commits
Jim added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1744
+  // which multi-lib should be used.
+  return ScanGCCMultilibConfig(D, TargetTriple, Path, Args, MultilibOutput,
+   Result);

scanGCCMultilibConfig ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

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


[clang-tools-extra] 889da99 - [clang][AST] Fix Wreturn-type gcc warning (NFC)

2021-03-04 Thread Yang Fan via cfe-commits

Author: Yang Fan
Date: 2021-03-05T11:24:55+08:00
New Revision: 889da99523930e81e4080084ab9530251c23873d

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

LOG: [clang][AST] Fix Wreturn-type gcc warning (NFC)

GCC warning:
```
/llvm-project/clang-tools-extra/clangd/SemanticHighlighting.cpp: In function 
‘bool clang::clangd::{anonymous}::canHighlightName(clang::DeclarationName)’:
/llvm-project/clang-tools-extra/clangd/SemanticHighlighting.cpp:64:1: warning: 
control reaches end of non-void function [-Wreturn-type]
   64 | }
  | ^
```

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 9e24b92b0f5c..0b4965c42715 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -61,6 +61,7 @@ bool canHighlightName(DeclarationName Name) {
   case DeclarationName::CXXUsingDirective:
 return false;
   }
+  llvm_unreachable("invalid name kind");
 }
 
 llvm::Optional kindForType(const Type *TP);



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


[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-03-04 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment.

Sorry, was a bit caught up with assignments. I will try to come up with a 
better implementation with the advice given by @NoQ and @steakhal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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


[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

2021-03-04 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment.

Talking on the mailing list, I  got a link on reinterpret_casting for 
pointer-to-member: 
https://timsong-cpp.github.io/cppwp/n4861/expr.reinterpret.cast#10
The gist is that this sort of cast is only valid if we cast back to the 
original type (that too with a caveat). Now currently there are three things 
that can be done:

- Leave things the way they are - reinterpret_cast will cause an assertion to 
fail
- Never analyze reinterpret_cast - this is easy, just return nullptr for  
PointerToMemberData
- Properly model this - this would require possibly adding more information in 
PointerToMember and a significantly complex logic to handle it.

The first two are trivially done. I would like to know if it is worth following 
the third - especially since it is a really obscure feature.
@vsavchenko, @NoQ, @steakhal what do you think should be done?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96976

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


[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

2021-03-04 Thread Jim Lin via Phabricator via cfe-commits
Jim added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1703
+}
+  }
+  Ms.emplace_back(Multilib);

Do you have plan to support other kind of options to build multilib?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

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


[PATCH] D94355: [Passes] Add relative lookup table converter pass

2021-03-04 Thread Gulfem Savrun Yeniceri via Phabricator via cfe-commits
gulfem updated this revision to Diff 328354.
gulfem added a comment.

Use TTI hook for target machine checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

Files:
  clang/test/CodeGen/switch-to-lookup-table.c
  llvm/docs/Passes.rst
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
  llvm/include/llvm/CodeGen/BasicTTIImpl.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Scalar.h
  llvm/include/llvm/Transforms/Utils/RelLookupTableConverter.h
  llvm/lib/Analysis/TargetTransformInfo.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp
  llvm/lib/Transforms/Utils/Utils.cpp
  llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
  llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
@@ -61,6 +61,7 @@
 "NameAnonGlobals.cpp",
 "PredicateInfo.cpp",
 "PromoteMemoryToRegister.cpp",
+"RelLookupTableConverter.cpp"
 "SSAUpdater.cpp",
 "SSAUpdaterBulk.cpp",
 "SampleProfileLoaderBaseUtil.cpp",
Index: llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
===
--- /dev/null
+++ llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
@@ -0,0 +1,310 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -rel-lookup-table-converter -relocation-model=pic -S | FileCheck %s
+; RUN: opt < %s -passes=rel-lookup-table-converter -relocation-model=pic -S | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str = private unnamed_addr constant [5 x i8] c"zero\00", align 1
+@.str.1 = private unnamed_addr constant [4 x i8] c"one\00", align 1
+@.str.2 = private unnamed_addr constant [4 x i8] c"two\00", align 1
+@.str.3 = private unnamed_addr constant [8 x i8] c"default\00", align 1
+@.str.4 = private unnamed_addr constant [6 x i8] c"three\00", align 1
+@.str.5 = private unnamed_addr constant [5 x i8] c"str1\00", align 1
+@.str.6 = private unnamed_addr constant [5 x i8] c"str2\00", align 1
+@.str.7 = private unnamed_addr constant [12 x i8] c"singlevalue\00", align 1
+
+@a1 = external global i32, align 4
+@b1 = external global i32, align 4
+@c1 = external global i32, align 4
+@d1 = external global i32, align 4
+
+@a2 = internal global i32 0, align 4
+@b2 = internal global i32 0, align 4
+@c2 = internal global i32 0, align 4
+@d2 = internal global i32 0, align 4
+
+@hidden0 = external hidden global i32, align 8
+@hidden1 = external hidden global i32, align 8
+@hidden2 = external hidden global i32, align 8
+@hidden3 = external hidden global i32, align 8
+
+@switch.table.no_dso_local = private unnamed_addr constant [3 x i32*] [i32* @a1, i32* @b1, i32* @c1], align 8
+
+@switch.table.dso_local = private unnamed_addr constant [3 x i32*] [i32* @a2, i32* @b2, i32* @c2], align 8
+
+@switch.table.hidden = private unnamed_addr constant [3 x i32*] [i32* @hidden0, i32* @hidden1, i32* @hidden2], align 8
+
+@switch.table.string_table = private unnamed_addr constant [3 x i8*]
+ [
+  i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0),
+  i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.1, i64 0, i64 0),
+  i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.2, i64 0, i64 0)
+ ], align 8
+
+@switch.table.string_table_holes = private unnamed_addr constant [4 x i8*]
+   [
+i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0),
+i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str.3, i64 0, i64 0),
+i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.2, i64 0, i64 0),
+i8* getelementptr 

[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

2021-03-04 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

Thanks for explaining that it only affects "-B". While I believe that this 
change won't affect us in Chrome OS, I think it should be reviewed and approved 
by a few Linux distro contributors since there is already known reliance e.g. 
Android on the current behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97993

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


[PATCH] D97902: [Driver] Clarify --gcc-toolchain

2021-03-04 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

thanks, this is much needed documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

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


[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D97993#2605157 , @manojgupta wrote:

> I am not sure of the rationale or upside of this change and why do we want to 
> drop gcc detection? GCC does not need to do the GCC detection because it has 
> the needed information at configure time.

The dropped detection is for -B.  --gcc-toolchain and --sysroot (default "") 
still have GCC detection.

The change is to make Linux cross compilation more predictable, not shadowing 
--gcc-toolchain with -B or system cross compilation packages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97993

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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

If the end goal is to provide a complete libc, and currently the usage is 
expected to shadow system libc (this has to be very careful, as I don't know 
how shadowing some important components like pthread shall work), perhaps the 
name should convey this point?




Comment at: clang/test/Driver/linux-ld.c:235
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \

Empty `// ` actually makes browsing difficult and we generally don't do this 
for other tests. You can omit them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

2021-03-04 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

I am not sure of the rationale or upside of this change and why do we want to 
drop gcc detection? GCC does not need to do the GCC detection because it has 
the needed information at configure time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97993

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


[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: hubert.reinterpretcast, manojgupta, nickdesaulniers, 
phosek.
Herald added a subscriber: danielkiss.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In GCC, if `-B $prefix` is specified, `$prefix` is used to find executable 
files and startup files.
`$prefix/include` is added as an include search directory.

In Clang, we detect GCC installation in various target dependent directories.
`$sysroot/usr` (sysroot defaults to "") is a common directory used by most 
targets.
Such a directory is expected to contain something like 
`lib{,32,64}/gcc{,-cross}/$triple`.
Clang will then construct library/include paths from the directory.
The usage is different from GCC.

This patch drops GCC detection under -B for non-Android targets.
(android-ndk-standalone.cpp & android-stanlone.cpp depend on the -B detection.)
We expect users to specify --gcc-toolchain if they want to customize the 
detection roots.

Note: without --gcc-toolchain, the detected GCC installation under
`$sysroot/usr` (all targets) or `/opt/rh/devtoolset-*/root/usr` (Linux, sysroot
is empty) can override -B specified GCC installation if the system GCC has a
larger version. So the current -B behavior is not very predictable. Hope
dropping the behavior has little impact as users likely already specify
--gcc-toolchain/--sysroot to make the behavior predictable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97993

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/gcc-toolchain.cpp


Index: clang/test/Driver/gcc-toolchain.cpp
===
--- clang/test/Driver/gcc-toolchain.cpp
+++ clang/test/Driver/gcc-toolchain.cpp
@@ -29,3 +29,14 @@
 // CHECK: 
"{{[^"]*}}/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5{{/|}}crtbegin.o"
 // CHECK: "-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5"
 // CHECK: 
"-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5/../../../.."
+
+/// Test we don't detect GCC installation under -B.
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN:   --target=aarch64-suse-linux 
--gcc-toolchain=%S/Inputs/opensuse_42.2_aarch64_tree/usr | \
+// RUN:   FileCheck %s --check-prefix=AARCH64
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN:   --target=aarch64-suse-linux 
-B%S/Inputs/opensuse_42.2_aarch64_tree/usr | \
+// RUN:   FileCheck %s --check-prefix=NO_AARCH64
+
+// AARCH64:Inputs{{[^"]+}}aarch64-suse-linux/{{[^"]+}}crt1.o"
+// NO_AARCH64-NOT: Inputs{{[^"]+}}aarch64-suse-linux/{{[^"]+}}crt1.o"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1907,8 +1907,9 @@
CandidateBiarchTripleAliases);
 
   // Compute the set of prefixes for our search.
-  SmallVector Prefixes(D.PrefixDirs.begin(),
-   D.PrefixDirs.end());
+  SmallVector Prefixes;
+  if (TargetTriple.isAndroid())
+Prefixes.assign(D.PrefixDirs.begin(), D.PrefixDirs.end());
 
   StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
   if (GCCToolchainDir != "") {


Index: clang/test/Driver/gcc-toolchain.cpp
===
--- clang/test/Driver/gcc-toolchain.cpp
+++ clang/test/Driver/gcc-toolchain.cpp
@@ -29,3 +29,14 @@
 // CHECK: "{{[^"]*}}/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5{{/|}}crtbegin.o"
 // CHECK: "-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5"
 // CHECK: "-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5/../../../.."
+
+/// Test we don't detect GCC installation under -B.
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN:   --target=aarch64-suse-linux --gcc-toolchain=%S/Inputs/opensuse_42.2_aarch64_tree/usr | \
+// RUN:   FileCheck %s --check-prefix=AARCH64
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN:   --target=aarch64-suse-linux -B%S/Inputs/opensuse_42.2_aarch64_tree/usr | \
+// RUN:   FileCheck %s --check-prefix=NO_AARCH64
+
+// AARCH64:Inputs{{[^"]+}}aarch64-suse-linux/{{[^"]+}}crt1.o"
+// NO_AARCH64-NOT: Inputs{{[^"]+}}aarch64-suse-linux/{{[^"]+}}crt1.o"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1907,8 +1907,9 @@
CandidateBiarchTripleAliases);
 
   // Compute the set of prefixes for our search.
-  SmallVector Prefixes(D.PrefixDirs.begin(),
-   D.PrefixDirs.end());
+  SmallVector Prefixes;
+  if (TargetTriple.isAndroid())
+Prefixes.assign(D.PrefixDirs.begin(), D.PrefixDirs.end());
 
   StringRef GCCToolchainDir = 

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450
+Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero);
+return RValue::get(Builder.CreateAdd(SizeCall, Extra));
   }

rjmccall wrote:
> ychen wrote:
> > ychen wrote:
> > > rjmccall wrote:
> > > > Okay, so you're implicitly increasing the coroutine size to allow you 
> > > > to round up to get an aligned frame.  But how do you round back down to 
> > > > get the actual pointer that you need to delete?  This just doesn't work.
> > > > 
> > > > You really ought to just be using the aligned `operator new` instead 
> > > > when the required alignment is too high.  If that means checking the 
> > > > alignment "dynamically" before calling `operator new` / `operator 
> > > > delete`, so be it.  In practice, it will not be dynamic because 
> > > > lowering will replace the `coro.align` call with a constant, at which 
> > > > point the branch will be foldable.
> > > > 
> > > > I don't know what to suggest if the aligned `operator new` isn't 
> > > > reliably available on the target OS.  You could outline a function to 
> > > > pick the best allocator/deallocator, I suppose.
> > > Thanks for the review!
> > > 
> > > > Okay, so you're implicitly increasing the coroutine size to allow you 
> > > > to round up to get an aligned frame. But how do you round back down to 
> > > > get the actual pointer that you need to delete? This just doesn't work.
> > > 
> > > Hmm, you're right that I missed the `delete` part, thanks. The adjusted 
> > > amount is constant, I could just dial it down in `coro.free`, right?
> > > 
> > > >  You really ought to just be using the aligned operator new instead 
> > > > when the required alignment is too high. If that means checking the 
> > > > alignment "dynamically" before calling operator new / operator delete, 
> > > > so be it. In practice, it will not be dynamic because lowering will 
> > > > replace the coro.align call with a constant, at which point the branch 
> > > > will be foldable.
> > >  
> > > That's my intuition at first. But spec is written in a way suggesting 
> > > (IMHO) that the aligned version should not be used? What if the user 
> > > specify their own allocator, then which one they should use?
> > Sorry, I meant the adjusted amount is coro.align - std::max_align_t,  I 
> > could subtract it in `coro.free` . I think it should work?
> No, because the adjustment you have to do in `coro.alloc` isn't just an 
> addition, it's an addition plus a mask, which isn't reversible.  Suppose the 
> frame needs to be 32-byte-aligned, but the allocator only promises 8-byte 
> alignment.  The problem is that when you go to free a frame pointer, and you 
> see that it's 32-byte-aligned (which, again, it always will be), the pointer 
> you got from the allocator might be the frame pointer minus any of 8, 16, or 
> 24 — or it might be exactly the same.  The only way to reverse that is to 
> store some sort of cookie, either the amount to subtract or even just the 
> original pointer.
> 
> Now, if you could change the entire coroutine ABI, you could make the frame 
> handle that you pass around be the unadjusted pointer and then just repeat 
> the adjustment every time you enter the coroutine.  But that doesn't work 
> because the ABI relies on things like the promise being at a reliable offset 
> from the frame handle.
> 
> I think the best solution would be to figure out a way to use an aligned 
> allocator, which at worst does this in a more systematic way and at best can 
> actually just satisfy your requirement directly without any overhead.  If you 
> can't do that, adding an offset to the frame would be best; if you can't do 
> that, doing it as a cookie is okay.
> 
> > That's my intuition at first. But spec is written in a way suggesting 
> > (IMHO) that the aligned version should not be used? What if the user 
> > specify their own allocator, then which one they should use?
> 
> It seems like a spec bug that this doesn't use aligned allocators even when 
> they're available.  If there's an aligned allocator available, I think this 
> should essentially do dynamically what it would normally do statically, i.e.:
> 
> ```
> void *allocation = alignment > __STDCPP_DEFAULT_NEW_ALIGNMENT__ ? operator 
> new(size, align_val_t(alignment)) : operator new(size);
> ```
> 
> This would ODR-use both allocation functions, of course.
> 
> Maybe it's right to do this cookie thing if we can't rely on an aligned 
> allocator, like if the promise class provides only an `operator new(size_t)`.
> No, because the adjustment you have to do in `coro.alloc` isn't just an 
> addition, it's an addition plus a mask, which isn't reversible.  Suppose the 
> frame needs to be 32-byte-aligned, but the allocator only promises 8-byte 
> alignment.  The problem is that when you go to free a frame pointer, and you 
> see that it's 32-byte-aligned (which, again, it always will be), the pointer 
> 

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Siva Chandra via Phabricator via cfe-commits
sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

sivachandra wrote:
> phosek wrote:
> > This wouldn't handle the case where you use `-nolibc path/to/libc.a` in 
> > which case you'd have to manually pass in the `libllvmllibc.a`, but I'm not 
> > sure if that's a case we care about.
> Yeah, its hard to cater to all combinations.  In this case though, assuming 
> `libllvmlibc.a` is available in path, one can add `-lllvmlibc` in the right 
> place. 
Another point to note is that `-nolibc` does not prevent `-lm` from getting 
added for C++ compilations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Siva Chandra via Phabricator via cfe-commits
sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

phosek wrote:
> This wouldn't handle the case where you use `-nolibc path/to/libc.a` in which 
> case you'd have to manually pass in the `libllvmllibc.a`, but I'm not sure if 
> that's a case we care about.
Yeah, its hard to cater to all combinations.  In this case though, assuming 
`libllvmlibc.a` is available in path, one can add `-lllvmlibc` in the right 
place. 



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:684
+WithLLVMLibc.push_back("-lllvmlibc");
+WithLLVMLibc.push_back(Arg.data());
+  } else {

phosek wrote:
> You can move this after the condition and omit the `else` branch.
[Shame cube] Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Siva Chandra via Phabricator via cfe-commits
sivachandra updated this revision to Diff 328340.
sivachandra added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,23 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+  }
+  WithLLVMLibc.push_back(Arg.data());
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// 

[PATCH] D97990: [clang] Always provide relevant subobject for 'no viable comparison'

2021-03-04 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov created this revision.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Diagnostics for defaulted no viable not-equal and relational operators
where not providing relevant subobject in diagnostics, unlike the cases
for equal and three-way operators.

This patch changes it so that it is provided for all cases.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97990

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
  clang/test/CXX/class/class.compare/class.eq/p2.cpp


Index: clang/test/CXX/class/class.compare/class.eq/p2.cpp
===
--- clang/test/CXX/class/class.compare/class.eq/p2.cpp
+++ clang/test/CXX/class/class.compare/class.eq/p2.cpp
@@ -18,20 +18,17 @@
 struct H1 {
   bool operator==(const H1 &) const = default;
   bool operator<(const H1 &) const = default; // expected-warning {{implicitly 
deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
-  void (*x)();
+  void (*x)(); // expected-note {{because there is no viable comparison 
function for member 'x'}}
 };
 struct H2 {
   bool operator==(const H2 &) const = default;
   bool operator<(const H2 &) const = default; // expected-warning {{implicitly 
deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
-  void (H2::*x)();
+  void (H2::*x)(); // expected-note {{because there is no viable comparison 
function for member 'x'}}
 };
 struct H3 {
   bool operator==(const H3 &) const = default;
   bool operator<(const H3 &) const = default; // expected-warning {{implicitly 
deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
-  int H3::*x;
+  int H3::*x; // expected-note {{because there is no viable comparison 
function for member 'x'}}
 };
 
 template struct X {
Index: clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
@@ -40,11 +40,10 @@
 }
 
 struct A3 {
-  int  // expected-note {{because class 'A3' has a reference member}}
+  int  // expected-note 2{{because class 'A3' has a reference member}}
 
   bool operator==(const A3 &) const = default; // expected-warning 
{{implicitly deleted}}
   bool operator<(const A3 &) const = default;  // expected-warning 
{{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
 };
 
 struct B1 {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7857,6 +7857,13 @@
   }
 
   if (Diagnose == ExplainDeleted) {
+if (Subobj.Kind == DefaultedComparisonSubobject::CompleteObject) {
+  QualType ParamLvalType =
+  FD->getParamDecl(0)->getType().getNonReferenceType();
+  getDerived().visitSubobjects(R, RD, ParamLvalType.getQualifiers());
+  R = Result::deleted();
+  break;
+}
 S.Diag(Subobj.Loc, diag::note_defaulted_comparison_no_viable_function)
 << FD << Subobj.Kind << Subobj.Decl;
 


Index: clang/test/CXX/class/class.compare/class.eq/p2.cpp
===
--- clang/test/CXX/class/class.compare/class.eq/p2.cpp
+++ clang/test/CXX/class/class.compare/class.eq/p2.cpp
@@ -18,20 +18,17 @@
 struct H1 {
   bool operator==(const H1 &) const = default;
   bool operator<(const H1 &) const = default; // expected-warning {{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
-  void (*x)();
+  void (*x)(); // expected-note {{because there is no viable comparison function for member 'x'}}
 };
 struct H2 {
   bool operator==(const H2 &) const = default;
   bool operator<(const H2 &) const = default; // expected-warning {{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
-  void (H2::*x)();
+  void (H2::*x)(); // expected-note {{because there is no viable comparison function for member 'x'}}
 };
 struct H3 {
   bool operator==(const H3 &) const = default;
   bool operator<(const H3 &) const = default; // expected-warning {{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
-  int H3::*x;
+  int H3::*x; // expected-note {{because there is no viable comparison function for member 'x'}}
 };
 
 template struct X {
Index: clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
+++ 

[clang] cedc532 - Fix clang for header move in LLVM/IR

2021-03-04 Thread David Blaikie via cfe-commits

Author: David Blaikie
Date: 2021-03-04T16:20:44-08:00
New Revision: cedc53254a5d2e04e79cc0e7bf5a8c71fafa295e

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

LOG: Fix clang for header move in LLVM/IR

Added: 


Modified: 
clang/lib/CodeGen/CGObjC.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index 663666b6bf8b..a5d19291e722 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -23,7 +23,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/Analysis/ObjCARCUtil.h"
+#include "llvm/IR/ObjCARCUtil.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"



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


[PATCH] D97894: [Driver] Drop $sysroot/usr special case from Gentoo gcc-config detection

2021-03-04 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

> With the special rule, it is: if --gcc-toolchain is $sysroot/usr, suppress 
> sysroot GCC detection as well.
>
> Clang -B and --gcc-toolchain have some weird behaviors. Hope you can share 
> your opinions on 
> https://lists.llvm.org/pipermail/cfe-dev/2021-March/067827.html

Sure, I have replied on the thread with chrome os usage explanation. I also 
noticed that we also use "--gcc-toolchain" as well so the whole lot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97894

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


[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-04 Thread Adrian McCarthy via Phabricator via cfe-commits
amccarth added a comment.

From a Windows point of view, this LGTM.




Comment at: clang/lib/Frontend/FrontendActions.cpp:841
 }
   }
 

The preceding block that detects the type of line separator seems ripe for 
factoring out into a separate function.  It's a lot of low-level detail that 
visually outweighs the primary intent of this method.

But I'm fine with the change as it doesn't impact Windows and--as far as I 
understand--Posix platforms don't distinguish between text and binary mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

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


[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450
+Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero);
+return RValue::get(Builder.CreateAdd(SizeCall, Extra));
   }

ychen wrote:
> ychen wrote:
> > rjmccall wrote:
> > > Okay, so you're implicitly increasing the coroutine size to allow you to 
> > > round up to get an aligned frame.  But how do you round back down to get 
> > > the actual pointer that you need to delete?  This just doesn't work.
> > > 
> > > You really ought to just be using the aligned `operator new` instead when 
> > > the required alignment is too high.  If that means checking the alignment 
> > > "dynamically" before calling `operator new` / `operator delete`, so be 
> > > it.  In practice, it will not be dynamic because lowering will replace 
> > > the `coro.align` call with a constant, at which point the branch will be 
> > > foldable.
> > > 
> > > I don't know what to suggest if the aligned `operator new` isn't reliably 
> > > available on the target OS.  You could outline a function to pick the 
> > > best allocator/deallocator, I suppose.
> > Thanks for the review!
> > 
> > > Okay, so you're implicitly increasing the coroutine size to allow you to 
> > > round up to get an aligned frame. But how do you round back down to get 
> > > the actual pointer that you need to delete? This just doesn't work.
> > 
> > Hmm, you're right that I missed the `delete` part, thanks. The adjusted 
> > amount is constant, I could just dial it down in `coro.free`, right?
> > 
> > >  You really ought to just be using the aligned operator new instead when 
> > > the required alignment is too high. If that means checking the alignment 
> > > "dynamically" before calling operator new / operator delete, so be it. In 
> > > practice, it will not be dynamic because lowering will replace the 
> > > coro.align call with a constant, at which point the branch will be 
> > > foldable.
> >  
> > That's my intuition at first. But spec is written in a way suggesting 
> > (IMHO) that the aligned version should not be used? What if the user 
> > specify their own allocator, then which one they should use?
> Sorry, I meant the adjusted amount is coro.align - std::max_align_t,  I could 
> subtract it in `coro.free` . I think it should work?
No, because the adjustment you have to do in `coro.alloc` isn't just an 
addition, it's an addition plus a mask, which isn't reversible.  Suppose the 
frame needs to be 32-byte-aligned, but the allocator only promises 8-byte 
alignment.  The problem is that when you go to free a frame pointer, and you 
see that it's 32-byte-aligned (which, again, it always will be), the pointer 
you got from the allocator might be the frame pointer minus any of 8, 16, or 24 
— or it might be exactly the same.  The only way to reverse that is to store 
some sort of cookie, either the amount to subtract or even just the original 
pointer.

Now, if you could change the entire coroutine ABI, you could make the frame 
handle that you pass around be the unadjusted pointer and then just repeat the 
adjustment every time you enter the coroutine.  But that doesn't work because 
the ABI relies on things like the promise being at a reliable offset from the 
frame handle.

I think the best solution would be to figure out a way to use an aligned 
allocator, which at worst does this in a more systematic way and at best can 
actually just satisfy your requirement directly without any overhead.  If you 
can't do that, adding an offset to the frame would be best; if you can't do 
that, doing it as a cookie is okay.

> That's my intuition at first. But spec is written in a way suggesting (IMHO) 
> that the aligned version should not be used? What if the user specify their 
> own allocator, then which one they should use?

It seems like a spec bug that this doesn't use aligned allocators even when 
they're available.  If there's an aligned allocator available, I think this 
should essentially do dynamically what it would normally do statically, i.e.:

```
void *allocation = alignment > __STDCPP_DEFAULT_NEW_ALIGNMENT__ ? operator 
new(size, align_val_t(alignment)) : operator new(size);
```

This would ODR-use both allocation functions, of course.

Maybe it's right to do this cookie thing if we can't rely on an aligned 
allocator, like if the promise class provides only an `operator new(size_t)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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


[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate

2021-03-04 Thread Heejin Ahn via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG561abd83ffec: [WebAssembly] Disable uses of 
__clang_call_terminate (authored by aheejin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97834

Files:
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/wasm-eh.cpp
  llvm/lib/CodeGen/WasmEHPrepare.cpp
  llvm/lib/Target/WebAssembly/CMakeLists.txt
  llvm/lib/Target/WebAssembly/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyHandleEHTerminatePads.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
  llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
  llvm/test/CodeGen/WebAssembly/eh-lsda.ll
  llvm/test/CodeGen/WebAssembly/exception.ll
  llvm/test/CodeGen/WebAssembly/wasmehprepare.ll

Index: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
===
--- llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
+++ llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
@@ -132,59 +132,6 @@
   ret void
 }
 
-; A cleanuppad with a call to __clang_call_terminate().
-;
-; void foo();
-; void test2() {
-;   try {
-; foo();
-;   } catch (...) {
-; foo();
-;   }
-; }
-define void @test2() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
-; CHECK-LABEL: @test2
-entry:
-  invoke void @foo()
-  to label %try.cont unwind label %catch.dispatch
-
-catch.dispatch:   ; preds = %entry
-  %0 = catchswitch within none [label %catch.start] unwind to caller
-
-catch.start:  ; preds = %catch.dispatch
-  %1 = catchpad within %0 [i8* null]
-  %2 = call i8* @llvm.wasm.get.exception(token %1)
-  %3 = call i32 @llvm.wasm.get.ehselector(token %1)
-  %4 = call i8* @__cxa_begin_catch(i8* %2) [ "funclet"(token %1) ]
-  invoke void @foo() [ "funclet"(token %1) ]
-  to label %invoke.cont1 unwind label %ehcleanup
-
-invoke.cont1: ; preds = %catch.start
-  call void @__cxa_end_catch() [ "funclet"(token %1) ]
-  catchret from %1 to label %try.cont
-
-try.cont: ; preds = %entry, %invoke.cont1
-  ret void
-
-ehcleanup:; preds = %catch.start
-  %5 = cleanuppad within %1 []
-  invoke void @__cxa_end_catch() [ "funclet"(token %5) ]
-  to label %invoke.cont2 unwind label %terminate
-
-invoke.cont2: ; preds = %ehcleanup
-  cleanupret from %5 unwind to caller
-
-terminate:; preds = %ehcleanup
-  %6 = cleanuppad within %5 []
-  %7 = call i8* @llvm.wasm.get.exception(token %6)
-  call void @__clang_call_terminate(i8* %7) [ "funclet"(token %6) ]
-  unreachable
-; CHECK: terminate:
-; CHECK-NEXT: cleanuppad
-; CHECK-NEXT:   %[[EXN:.*]] = call i8* @llvm.wasm.catch
-; CHECK-NEXT:   call void @__clang_call_terminate(i8* %[[EXN]])
-}
-
 ; PHI demotion test. Only the phi before catchswitch should be demoted; the phi
 ; before cleanuppad should NOT.
 ;
@@ -194,7 +141,7 @@
 ;   ~Temp() {}
 ; };
 ;
-; void test3() {
+; void test2() {
 ;   int num;
 ;   try {
 ; Temp t;
@@ -214,8 +161,8 @@
 ; bar(num);
 ;   }
 ; }
-define void @test3() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
-; CHECK-LABEL: @test3
+define void @test2() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+; CHECK-LABEL: @test2
 entry:
   %t = alloca %struct.Temp, align 1
   invoke void @foo()
@@ -279,8 +226,8 @@
 ; Tests if instructions after a call to @llvm.wasm.throw are deleted and the
 ; BB's dead children are deleted.
 
-; CHECK-LABEL: @test4
-define i32 @test4(i1 %b, i8* %p) {
+; CHECK-LABEL: @test3
+define i32 @test3(i1 %b, i8* %p) {
 entry:
   br i1 %b, label %bb.true, label %bb.false
 
@@ -308,14 +255,22 @@
 declare void @bar(i32)
 declare %struct.Temp* @_ZN4TempD2Ev(%struct.Temp* returned)
 declare i32 @__gxx_wasm_personality_v0(...)
-declare i8* @llvm.wasm.get.exception(token)
-declare i32 @llvm.wasm.get.ehselector(token)
-declare i32 @llvm.eh.typeid.for(i8*)
-declare void @llvm.wasm.throw(i32, i8*)
-declare void @llvm.wasm.rethrow()
+; Function Attrs: nounwind
+declare i8* @llvm.wasm.get.exception(token) #0
+; Function Attrs: nounwind
+declare i32 @llvm.wasm.get.ehselector(token) #0
+; Function Attrs: nounwind
+declare i32 @llvm.eh.typeid.for(i8*) #0
+; Function Attrs: noreturn
+declare void @llvm.wasm.throw(i32, i8*) #1
+; Function Attrs: noreturn
+declare void @llvm.wasm.rethrow() #1
 declare i8* @__cxa_begin_catch(i8*)
 declare void @__cxa_end_catch()
-declare void @__clang_call_terminate(i8*)
+declare void 

[clang] 561abd8 - [WebAssembly] Disable uses of __clang_call_terminate

2021-03-04 Thread Heejin Ahn via cfe-commits

Author: Heejin Ahn
Date: 2021-03-04T14:26:35-08:00
New Revision: 561abd83ffecc8d4ba8fcbbbcadb31efc55985c2

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

LOG: [WebAssembly] Disable uses of __clang_call_terminate

Background:

Wasm EH, while using Windows EH (catchpad/cleanuppad based) IR, uses
Itanium-based libraries and ABIs with some modifications.

`__clang_call_terminate` is a wrapper generated in Clang's Itanium C++
ABI implementation. It contains this code, in C-style pseudocode:
```
void __clang_call_terminate(void *exn) {
  __cxa_begin_catch(exn);
  std::terminate();
}
```
So this function is a wrapper to call `__cxa_begin_catch` on the
exception pointer before termination.

In Itanium ABI, this function is called when another exception is thrown
while processing an exception. The pointer for this second, violating
exception is passed as the argument of this `__clang_call_terminate`,
which calls `__cxa_begin_catch` with that pointer and calls
`std::terminate` to terminate the program.

The spec (https://libcxxabi.llvm.org/spec.html) for `__cxa_begin_catch`
says,
```
When the personality routine encounters a termination condition, it
will call __cxa_begin_catch() to mark the exception as handled and then
call terminate(), which shall not return to its caller.
```

In wasm EH's Clang implementation, this function is called from
cleanuppads that terminates the program, which we also call terminate
pads. Cleanuppads normally don't access the thrown exception and the
wasm backend converts them to `catch_all` blocks. But because we need
the exception pointer in this cleanuppad, we generate
`wasm.get.exception` intrinsic (which will eventually be lowered to
`catch` instruction) as we do in the catchpads. But because terminate
pads are cleanup pads and should run even when a foreign exception is
thrown, so what we have been doing is:
1. In `WebAssemblyLateEHPrepare::ensureSingleBBTermPads()`, we make sure
terminate pads are in this simple shape:
```
%exn = catch
call @__clang_call_terminate(%exn)
unreachable
```
2. In `WebAssemblyHandleEHTerminatePads` pass at the end of the
pipeline, we attach a `catch_all` to terminate pads, so they will be in
this form:
```
%exn = catch
call @__clang_call_terminate(%exn)
unreachable
catch_all
call @std::terminate()
unreachable
```
In `catch_all` part, we don't have the exception pointer, so we call
`std::terminate()` directly. The reason we ran HandleEHTerminatePads at
the end of the pipeline, separate from LateEHPrepare, was it was
convenient to assume there was only a single `catch` part per `try`
during CFGSort and CFGStackify.

---

Problem:

While it thinks terminate pads could have been possibly split or calls
to `__clang_call_terminate` could have been duplicated,
`WebAssemblyLateEHPrepare::ensureSingleBBTermPads()` assumes terminate
pads contain no more than calls to `__clang_call_terminate` and
`unreachable` instruction. I assumed that because in LLVM very limited
forms of transformations are done to catchpads and cleanuppads to
maintain the scoping structure. But it turned out to be incorrect;
passes can merge cleanuppads into one, including terminate pads, as long
as the new code has a correct scoping structure. One pass that does this
I observed was `SimplifyCFG`, but there can be more. After this
transformation, a single cleanuppad can contain any number of other
instructions with the call to `__clang_call_terminate` and can span many
BBs. It wouldn't be practical to duplicate all these BBs within the
cleanuppad to generate the equivalent `catch_all` blocks, only with
calls to `__clang_call_terminate` replaced by calls to `std::terminate`.

Unless we do more complicated transformation to split those calls to
`__clang_call_terminate` into a separate cleanuppad, it is tricky to
solve.

---

Solution (?):

This CL just disables the generation and use of `__clang_call_terminate`
and calls `std::terminate()` directly in its place.

The possible downside of this approach can be, because the Itanium ABI
intended to "mark" the violating exception handled, we don't do that
anymore. What `__cxa_begin_catch` actually does is increment the
exception's handler count and decrement the uncaught exception count,
which in my opinion do not matter much given that we are about to
terminate the program anyway. Also it does not affect info like stack
traces that can be possibly shown to developers.

And while we use a variant of Itanium EH ABI, we can make some
deviations if we choose to; we are already different in that in the
current version of the EH spec we don't support two-phase unwinding. We
can possibly consider a more complicated transformation later to
reenable this, but I don't think that has high priority.

Changes in this CL contains:
- In Clang, we don't generate a call to `wasm.get.exception()` intrinsic
  and 

[PATCH] D97983: [clangd] strictly respect formatting range

2021-03-04 Thread Quentin Chateau via Phabricator via cfe-commits
qchateau updated this revision to Diff 328314.
qchateau added a comment.

fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97983

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/test/formatting.test


Index: clang-tools-extra/clangd/test/formatting.test
===
--- clang-tools-extra/clangd/test/formatting.test
+++ clang-tools-extra/clangd/test/formatting.test
@@ -8,19 +8,6 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:{
-# CHECK-NEXT:  "newText": "\n  ",
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": 4,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": 19,
-# CHECK-NEXT:  "line": 0
-# CHECK-NEXT:}
-# CHECK-NEXT:  }
-# CHECK-NEXT:},
-# CHECK-NEXT:{
 # CHECK-NEXT:  "newText": " ",
 # CHECK-NEXT:  "range": {
 # CHECK-NEXT:"end": {
@@ -45,19 +32,6 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  }
-# CHECK-NEXT:},
-# CHECK-NEXT:{
-# CHECK-NEXT:  "newText": "\n  ",
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": 4,
-# CHECK-NEXT:  "line": 2
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": 12,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:}
-# CHECK-NEXT:  }
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 ---
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -423,10 +423,24 @@
 if (!Changed)
   return CB(Changed.takeError());
 
-CB(IncludeReplaces.merge(format::reformat(
+auto Replacements = IncludeReplaces.merge(format::reformat(
 Style, *Changed,
 tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-File)));
+File));
+clang::tooling::Replacements ReplacementsInRange;
+for (const auto  : Replacements) {
+  tooling::Range ReplRange{
+  Repl.getOffset(),
+  std::max(Repl.getLength(),
+ Repl.getReplacementText().size()),
+  };
+  if (ReplRange.overlapsWith(Ranges.front())) {
+if (auto Err = ReplacementsInRange.add(Repl)) {
+  CB(std::move(Err));
+}
+  }
+}
+CB(ReplacementsInRange);
   };
   WorkScheduler->runQuick("Format", File, std::move(Action));
 }


Index: clang-tools-extra/clangd/test/formatting.test
===
--- clang-tools-extra/clangd/test/formatting.test
+++ clang-tools-extra/clangd/test/formatting.test
@@ -8,19 +8,6 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:{
-# CHECK-NEXT:  "newText": "\n  ",
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": 4,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": 19,
-# CHECK-NEXT:  "line": 0
-# CHECK-NEXT:}
-# CHECK-NEXT:  }
-# CHECK-NEXT:},
-# CHECK-NEXT:{
 # CHECK-NEXT:  "newText": " ",
 # CHECK-NEXT:  "range": {
 # CHECK-NEXT:"end": {
@@ -45,19 +32,6 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  }
-# CHECK-NEXT:},
-# CHECK-NEXT:{
-# CHECK-NEXT:  "newText": "\n  ",
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": 4,
-# CHECK-NEXT:  "line": 2
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": 12,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:}
-# CHECK-NEXT:  }
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 ---
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -423,10 +423,24 @@
 if (!Changed)
   return CB(Changed.takeError());
 
-CB(IncludeReplaces.merge(format::reformat(
+auto Replacements = IncludeReplaces.merge(format::reformat(
 Style, *Changed,
 tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-File)));
+File));
+clang::tooling::Replacements ReplacementsInRange;
+for (const auto  : Replacements) {
+  tooling::Range ReplRange{
+  Repl.getOffset(),
+  std::max(Repl.getLength(),
+ Repl.getReplacementText().size()),
+  };
+  if (ReplRange.overlapsWith(Ranges.front())) {
+if 

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

This wouldn't handle the case where you use `-nolibc path/to/libc.a` in which 
case you'd have to manually pass in the `libllvmllibc.a`, but I'm not sure if 
that's a case we care about.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:684
+WithLLVMLibc.push_back("-lllvmlibc");
+WithLLVMLibc.push_back(Arg.data());
+  } else {

You can move this after the condition and omit the `else` branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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


[PATCH] D97983: [clangd] strictly respect formatting range

2021-03-04 Thread Quentin Chateau via Phabricator via cfe-commits
qchateau created this revision.
qchateau added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
qchateau requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Clang format will often format a range of code wider then the requested format 
range. Returning a replacement outside of the requested range can mess with 
clients. Keeping the formatting in range of what the client asks gives more 
expected results and will limit the impact of partial formatting in diffs.

A specific example: a problem often occurs when using VSCode in "format 
modified lines" mode. The client will ask clangd to format two non-continugous 
lines, separately. Clangd replies to both requests with replacements for both 
lines, effectively providing four replacements in total. VSCode will either 
apply replacements twice (messing up the results) or simply error out and skip 
the formatting.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97983

Files:
  clang-tools-extra/clangd/ClangdServer.cpp


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -423,10 +423,24 @@
 if (!Changed)
   return CB(Changed.takeError());
 
-CB(IncludeReplaces.merge(format::reformat(
+auto Replacements = IncludeReplaces.merge(format::reformat(
 Style, *Changed,
 tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-File)));
+File));
+clang::tooling::Replacements ReplacementsInRange;
+for (const auto  : Replacements) {
+  tooling::Range ReplRange{
+  Repl.getOffset(),
+  std::max(Repl.getLength(),
+ Repl.getReplacementText().size()),
+  };
+  if (ReplRange.overlapsWith(Ranges.front())) {
+if (auto Err = ReplacementsInRange.add(Repl)) {
+  CB(std::move(Err));
+}
+  }
+}
+CB(ReplacementsInRange);
   };
   WorkScheduler->runQuick("Format", File, std::move(Action));
 }


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -423,10 +423,24 @@
 if (!Changed)
   return CB(Changed.takeError());
 
-CB(IncludeReplaces.merge(format::reformat(
+auto Replacements = IncludeReplaces.merge(format::reformat(
 Style, *Changed,
 tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-File)));
+File));
+clang::tooling::Replacements ReplacementsInRange;
+for (const auto  : Replacements) {
+  tooling::Range ReplRange{
+  Repl.getOffset(),
+  std::max(Repl.getLength(),
+ Repl.getReplacementText().size()),
+  };
+  if (ReplRange.overlapsWith(Ranges.front())) {
+if (auto Err = ReplacementsInRange.add(Repl)) {
+  CB(std::move(Err));
+}
+  }
+}
+CB(ReplacementsInRange);
   };
   WorkScheduler->runQuick("Format", File, std::move(Action));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

2021-03-04 Thread Tim Wojtulewicz via Phabricator via cfe-commits
timwoj updated this revision to Diff 328308.
timwoj added a comment.

Rebase onto master again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14767,6 +14767,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -14791,6 +14792,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -14825,7 +14909,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -14833,7 +14917,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -14843,7 +14927,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -14851,9 +14935,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -14866,17 +14950,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
   

[PATCH] D97894: [Driver] Drop $sysroot/usr special case from Gentoo gcc-config detection

2021-03-04 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

In Chrome OS, we currently both "-B" and "--prefix". "-B" points to binutils 
bin directory and the "--prefix" has the binutils directory + target suffix. 
Should we drop "-B" and still get the same behavior?

Sample invocation: '/usr/bin/clang++' '--sysroot=/usr/x86_64-cros-linux-gnu 
'-fcrash-diagnostics-dir=/tmp/clang_crash_diagnostics' '-fcommon' 
'-fstack-protector-strong' '-fPIE' '-pie' '-D_FORTIFY_SOURCE=2' 
'-fno-omit-frame-pointer' 
'--prefix=../../../../../../usr/libexec/gcc/x86_64-cros-linux-gnu/x86_64-cros-linux-gnu-'
 'foo.o' '-o' 'main' 
'-B../../../../../../usr/libexec/gcc/x86_64-cros-linux-gnu' '-target' 
'x86_64-cros-linux-gnu'

I am not yet able to test this change in Chrome OS thoroughly yet because of 
some CQ issues. But manually checking the command lines for a few common cases 
does not show any difference so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97894

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


[clang] 1c2e7d2 - [MS] Fix crash involving gnu stmt exprs and inalloca

2021-03-04 Thread Reid Kleckner via cfe-commits

Author: Reid Kleckner
Date: 2021-03-04T13:57:46-08:00
New Revision: 1c2e7d200df27e91631ba300965245518bfe252c

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

LOG: [MS] Fix crash involving gnu stmt exprs and inalloca

Use a WeakTrackingVH to cope with the stmt emission logic that cleans up
unreachable blocks. This invalidates the reference to the deferred
replacement placeholder. Cope with it.

Fixes PR25102 (from 2015!)

Added: 
clang/test/CodeGenCXX/inalloca-stmtexpr.cpp

Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index f5411daaa677..edcaa528437b 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4436,7 +4436,8 @@ llvm::CallBase 
*CodeGenFunction::EmitCallOrInvoke(llvm::FunctionCallee Callee,
 
 void CodeGenFunction::deferPlaceholderReplacement(llvm::Instruction *Old,
   llvm::Value *New) {
-  DeferredReplacements.push_back(std::make_pair(Old, New));
+  DeferredReplacements.push_back(
+  std::make_pair(llvm::WeakTrackingVH(Old), New));
 }
 
 namespace {

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 6d95adcb6ef0..53bf69f8f86d 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -452,13 +452,13 @@ void CodeGenFunction::FinishFunction(SourceLocation 
EndLoc) {
   if (CGM.getCodeGenOpts().EmitDeclMetadata)
 EmitDeclMetadata();
 
-  for (SmallVectorImpl 
>::iterator
-   I = DeferredReplacements.begin(),
-   E = DeferredReplacements.end();
-   I != E; ++I) {
-I->first->replaceAllUsesWith(I->second);
-I->first->eraseFromParent();
+  for (const auto  : DeferredReplacements) {
+if (llvm::Value *Old = R.first) {
+  Old->replaceAllUsesWith(R.second);
+  cast(Old)->eraseFromParent();
+}
   }
+  DeferredReplacements.clear();
 
   // Eliminate CleanupDestSlot alloca by replacing it with SSA values and
   // PHIs if the current function is a coroutine. We don't do it for all

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h 
b/clang/lib/CodeGen/CodeGenFunction.h
index 8ef0de018a98..2ce87ac7c8e3 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4539,8 +4539,8 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   void deferPlaceholderReplacement(llvm::Instruction *Old, llvm::Value *New);
 
-  llvm::SmallVector, 4>
-  DeferredReplacements;
+  llvm::SmallVector, 4>
+  DeferredReplacements;
 
   /// Set the address of a local variable.
   void setAddrOfLocalVar(const VarDecl *VD, Address Addr) {

diff  --git a/clang/test/CodeGenCXX/inalloca-stmtexpr.cpp 
b/clang/test/CodeGenCXX/inalloca-stmtexpr.cpp
new file mode 100644
index ..e7ae2cb4e703
--- /dev/null
+++ b/clang/test/CodeGenCXX/inalloca-stmtexpr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 %s -emit-llvm -triple i686-windows-msvc -o - | FileCheck %s
+
+// Statement allow the user to exit the evaluation scope of a CallExpr without
+// executing the call. Check that clang generates reasonable IR for that case.
+
+// Not trivially copyable, subject to inalloca.
+struct Foo {
+  int x;
+  Foo();
+  ~Foo();
+};
+
+void inalloca(Foo x, Foo y);
+
+// PR25102: In this case, clang attempts to clean up unreachable blocks 
*during*
+// IR generation. inalloca defers some RAUW operations to the end of codegen,
+// and those references would become stale when the unreachable call to
+// 'inalloca' got deleted.
+extern "C" void pr25102() {
+  inalloca(Foo(), ({
+ goto out;
+ Foo();
+   }));
+out:;
+}
+
+// CHECK-LABEL: define dso_local void @pr25102()
+// CHECK: br label %out
+// CHECK: out:
+// CHECK: ret void
+
+bool cond();
+extern "C" void seqAbort() {
+  inalloca(Foo(), ({
+ if (cond())
+   goto out;
+ Foo();
+   }));
+out:;
+}
+
+// FIXME: This can cause a stack leak. We should really have a "normal" cleanup
+// that goto branches through.
+// CHECK-LABEL: define dso_local void @seqAbort()
+// CHECK: alloca inalloca <{ %struct.Foo, %struct.Foo }>
+// CHECK: call zeroext i1 @"?cond@@YA_NXZ"()
+// CHECK: br i1
+// CHECK: br label %out
+// CHECK: call void @"?inalloca@@YAXUFoo@@0@Z"(<{ %struct.Foo, %struct.Foo }>* 
inalloca %{{.*}})
+// CHECK: call void @llvm.stackrestore(i8* %inalloca.save)
+// CHECK: out:



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


[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450
+Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero);
+return RValue::get(Builder.CreateAdd(SizeCall, Extra));
   }

ychen wrote:
> rjmccall wrote:
> > Okay, so you're implicitly increasing the coroutine size to allow you to 
> > round up to get an aligned frame.  But how do you round back down to get 
> > the actual pointer that you need to delete?  This just doesn't work.
> > 
> > You really ought to just be using the aligned `operator new` instead when 
> > the required alignment is too high.  If that means checking the alignment 
> > "dynamically" before calling `operator new` / `operator delete`, so be it.  
> > In practice, it will not be dynamic because lowering will replace the 
> > `coro.align` call with a constant, at which point the branch will be 
> > foldable.
> > 
> > I don't know what to suggest if the aligned `operator new` isn't reliably 
> > available on the target OS.  You could outline a function to pick the best 
> > allocator/deallocator, I suppose.
> Thanks for the review!
> 
> > Okay, so you're implicitly increasing the coroutine size to allow you to 
> > round up to get an aligned frame. But how do you round back down to get the 
> > actual pointer that you need to delete? This just doesn't work.
> 
> Hmm, you're right that I missed the `delete` part, thanks. The adjusted 
> amount is constant, I could just dial it down in `coro.free`, right?
> 
> >  You really ought to just be using the aligned operator new instead when 
> > the required alignment is too high. If that means checking the alignment 
> > "dynamically" before calling operator new / operator delete, so be it. In 
> > practice, it will not be dynamic because lowering will replace the 
> > coro.align call with a constant, at which point the branch will be foldable.
>  
> That's my intuition at first. But spec is written in a way suggesting (IMHO) 
> that the aligned version should not be used? What if the user specify their 
> own allocator, then which one they should use?
Sorry, I meant the adjusted amount is coro.align - std::max_align_t,  I could 
subtract it in `coro.free` . I think it should work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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


[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450
+Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero);
+return RValue::get(Builder.CreateAdd(SizeCall, Extra));
   }

rjmccall wrote:
> Okay, so you're implicitly increasing the coroutine size to allow you to 
> round up to get an aligned frame.  But how do you round back down to get the 
> actual pointer that you need to delete?  This just doesn't work.
> 
> You really ought to just be using the aligned `operator new` instead when the 
> required alignment is too high.  If that means checking the alignment 
> "dynamically" before calling `operator new` / `operator delete`, so be it.  
> In practice, it will not be dynamic because lowering will replace the 
> `coro.align` call with a constant, at which point the branch will be foldable.
> 
> I don't know what to suggest if the aligned `operator new` isn't reliably 
> available on the target OS.  You could outline a function to pick the best 
> allocator/deallocator, I suppose.
Thanks for the review!

> Okay, so you're implicitly increasing the coroutine size to allow you to 
> round up to get an aligned frame. But how do you round back down to get the 
> actual pointer that you need to delete? This just doesn't work.

Hmm, you're right that I missed the `delete` part, thanks. The adjusted amount 
is constant, I could just dial it down in `coro.free`, right?

>  You really ought to just be using the aligned operator new instead when the 
> required alignment is too high. If that means checking the alignment 
> "dynamically" before calling operator new / operator delete, so be it. In 
> practice, it will not be dynamic because lowering will replace the coro.align 
> call with a constant, at which point the branch will be foldable.
 
That's my intuition at first. But spec is written in a way suggesting (IMHO) 
that the aligned version should not be used? What if the user specify their own 
allocator, then which one they should use?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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


[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450
+Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero);
+return RValue::get(Builder.CreateAdd(SizeCall, Extra));
   }

Okay, so you're implicitly increasing the coroutine size to allow you to round 
up to get an aligned frame.  But how do you round back down to get the actual 
pointer that you need to delete?  This just doesn't work.

You really ought to just be using the aligned `operator new` instead when the 
required alignment is too high.  If that means checking the alignment 
"dynamically" before calling `operator new` / `operator delete`, so be it.  In 
practice, it will not be dynamic because lowering will replace the `coro.align` 
call with a constant, at which point the branch will be foldable.

I don't know what to suggest if the aligned `operator new` isn't reliably 
available on the target OS.  You could outline a function to pick the best 
allocator/deallocator, I suppose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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


[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

2021-03-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

In D97878#2602077 , @plotfi wrote:

> This makes sense to me. I approve. Can we move the 3/60 seconds number to a 
> const int value set somewhere higher up in the file as a global with a 
> comment explaining this as well?

Updated the diff; lemme know if this is what you had in mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97878

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


[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

2021-03-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 328283.
smeenai added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97878

Files:
  clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp


Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -34,6 +34,17 @@
 
 typedef DirectoryWatcher::Event::EventKind EventKind;
 
+// We've observed this test being significantly flaky when running on a heavily
+// loaded machine (e.g. when it's being run as part of the full check-clang
+// suite). Set a high timeout value to avoid this flakiness. The 60s timeout
+// value was determined empirically. It's a timeout value, not a sleep value,
+// and the test should require much less time in practice the vast majority of
+// instances. The cases where we do come close to (or still end up hitting) the
+// longer timeout are most likely to occur when other tests are also running at
+// the same time (e.g. as part of the full check-clang suite), in which case 
the
+// latency of the timeout will be masked by the latency of the other tests.
+constexpr std::chrono::seconds EventualResultTimeout(60);
+
 struct DirectoryWatcherTestFixture {
   std::string TestRootDir;
   std::string TestWatchedDir;
@@ -243,7 +254,7 @@
   std::thread worker(std::move(task));
   worker.detach();
 
-  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==
+  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(EventualResultTimeout) ==
   std::future_status::ready)
   << "The expected result state wasn't reached before the time-out.";
   std::unique_lock L(TestConsumer.Mtx);


Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -34,6 +34,17 @@
 
 typedef DirectoryWatcher::Event::EventKind EventKind;
 
+// We've observed this test being significantly flaky when running on a heavily
+// loaded machine (e.g. when it's being run as part of the full check-clang
+// suite). Set a high timeout value to avoid this flakiness. The 60s timeout
+// value was determined empirically. It's a timeout value, not a sleep value,
+// and the test should require much less time in practice the vast majority of
+// instances. The cases where we do come close to (or still end up hitting) the
+// longer timeout are most likely to occur when other tests are also running at
+// the same time (e.g. as part of the full check-clang suite), in which case the
+// latency of the timeout will be masked by the latency of the other tests.
+constexpr std::chrono::seconds EventualResultTimeout(60);
+
 struct DirectoryWatcherTestFixture {
   std::string TestRootDir;
   std::string TestWatchedDir;
@@ -243,7 +254,7 @@
   std::thread worker(std::move(task));
   worker.detach();
 
-  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==
+  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(EventualResultTimeout) ==
   std::future_status::ready)
   << "The expected result state wasn't reached before the time-out.";
   std::unique_lock L(TestConsumer.Mtx);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg.

2021-03-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

> The idea here is not to "ignore type alignment".  `EmitPointerWithAlignment` 
> will sometimes return an alignment for a pointer that's less than the 
> alignment of the pointee type, e.g. because you're taking the address of a 
> packed struct field.  The critical question is whether the atomic builtins 
> ought to make an effort to honor that reduced alignment, even if it leads to 
> terrible code, or if we should treat the use of the builtin as a user promise 
> that the pointer is actually more aligned than you might think from the 
> information statically available.

Agreed -- that is the question. In general, I'd like to default to basing 
decisions only on the statically-known alignments, because I think that'll 
typically be the best choice for users. Where there's something like a packed 
struct, it's likely that the values will end up under-aligned in fact, not just 
in the compiler's understanding.

> For example, the MSDN documentation for `InterlockedIncrement` says that it 
> requires 32-bit alignment [...].  I would say that all of the Interlocked 
> APIs ought to be read as guaranteeing the natural, full-width alignment for 
> their operation.

I had missed that it was documented in some of the other functions (beyond 
InterlockedCompareExchange128). I'll change the remainder of the _Interlocked 
APIs to assume at least natural alignment.

> I'm less certain about what to do with the `__atomic_*` builtins

The `__atomic` builtins have already been supporting under-aligned pointers all 
along -- and that behavior is unchanged by this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97224

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


[clang] 10264a1 - Introduce noundef attribute at call sites for stricter poison analysis

2021-03-04 Thread Gui Andrade via cfe-commits

Author: Gui Andrade
Date: 2021-03-04T12:15:12-08:00
New Revision: 10264a1b21aebf75a8102116c9648c3386e8021e

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

LOG: Introduce noundef attribute at call sites for stricter poison analysis

This change adds a new IR noundef attribute, which denotes when a function call 
argument or return val may never contain uninitialized bits.

In MemorySanitizer, this attribute enables optimizations which decrease 
instrumented code size by up to 17% (measured with an instrumented build of 
clang) . I'll introduce the change allowing msan to take advantage of this 
information in a separate patch.

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

Added: 
clang/test/CodeGen/attr-noundef.cpp
clang/test/CodeGen/indirect-noundef.cpp

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.h

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 50503dbe4f84..a1db1b101620 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -64,6 +64,7 @@ CODEGENOPT(DisableLifetimeMarkers, 1, 0) ///< Don't emit any 
lifetime markers
 CODEGENOPT(DisableO0ImplyOptNone , 1, 0) ///< Don't annonate function with 
optnone at O0
 CODEGENOPT(ExperimentalStrictFloatingPoint, 1, 0) ///< Enables the new, 
experimental
   ///< strict floating point.
+CODEGENOPT(EnableNoundefAttrs, 1, 0) ///< Enable emitting `noundef` attributes 
on IR call arguments and return values
 CODEGENOPT(LegacyPassManager, 1, 0) ///< Use the legacy pass manager.
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 583d08151e1a..d4d48deb649f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4994,6 +4994,9 @@ def code_completion_with_fixits : Flag<["-"], 
"code-completion-with-fixits">,
 def disable_free : Flag<["-"], "disable-free">,
   HelpText<"Disable freeing of memory on exit">,
   MarshallingInfoFlag>;
+def enable_noundef_analysis : Flag<["-"], "enable-noundef-analysis">, 
Group,
+  HelpText<"Enable analyzing function argument and return types for mandatory 
definedness">,
+  MarshallingInfoFlag>;
 def discard_value_names : Flag<["-"], "discard-value-names">,
   HelpText<"Discard value names in LLVM IR">,
   MarshallingInfoFlag>;

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 4ea707621b33..f5411daaa677 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1732,6 +1732,18 @@ static void 
AddAttributesFromFunctionProtoType(ASTContext ,
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
 }
 
+bool CodeGenModule::MayDropFunctionReturn(const ASTContext ,
+  QualType ReturnType) {
+  // We can't just discard the return value for a record type with a
+  // complex destructor or a non-trivially copyable type.
+  if (const RecordType *RT =
+  ReturnType.getCanonicalType()->getAs()) {
+if (const auto *ClassDecl = dyn_cast(RT->getDecl()))
+  return ClassDecl->hasTrivialDestructor();
+  }
+  return ReturnType.isTriviallyCopyableType(Context);
+}
+
 void CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
  bool HasOptnone,
  bool AttrOnCallSite,
@@ -1905,6 +1917,55 @@ static void addNoBuiltinAttributes(llvm::AttrBuilder 
,
   llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
 }
 
+static bool DetermineNoUndef(QualType QTy, CodeGenTypes ,
+ const llvm::DataLayout , const ABIArgInfo ,
+ bool CheckCoerce = true) {
+  llvm::Type *Ty = Types.ConvertTypeForMem(QTy);
+  if (AI.getKind() == ABIArgInfo::Indirect)
+return true;
+  if (AI.getKind() == ABIArgInfo::Extend)
+return true;
+  if (!DL.typeSizeEqualsStoreSize(Ty))
+// TODO: This will result in a modest amount of values not marked noundef
+// when they could be. We care about values that *invisibly* contain undef
+// bits from the perspective of LLVM IR.
+return false;
+  if (CheckCoerce && AI.canHaveCoerceToType()) {
+llvm::Type *CoerceTy = AI.getCoerceToType();
+if (llvm::TypeSize::isKnownGT(DL.getTypeSizeInBits(CoerceTy),
+  DL.getTypeSizeInBits(Ty)))
+

[PATCH] D96281: [clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check.

2021-03-04 Thread Kirill Bobyrev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbff7faea2034: [clang-tidy] Add options to describe 
individual core increments to readability… (authored by massberg, committed by 
kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96281

Files:
  clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- \
+// RUN:   -config='{CheckOptions: \
+// RUN: [{key: readability-function-cognitive-complexity.Threshold, \
+// RUN:   value: 0}, \
+// RUN:  {key: readability-function-cognitive-complexity.DescribeBasicIncrements, \
+// RUN:   value: "false"} ]}'
+// RUN: %check_clang_tidy -check-suffix=THRESHOLD5 %s readability-function-cognitive-complexity %t -- \
+// RUN:   -config='{CheckOptions: \
+// RUN: [{key: readability-function-cognitive-complexity.Threshold, \
+// RUN:   value: 5}, \
+// RUN:  {key: readability-function-cognitive-complexity.DescribeBasicIncrements, \
+// RUN:   value: "false"} ]}'
+
+void func_of_complexity_4() {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'func_of_complexity_4' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
+  if (1) {
+if (1) {
+}
+  }
+  if (1) {
+  }
+}
+
+#define MacroOfComplexity10 \
+  if (1) {  \
+if (1) {\
+  if (1) {  \
+if (1) {\
+}   \
+  } \
+}   \
+  }
+
+void function_with_macro() {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'function_with_macro' has cognitive complexity of 11 (threshold 0) [readability-function-cognitive-complexity]
+  // CHECK-NOTES-THRESHOLD5: :[[@LINE-2]]:6: warning: function 'function_with_macro' has cognitive complexity of 11 (threshold 5) [readability-function-cognitive-complexity]
+
+  MacroOfComplexity10;
+
+  if (1) {
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
@@ -17,6 +17,13 @@
Flag functions with Cognitive Complexity exceeding this number.
The default is `25`.
 
+.. option:: DescribeBasicIncrements
+
+   If set to `true`, then for each function exceeding the complexity threshold
+   the check will issue additional diagnostics on every piece of code (loop,
+   `if` statement, etc.) which contributes to that complexity. See also the
+   examples below. Default is `true`.
+
 Building blocks
 ---
 
@@ -135,6 +142,11 @@
 return 0;
   }
 
+In the last example, the check will flag `function3` if the option Threshold is
+set to `2` or smaller. If the option DescribeBasicIncrements is set to `true`,
+it will additionally flag the two `if` statements with the amounts by which they
+increase to the complexity of the function and the current nesting level.
+
 Limitations
 ---
 
Index: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
===
--- clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
+++ clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
@@ -17,10 +17,15 @@
 
 /// Checks function Cognitive Complexity metric.
 ///
-/// There is only one configuration option:
+/// There are the following configuration option:
 ///
 ///   * `Threshold` - flag functions with Cognitive Complexity exceeding
 /// this number. The default is `25`.
+///   * `DescribeBasicIncrements`- if set to `true`, then for each function
+/// exceeding the complexity threshold the check will issue additional
+/// diagnostics on every piece of code (loop, `if` statement, etc.) which
+/// contributes to that complexity.
+//  Default is `true`
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability-function-cognitive-complexity.html
@@ -37,6 

[clang-tools-extra] bff7fae - [clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check.

2021-03-04 Thread Kirill Bobyrev via cfe-commits

Author: Jens Massberg
Date: 2021-03-04T21:02:27+01:00
New Revision: bff7faea2034abed4535645d7c771e67c1f2bb23

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

LOG: [clang-tidy] Add options to describe individual core increments to 
readability-function-cognitive-complexity check.

Often you are only interested in the overall cognitive complexity of a
function and not every individual increment. Thus the flag
'DescribeBasicIncrements' is added. If it is set to 'true', each increment
is flagged. Otherwise, only the complexity of function with complexity
of at least the threshold are flagged.

By default 'DescribeBasisIncrements' is set to 'true', which is the original 
behavior of the check.

Added a new test for different flag combinations.

(The option to ignore macros which was original part of this patch will be 
added in another path)

Reviewed By: lebedev.ri

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp

Modified: 

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h

clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
index 3a4758302406..94ff38dcec05 100644
--- 
a/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
@@ -492,11 +492,13 @@ class FunctionASTVisitor final
 FunctionCognitiveComplexityCheck::FunctionCognitiveComplexityCheck(
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  Threshold(Options.get("Threshold", CognitiveComplexity::DefaultLimit)) {}
+  Threshold(Options.get("Threshold", CognitiveComplexity::DefaultLimit)),
+  DescribeBasicIncrements(Options.get("DescribeBasicIncrements", true)) {}
 
 void FunctionCognitiveComplexityCheck::storeOptions(
 ClangTidyOptions::OptionMap ) {
   Options.store(Opts, "Threshold", Threshold);
+  Options.store(Opts, "DescribeBasicIncrements", DescribeBasicIncrements);
 }
 
 void FunctionCognitiveComplexityCheck::registerMatchers(MatchFinder *Finder) {
@@ -537,6 +539,9 @@ void FunctionCognitiveComplexityCheck::check(
 diag(Loc, "lambda has cognitive complexity of %0 (threshold %1)")
 << Visitor.CC.Total << Threshold;
 
+  if (!DescribeBasicIncrements)
+return;
+
   // Output all the basic increments of complexity.
   for (const auto  : Visitor.CC.Details) {
 unsigned MsgId;  // The id of the message to output.

diff  --git 
a/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h 
b/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
index a21b8447029b..01244ab0ecf0 100644
--- 
a/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
+++ 
b/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
@@ -17,10 +17,15 @@ namespace readability {
 
 /// Checks function Cognitive Complexity metric.
 ///
-/// There is only one configuration option:
+/// There are the following configuration option:
 ///
 ///   * `Threshold` - flag functions with Cognitive Complexity exceeding
 /// this number. The default is `25`.
+///   * `DescribeBasicIncrements`- if set to `true`, then for each function
+/// exceeding the complexity threshold the check will issue additional
+/// diagnostics on every piece of code (loop, `if` statement, etc.) which
+/// contributes to that complexity.
+//  Default is `true`
 ///
 /// For the user-facing documentation see:
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/readability-function-cognitive-complexity.html
@@ -37,6 +42,7 @@ class FunctionCognitiveComplexityCheck : public 
ClangTidyCheck {
 
 private:
   const unsigned Threshold;
+  const bool DescribeBasicIncrements;
 };
 
 } // namespace readability

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
 
b/clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
index b863357a2132..b14c684f57f5 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
@@ -17,6 +17,13 @@ Options
Flag functions with Cognitive Complexity exceeding this number.
The default is `25`.
 
+.. option:: DescribeBasicIncrements
+

[PATCH] D97101: [Coverage] Emit gap region between statements if first statements contains terminate statements.

2021-03-04 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added a comment.

In D97101#2603321 , @thakis wrote:

> This broke ContinuousSyncMode/basic.c in check-profile on macOS (see e.g. end 
> of 
> https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8853690069583264896/+/steps/package_clang/0/stdout?format=raw),
>  so I reverted it for now.
>
> It repros on my laptop, so let me know if you want me to try things :)

Relanded with update on the broken test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97101

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


[clang] 9783e20 - Revert "Revert "[Coverage] Emit gap region between statements if first statements contains terminate statements.""

2021-03-04 Thread Zequan Wu via cfe-commits

Author: Zequan Wu
Date: 2021-03-04T11:52:43-08:00
New Revision: 9783e2098800b954c55ae598a1ce5c4b93444fc0

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

LOG: Revert "Revert "[Coverage] Emit gap region between statements if first 
statements contains terminate statements.""

Reland with update on test case ContinuousSyncmode/basic.c.

This reverts commit fe5c2c3ca682b140dd5e640e75948363b6b25ef9.

Added: 
clang/test/CoverageMapping/terminate-statements.cpp

Modified: 
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/break.c
clang/test/CoverageMapping/classtemplate.cpp
clang/test/CoverageMapping/continue.c
clang/test/CoverageMapping/coroutine.cpp
clang/test/CoverageMapping/label.cpp
clang/test/CoverageMapping/return.c
clang/test/CoverageMapping/switch.cpp
clang/test/CoverageMapping/switchmacro.c
clang/test/CoverageMapping/trycatch.cpp
clang/test/CoverageMapping/unreachable-macro.c
compiler-rt/test/profile/ContinuousSyncMode/basic.c
compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c

Removed: 
clang/test/CoverageMapping/deferred-region.cpp



diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 4a008b4ef632..8a11da600e4a 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -104,26 +104,21 @@ class SourceMappingRegion {
   /// The region's ending location.
   Optional LocEnd;
 
-  /// Whether this region should be emitted after its parent is emitted.
-  bool DeferRegion;
-
   /// Whether this region is a gap region. The count from a gap region is set
   /// as the line execution count if there are no other regions on the line.
   bool GapRegion;
 
 public:
   SourceMappingRegion(Counter Count, Optional LocStart,
-  Optional LocEnd, bool DeferRegion = 
false,
-  bool GapRegion = false)
-  : Count(Count), LocStart(LocStart), LocEnd(LocEnd),
-DeferRegion(DeferRegion), GapRegion(GapRegion) {}
+  Optional LocEnd, bool GapRegion = false)
+  : Count(Count), LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion) 
{
+  }
 
   SourceMappingRegion(Counter Count, Optional FalseCount,
   Optional LocStart,
-  Optional LocEnd, bool DeferRegion = 
false,
-  bool GapRegion = false)
+  Optional LocEnd, bool GapRegion = false)
   : Count(Count), FalseCount(FalseCount), LocStart(LocStart),
-LocEnd(LocEnd), DeferRegion(DeferRegion), GapRegion(GapRegion) {}
+LocEnd(LocEnd), GapRegion(GapRegion) {}
 
   const Counter () const { return Count; }
 
@@ -155,10 +150,6 @@ class SourceMappingRegion {
 return *LocEnd;
   }
 
-  bool isDeferred() const { return DeferRegion; }
-
-  void setDeferred(bool Deferred) { DeferRegion = Deferred; }
-
   bool isGap() const { return GapRegion; }
 
   void setGap(bool Gap) { GapRegion = Gap; }
@@ -544,10 +535,6 @@ struct CounterCoverageMappingBuilder
   /// A stack of currently live regions.
   std::vector RegionStack;
 
-  /// The currently deferred region: its end location and count can be set once
-  /// its parent has been popped from the region stack.
-  Optional DeferredRegion;
-
   CounterExpressionBuilder Builder;
 
   /// A location in the most recently visited file or macro.
@@ -556,8 +543,11 @@ struct CounterCoverageMappingBuilder
   /// expressions cross file or macro boundaries.
   SourceLocation MostRecentLocation;
 
-  /// Location of the last terminated region.
-  Optional> LastTerminatedRegion;
+  /// Whether the visitor at a terminate statement.
+  bool HasTerminateStmt = false;
+
+  /// Gap region counter after terminate statement.
+  Counter GapRegionCounter;
 
   /// Return a counter for the subtraction of \c RHS from \c LHS
   Counter subtractCounters(Counter LHS, Counter RHS) {
@@ -590,77 +580,13 @@ struct CounterCoverageMappingBuilder
 
 if (StartLoc && !FalseCount.hasValue()) {
   MostRecentLocation = *StartLoc;
-  completeDeferred(Count, MostRecentLocation);
 }
 
-RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc,
- FalseCount.hasValue());
+RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
   }
 
-  /// Complete any pending deferred region by setting its end location and
-  /// count, and then pushing it onto the region stack.
-  size_t completeDeferred(Counter Count, SourceLocation DeferredEndLoc) {
-size_t Index = RegionStack.size();
-if (!DeferredRegion)
-  return Index;
-
-// Consume the pending region.
-SourceMappingRegion DR = 

[PATCH] D97743: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

@jansvoboda11 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97743

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


[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

2021-03-04 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Thanks! Reapplied the patch in 
https://github.com/llvm/llvm-project/commit/1900503595cbb84a4c6e140a9ba1a2c574c0586d


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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


[PATCH] D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg.

2021-03-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D97224#2604069 , @jyknight wrote:

> In D97224#2596410 , @rjmccall wrote:
>
>> Do we really consider the existing atomic intrinsics to not impose added 
>> alignment restrictions?  I'm somewhat concerned that we're going to produce 
>> IR here that's either really suboptimal or, worse, unimplementable, just 
>> because we over-interpreted some cue about alignment.  I guess it would only 
>> be a significant problem on a target where types are frequently 
>> under-aligned for what atomics need, which is not typical, or when the user 
>> is doing atomics on a field of something like a packed struct.
>
> For the `__atomic_*` intrinsics, we don't consider those as imposing 
> additional alignment restrictions -- currently, we avoid generating the LLVM 
> IR instruction if it's below natural alignment, since we couldn't specify 
> alignment on the IR instruction. (Now that we have alignment on the LLVM IR 
> operations, I'd like to eventually get rid of that logic from Clang, since 
> it's also handled by LLVM.)

Frontends ultimately have the responsibility of making sure they ultimately 
emit code that follows the platform ABI for atomics.  In most other parts of 
the ABI, we usually find that it is possible (even necessary) to delegate 
*part* of that ABI responsibility down to LLVM — e.g. to emit inline atomic 
sequences, which I suppose frontends could do with inline assembly, but there 
are obvious reasons to prefer a more semantic IR — but that at least in some 
cases, there is information that we cannot pass down and so must handle in the 
frontend.  I am somewhat skeptical that atomics are going to prove an exception 
here.  At the very least, there will always be *some* operations that we have 
to expand into compare-exchange loops in the frontend, for want of a 
sufficiently powerful instruction/intrinsic.  That said, if you find that you 
can actually free Clang from having to make certain decisions here, that's 
great; I just want you to understand that usually we find that there are limits 
to what we can usefully delegate to LLVM, and ultimately the responsibility is 
ours.

> For other intrinsics (e.g. MSVCIntrin::_InterlockedAnd, 
> Builtin::BI__sync_fetch_and_add_4, NVPTX::BI__nvvm_atom_add_gen_i, and the 
> others in those 3 function families), we currently entirely ignore the 
> alignment, and simply assume the argument is naturally-aligned. So, yes, this 
> change could potentially affect the behavior for underaligned types.
>
> So, I could change these to explicitly increase the assumed alignment of 
> their arguments, like I did for InterlockedCompareExchange128. My inclination 
> is not to do so, however...it doesn't seem like a good idea in general to 
> ignore type alignment. But, I'd not be opposed to doing that, if there's a 
> good reason.

The idea here is not to "ignore type alignment".  `EmitPointerWithAlignment` 
will sometimes return an alignment for a pointer that's less than the alignment 
of the pointee type, e.g. because you're taking the address of a packed struct 
field.  The critical question is whether the atomic builtins ought to make an 
effort to honor that reduced alignment, even if it leads to terrible code, or 
if we should treat the use of the builtin as a user promise that the pointer is 
actually more aligned than you might think from the information statically 
available.  That has to be resolved by the semantics of the builtin, and 
unfortunately intrinsic documentation is often spotty on this point.  For 
example, the MSDN documentation for `InterlockedIncrement` says that it 
requires 32-bit alignment, but the documentation for `InterlockedAdd` doesn't.  
It seems extremely unlikely that that is meant to be read as a statement that 
`InterlockedAdd` is actually more permissive.  I would say that all of the 
Interlocked APIs ought to be read as guaranteeing the natural, full-width 
alignment for their operation.  I'm less certain about what to do with the 
`__atomic_*` builtins, because maybe we should take this as an opportunity to 
try to "do the right thing" with under-aligned atomics; on the other hand, that 
assumes that we always *can* "do the right thing", and I don't want Clang to 
start miscompiling or refusing to compile code because we're trying to do 
something above and beyond the normal language semantics.  It might be more 
reasonable to always use the type alignment as a minimum.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97224

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


[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Yes, we took steps to suppress CRLF generation. Part of that was to avoid 
Windows-only test failures from trailing CR whitespace interacting with tools 
like `diff`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

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


[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

2021-03-04 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

In D92808#2602687 , @hans wrote:

> In D92808#2601408 , @fhahn wrote:
>
>> In D92808#2600245 , @hans wrote:
>>
>>> Reverted in 
>>> https://github.com/llvm/llvm-project/commit/0a5dd067181dac2a8882a139ea3bd19bdea5fa44
>>>  until this can be fixed.
>>
>> Thanks! I pushed a fix for the issue in 75805dce5ff8 
>> . 
>> @Hans, would it be possible to check if D92808 
>>  + 75805dce5ff8 
>>  fixes 
>> the issue or should we just recommit D92808 
>> ?
>
> Yes, I confirmed that 75805dce5ff8 
>  fixes 
> the test case I was looking at. Thanks!

Thanks for confirming!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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


[clang] 1900503 - [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of

2021-03-04 Thread Akira Hatanaka via cfe-commits

Author: Akira Hatanaka
Date: 2021-03-04T11:22:30-08:00
New Revision: 1900503595cbb84a4c6e140a9ba1a2c574c0586d

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

LOG: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of
explicitly emitting retainRV or claimRV calls in the IR

This reapplies ed4718eccb12bd42214ca4fb17d196d49561c0c7, which was reverted
because it was causing a miscompile. The bug that was causing the miscompile
has been fixed in 75805dce5ff874676f3559c069fcd6737838f5c0.

Original commit message:

Background:

This fixes a longstanding problem where llvm breaks ARC's autorelease
optimization (see the link below) by separating calls from the marker
instructions or retainRV/claimRV calls. The backend changes are in
https://reviews.llvm.org/D92569.

https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-autoreleasereturnvalue

What this patch does to fix the problem:

- The front-end adds operand bundle "clang.arc.attachedcall" to calls,
  which indicates the call is implicitly followed by a marker
  instruction and an implicit retainRV/claimRV call that consumes the
  call result. In addition, it emits a call to
  @llvm.objc.clang.arc.noop.use, which consumes the call result, to
  prevent the middle-end passes from changing the return type of the
  called function. This is currently done only when the target is arm64
  and the optimization level is higher than -O0.

- ARC optimizer temporarily emits retainRV/claimRV calls after the calls
  with the operand bundle in the IR and removes the inserted calls after
  processing the function.

- ARC contract pass emits retainRV/claimRV calls after the call with the
  operand bundle. It doesn't remove the operand bundle on the call since
  the backend needs it to emit the marker instruction. The retainRV and
  claimRV calls are emitted late in the pipeline to prevent optimization
  passes from transforming the IR in a way that makes it harder for the
  ARC middle-end passes to figure out the def-use relationship between
  the call and the retainRV/claimRV calls (which is the cause of
  PR31925).

- The function inliner removes an autoreleaseRV call in the callee if
  nothing in the callee prevents it from being paired up with the
  retainRV/claimRV call in the caller. It then inserts a release call if
  claimRV is attached to the call since autoreleaseRV+claimRV is
  equivalent to a release. If it cannot find an autoreleaseRV call, it
  tries to transfer the operand bundle to a function call in the callee.
  This is important since the ARC optimizer can remove the autoreleaseRV
  returning the callee result, which makes it impossible to pair it up
  with the retainRV/claimRV call in the caller. If that fails, it simply
  emits a retain call in the IR if retainRV is attached to the call and
  does nothing if claimRV is attached to it.

- SCCP refrains from replacing the return value of a call with a
  constant value if the call has the operand bundle. This ensures the
  call always has at least one user (the call to
  @llvm.objc.clang.arc.noop.use).

- This patch also fixes a bug in replaceUsesOfNonProtoConstant where
  multiple operand bundles of the same kind were being added to a call.

Future work:

- Use the operand bundle on x86-64.

- Fix the auto upgrader to convert call+retainRV/claimRV pairs into
  calls with the operand bundles.

rdar://71443534

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

Added: 
clang/test/CodeGenObjC/arc-rv-attr.m
llvm/include/llvm/Analysis/ObjCARCUtil.h
llvm/test/Transforms/Inline/inline-retainRV-call.ll
llvm/test/Transforms/ObjCARC/contract-rv-attr.ll
llvm/test/Transforms/SCCP/clang-arc-rv.ll

Modified: 
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenObjC/arc-unsafeclaim.m
llvm/docs/LangRef.rst
llvm/include/llvm/IR/InstrTypes.h
llvm/include/llvm/IR/Intrinsics.td
llvm/include/llvm/IR/LLVMContext.h
llvm/lib/Analysis/ObjCARCInstKind.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/AutoUpgrade.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/LLVMContext.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h
llvm/lib/Transforms/ObjCARC/ObjCARC.cpp
llvm/lib/Transforms/ObjCARC/ObjCARC.h
llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
llvm/lib/Transforms/ObjCARC/PtrState.cpp
llvm/lib/Transforms/ObjCARC/PtrState.h
llvm/lib/Transforms/Scalar/SCCP.cpp
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp

[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

2021-03-04 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment.

LGTM.

Adding the comment would be great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97878

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


[PATCH] D97941: [Reland] "Do not apply calling conventions to MSVC entry points"

2021-03-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Seems reasonable




Comment at: clang/lib/Sema/SemaDecl.cpp:11206-11207
+} else if (FT->getCallConv() != CC_X86StdCall) {
+  // Default calling convention for WinMain, wWinMain and DllMain is
+  // __stdcall
+  FT = Context.adjustFunctionType(

This should only be done on 32-bit x86 platforms. I think i686-window-msvc is 
the more special case platform here, so I would recommend making the helper 
something like `isDefaultStdCall`, which is true for i686-windows-msvc, false 
for mingw and non-x86 triples, and false for main/wmain.


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

https://reviews.llvm.org/D97941

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


[PATCH] D97894: [Driver] Drop $sysroot/usr special case from Gentoo gcc-config detection

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D97894#2602638 , @mgorny wrote:

> Well, I think this was mostly 'just in case' condition, so I guess it's fine. 
> Let's just wait for @manojgupta to give it some more testing.

Thanks. I sent D97902  to improve the 
documentation. Dropping the special case makes it more consistent: if 
--gcc-toolchain is not empty, suppress sysroot GCC detection

With the special rule, it is: if --gcc-toolchain is $sysroot/usr, suppress 
sysroot GCC detection as well.

Clang -B and --gcc-toolchain have some weird behaviors. Hope you can share your 
opinions on https://lists.llvm.org/pipermail/cfe-dev/2021-March/067827.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97894

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


[PATCH] D97940: [clang-tidy] Extend LoopConvert on array with `!=` comparison

2021-03-04 Thread Nathan James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa85eb11129ce: [clang-tidy] Extend LoopConvert on array with 
`!=` comparison (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97940

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
@@ -95,6 +95,33 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & Tea : Teas)
   // CHECK-FIXES-NEXT: Tea.g();
+
+  for (int I = 0; N > I; ++I) {
+printf("Fibonacci number %d has address %p\n", Arr[I], [I]);
+Sum += Arr[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int & I : Arr)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, );
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0; N != I; ++I) {
+printf("Fibonacci number %d has address %p\n", Arr[I], [I]);
+Sum += Arr[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int & I : Arr)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, );
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0; I != N; ++I) {
+printf("Fibonacci number %d has address %p\n", Arr[I], [I]);
+Sum += Arr[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int & I : Arr)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, );
+  // CHECK-FIXES-NEXT: Sum += I + 2;
 }
 
 const int *constArray() {
@@ -589,6 +616,33 @@
   // CHECK-FIXES: for (int I : *Cv)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
   // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = V.size(); E > I; ++I) {
+printf("Fibonacci number is %d\n", V[I]);
+Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = V.size(); I != E; ++I) {
+printf("Fibonacci number is %d\n", V[I]);
+Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = V.size(); E != I; ++I) {
+printf("Fibonacci number is %d\n", V[I]);
+Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
 }
 
 // Ensure that 'const auto &' is used with containers of non-trivial types.
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -83,6 +83,17 @@
   return declRefExpr(to(varDecl(equalsBoundNode(InitVarName;
 }
 
+static StatementMatcher
+arrayConditionMatcher(internal::Matcher LimitExpr) {
+  return binaryOperator(
+  anyOf(allOf(hasOperatorName("<"), hasLHS(integerComparisonMatcher()),
+  hasRHS(LimitExpr)),
+allOf(hasOperatorName(">"), hasLHS(LimitExpr),
+  hasRHS(integerComparisonMatcher())),
+allOf(hasOperatorName("!="),
+  hasOperands(integerComparisonMatcher(), LimitExpr;
+}
+
 /// The matcher for loops over arrays.
 /// \code
 ///   for (int i = 0; i < 3 + 2; ++i) { ... }
@@ -99,18 +110,12 @@
   StatementMatcher ArrayBoundMatcher =
   expr(hasType(isInteger())).bind(ConditionBoundName);
 
-  return forStmt(
- unless(isInTemplateInstantiation()),
- hasLoopInit(declStmt(hasSingleDecl(initToZeroMatcher(,
- hasCondition(anyOf(
- binaryOperator(hasOperatorName("<"),
-hasLHS(integerComparisonMatcher()),
-hasRHS(ArrayBoundMatcher)),
- binaryOperator(hasOperatorName(">"), hasLHS(ArrayBoundMatcher),
-hasRHS(integerComparisonMatcher(),
- hasIncrement(
- 

[clang-tools-extra] a85eb11 - [clang-tidy] Extend LoopConvert on array with `!=` comparison

2021-03-04 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2021-03-04T18:58:59Z
New Revision: a85eb11129ce941e91b5bc3b188e469aaca4425f

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

LOG: [clang-tidy] Extend LoopConvert on array with `!=` comparison

Enables transforming loops of the form:
```
for (int i = 0; I != container.size(); ++I) { container[I]...; }
for (int i = 0; I != N; ++I) { FixedArrSizeN[I]...; }
```

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index 9df343f81d1c..9f6ef08ef839 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -83,6 +83,17 @@ static const StatementMatcher incrementVarMatcher() {
   return declRefExpr(to(varDecl(equalsBoundNode(InitVarName;
 }
 
+static StatementMatcher
+arrayConditionMatcher(internal::Matcher LimitExpr) {
+  return binaryOperator(
+  anyOf(allOf(hasOperatorName("<"), hasLHS(integerComparisonMatcher()),
+  hasRHS(LimitExpr)),
+allOf(hasOperatorName(">"), hasLHS(LimitExpr),
+  hasRHS(integerComparisonMatcher())),
+allOf(hasOperatorName("!="),
+  hasOperands(integerComparisonMatcher(), LimitExpr;
+}
+
 /// The matcher for loops over arrays.
 /// \code
 ///   for (int i = 0; i < 3 + 2; ++i) { ... }
@@ -99,18 +110,12 @@ StatementMatcher makeArrayLoopMatcher() {
   StatementMatcher ArrayBoundMatcher =
   expr(hasType(isInteger())).bind(ConditionBoundName);
 
-  return forStmt(
- unless(isInTemplateInstantiation()),
- hasLoopInit(declStmt(hasSingleDecl(initToZeroMatcher(,
- hasCondition(anyOf(
- binaryOperator(hasOperatorName("<"),
-hasLHS(integerComparisonMatcher()),
-hasRHS(ArrayBoundMatcher)),
- binaryOperator(hasOperatorName(">"), 
hasLHS(ArrayBoundMatcher),
-hasRHS(integerComparisonMatcher(),
- hasIncrement(
- unaryOperator(hasOperatorName("++"),
-   hasUnaryOperand(incrementVarMatcher()
+  return forStmt(unless(isInTemplateInstantiation()),
+ hasLoopInit(declStmt(hasSingleDecl(initToZeroMatcher(,
+ hasCondition(arrayConditionMatcher(ArrayBoundMatcher)),
+ hasIncrement(
+ unaryOperator(hasOperatorName("++"),
+   hasUnaryOperand(incrementVarMatcher()
   .bind(LoopNameArray);
 }
 
@@ -278,22 +283,16 @@ StatementMatcher makePseudoArrayLoopMatcher() {
  declRefExpr(to(varDecl(equalsBoundNode(EndVarName),
  EndInitMatcher));
 
-  return forStmt(
- unless(isInTemplateInstantiation()),
- hasLoopInit(
- anyOf(declStmt(declCountIs(2),
-containsDeclaration(0, initToZeroMatcher()),
-containsDeclaration(1, EndDeclMatcher)),
-   declStmt(hasSingleDecl(initToZeroMatcher(),
- hasCondition(anyOf(
- binaryOperator(hasOperatorName("<"),
-hasLHS(integerComparisonMatcher()),
-hasRHS(IndexBoundMatcher)),
- binaryOperator(hasOperatorName(">"), 
hasLHS(IndexBoundMatcher),
-hasRHS(integerComparisonMatcher(),
- hasIncrement(
- unaryOperator(hasOperatorName("++"),
-   hasUnaryOperand(incrementVarMatcher()
+  return forStmt(unless(isInTemplateInstantiation()),
+ hasLoopInit(
+ anyOf(declStmt(declCountIs(2),
+containsDeclaration(0, 
initToZeroMatcher()),
+containsDeclaration(1, EndDeclMatcher)),
+   declStmt(hasSingleDecl(initToZeroMatcher(),
+ hasCondition(arrayConditionMatcher(IndexBoundMatcher)),
+ hasIncrement(
+ unaryOperator(hasOperatorName("++"),
+   hasUnaryOperand(incrementVarMatcher()
   .bind(LoopNamePseudoArray);
 }
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp 

[PATCH] D97927: [clang-format] Rename case sorting

2021-03-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D97927#2603089 , @curdeius wrote:

> LGTM. It's indeed more logical this way.
> Hopefully the original author will confirm.

I'll give him one week, if he or nobody else comes along to object I'll push it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97927

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


[PATCH] D97296: [analyzer] Add a new parameter ProgramStateRef to SValBuilder::evalCast function

2021-03-04 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 328238.
ASDenysPetrov added a comment.

Missed to add changes in unittests. Fixed.


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

https://reviews.llvm.org/D97296

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/unittests/StaticAnalyzer/StoreTest.cpp

Index: clang/unittests/StaticAnalyzer/StoreTest.cpp
===
--- clang/unittests/StaticAnalyzer/StoreTest.cpp
+++ clang/unittests/StaticAnalyzer/StoreTest.cpp
@@ -69,13 +69,13 @@
 SVal NarrowZero = Builder.makeZeroVal(ASTCtxt.CharTy);
 
 // Bind(Zero)
-Store StX0 = SManager.Bind(StInit, LX0, Zero).getStore();
-EXPECT_EQ(Zero, SManager.getBinding(StX0, LX0, ASTCtxt.IntTy));
+Store StX0 = SManager.Bind(nullptr, StInit, LX0, Zero).getStore();
+EXPECT_EQ(Zero, SManager.getBinding(nullptr, StX0, LX0, ASTCtxt.IntTy));
 
 // BindDefaultInitial(Zero)
 Store StY0 =
 SManager.BindDefaultInitial(StInit, LY0.getAsRegion(), Zero).getStore();
-EXPECT_EQ(Zero, SManager.getBinding(StY0, LY0, ASTCtxt.IntTy));
+EXPECT_EQ(Zero, SManager.getBinding(nullptr, StY0, LY0, ASTCtxt.IntTy));
 EXPECT_EQ(Zero, *SManager.getDefaultBinding(StY0, LY0.getAsRegion()));
 
 // BindDefaultZero()
@@ -83,17 +83,17 @@
 // BindDefaultZero wipes the region with '0 S8b', not with out Zero.
 // Direct load, however, does give us back the object of the type
 // that we specify for loading.
-EXPECT_EQ(Zero, SManager.getBinding(StZ0, LZ0, ASTCtxt.IntTy));
+EXPECT_EQ(Zero, SManager.getBinding(nullptr, StZ0, LZ0, ASTCtxt.IntTy));
 EXPECT_EQ(NarrowZero, *SManager.getDefaultBinding(StZ0, LZ0.getAsRegion()));
 
 // Bind(One)
-Store StX1 = SManager.Bind(StInit, LX1, One).getStore();
-EXPECT_EQ(One, SManager.getBinding(StX1, LX1, ASTCtxt.IntTy));
+Store StX1 = SManager.Bind(nullptr, StInit, LX1, One).getStore();
+EXPECT_EQ(One, SManager.getBinding(nullptr, StX1, LX1, ASTCtxt.IntTy));
 
 // BindDefaultInitial(One)
 Store StY1 =
 SManager.BindDefaultInitial(StInit, LY1.getAsRegion(), One).getStore();
-EXPECT_EQ(One, SManager.getBinding(StY1, LY1, ASTCtxt.IntTy));
+EXPECT_EQ(One, SManager.getBinding(nullptr, StY1, LY1, ASTCtxt.IntTy));
 EXPECT_EQ(One, *SManager.getDefaultBinding(StY1, LY1.getAsRegion()));
   }
 
@@ -134,10 +134,10 @@
 Store StInit = SManager.getInitialStore(SFC).getStore();
 // Let's bind constant 1 to 'test[0]'
 SVal One = Builder.makeIntVal(1, Int);
-Store StX = SManager.Bind(StInit, ZeroElement, One).getStore();
+Store StX = SManager.Bind(nullptr, StInit, ZeroElement, One).getStore();
 
 // And make sure that we can read this binding back as it was
-EXPECT_EQ(One, SManager.getBinding(StX, ZeroElement, Int));
+EXPECT_EQ(One, SManager.getBinding(nullptr, StX, ZeroElement, Int));
   }
 
 public:
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -43,7 +43,7 @@
 : svalBuilder(stateMgr.getSValBuilder()), StateMgr(stateMgr),
   MRMgr(svalBuilder.getRegionManager()), Ctx(stateMgr.getContext()) {}
 
-StoreRef StoreManager::enterStackFrame(Store OldStore,
+StoreRef StoreManager::enterStackFrame(ProgramStateRef State, Store OldStore,
const CallEvent ,
const StackFrameContext *LCtx) {
   StoreRef Store = StoreRef(OldStore, *this);
@@ -52,7 +52,7 @@
   Call.getInitialStackFrameContents(LCtx, InitialBindings);
 
   for (const auto  : InitialBindings)
-Store = 

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment.

In D97915#2603912 , @lxfind wrote:

> Could you describe in more detail what problem this patch solves?

Yes, updated description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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


[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 328232.
ychen edited the summary of this revision.
ychen added a comment.

- Add docs for `coro.align`
- clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-builtins.c
  clang/test/CodeGenCoroutines/coro-gro.cpp
  llvm/docs/Coroutines.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Coroutines/CoroInstr.h
  llvm/lib/Transforms/Coroutines/CoroInternal.h
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/Coroutines/Coroutines.cpp

Index: llvm/lib/Transforms/Coroutines/Coroutines.cpp
===
--- llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -234,6 +234,7 @@
   Shape.CoroBegin = nullptr;
   Shape.CoroEnds.clear();
   Shape.CoroSizes.clear();
+  Shape.CoroAligns.clear();
   Shape.CoroSuspends.clear();
 
   Shape.FrameTy = nullptr;
@@ -268,6 +269,9 @@
   case Intrinsic::coro_size:
 CoroSizes.push_back(cast(II));
 break;
+  case Intrinsic::coro_align:
+CoroAligns.push_back(cast(II));
+break;
   case Intrinsic::coro_frame:
 CoroFrames.push_back(cast(II));
 break;
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -997,23 +997,33 @@
   Shape.AsyncLowering.AsyncFuncPointer->setInitializer(NewFuncPtrStruct);
 }
 
-static void replaceFrameSize(coro::Shape ) {
+static void replaceFrameSizeAndAlign(coro::Shape ) {
   if (Shape.ABI == coro::ABI::Async)
 updateAsyncFuncPointerContextSize(Shape);
 
-  if (Shape.CoroSizes.empty())
-return;
+  if (!Shape.CoroSizes.empty()) {
+// In the same function all coro.sizes should have the same result type.
+auto *SizeIntrin = Shape.CoroSizes.back();
+Module *M = SizeIntrin->getModule();
+const DataLayout  = M->getDataLayout();
+auto Size = DL.getTypeAllocSize(Shape.FrameTy);
+auto *SizeConstant = ConstantInt::get(SizeIntrin->getType(), Size);
+
+for (CoroSizeInst *CS : Shape.CoroSizes) {
+  CS->replaceAllUsesWith(SizeConstant);
+  CS->eraseFromParent();
+}
+  }
 
-  // In the same function all coro.sizes should have the same result type.
-  auto *SizeIntrin = Shape.CoroSizes.back();
-  Module *M = SizeIntrin->getModule();
-  const DataLayout  = M->getDataLayout();
-  auto Size = DL.getTypeAllocSize(Shape.FrameTy);
-  auto *SizeConstant = ConstantInt::get(SizeIntrin->getType(), Size);
+  if (!Shape.CoroAligns.empty()) {
+auto *Intrin = Shape.CoroAligns.back();
+auto *AlignConstant =
+ConstantInt::get(Intrin->getType(), Shape.FrameAlign.value());
 
-  for (CoroSizeInst *CS : Shape.CoroSizes) {
-CS->replaceAllUsesWith(SizeConstant);
-CS->eraseFromParent();
+for (CoroAlignInst *CS : Shape.CoroAligns) {
+  CS->replaceAllUsesWith(AlignConstant);
+  CS->eraseFromParent();
+}
   }
 }
 
@@ -1748,7 +1758,7 @@
 
   simplifySuspendPoints(Shape);
   buildCoroutineFrame(F, Shape);
-  replaceFrameSize(Shape);
+  replaceFrameSizeAndAlign(Shape);
 
   // If there are no suspend points, no split required, just remove
   // the allocation and deallocation blocks, they are not needed.
Index: llvm/lib/Transforms/Coroutines/CoroInternal.h
===
--- llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -99,6 +99,7 @@
   CoroBeginInst *CoroBegin;
   SmallVector CoroEnds;
   SmallVector CoroSizes;
+  SmallVector CoroAligns;
   SmallVector CoroSuspends;
   SmallVector SwiftErrorOps;
 
Index: llvm/lib/Transforms/Coroutines/CoroInstr.h
===
--- llvm/lib/Transforms/Coroutines/CoroInstr.h
+++ llvm/lib/Transforms/Coroutines/CoroInstr.h
@@ -599,6 +599,18 @@
   }
 };
 
+/// This represents the llvm.coro.align instruction.
+class LLVM_LIBRARY_VISIBILITY CoroAlignInst : public IntrinsicInst {
+public:
+  // Methods to support type inquiry through isa, cast, and dyn_cast:
+  static bool classof(const IntrinsicInst *I) {
+return I->getIntrinsicID() == Intrinsic::coro_align;
+  }
+  static bool classof(const Value *V) {
+return isa(V) && classof(cast(V));
+  }
+};
+
 class LLVM_LIBRARY_VISIBILITY AnyCoroEndInst : public IntrinsicInst {
   enum { FrameArg, UnwindArg };
 
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ 

[PATCH] D94627: [PowerPC][PC Rel] Implement option to omit Power10 instructions from stubs

2021-03-04 Thread Albion Fung via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG36192790d84b: [PowerPC][PC Rel] Implement option to omit 
Power10 instructions from stubs (authored by Conanap).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94627

Files:
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/Options.td
  lld/ELF/Thunks.cpp
  lld/ELF/Thunks.h
  lld/test/ELF/ppc64-call-reach.s
  lld/test/ELF/ppc64-long-branch-localentry-offset.s
  lld/test/ELF/ppc64-long-branch-pi.s
  lld/test/ELF/ppc64-long-branch-rel14.s
  lld/test/ELF/ppc64-long-branch.s
  lld/test/ELF/ppc64-pcrel-call-to-extern.s
  lld/test/ELF/ppc64-pcrel-call-to-toc.s
  lld/test/ELF/ppc64-plt-stub-compatible.s
  lld/test/ELF/ppc64-tls-pcrel-gd.s
  lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
  lld/test/ELF/ppc64-toc-call-to-pcrel.s
  llvm/include/llvm/Object/ELF.h

Index: llvm/include/llvm/Object/ELF.h
===
--- llvm/include/llvm/Object/ELF.h
+++ llvm/include/llvm/Object/ELF.h
@@ -87,6 +87,9 @@
 
 enum PPCInstrMasks : uint64_t {
   PADDI_R12_NO_DISP = 0x06103980,
+  ADDIS_R12_TO_R2_NO_DISP = 0x3D82,
+  ADDI_R12_TO_R2_NO_DISP = 0x3982,
+  ADDI_R12_TO_R12_NO_DISP = 0x398C,
   PLD_R12_NO_DISP = 0x0410E580,
   MTCTR_R12 = 0x7D8903A6,
   BCTR = 0x4E800420,
Index: lld/test/ELF/ppc64-toc-call-to-pcrel.s
===
--- lld/test/ELF/ppc64-toc-call-to-pcrel.s
+++ lld/test/ELF/ppc64-toc-call-to-pcrel.s
@@ -14,6 +14,12 @@
 # RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=SYMBOL
 # RUN: llvm-objdump -d --no-show-raw-insn --mcpu=future %t | FileCheck %s
 
+# RUN: llvm-mc -filetype=obj -triple=powerpc64 %s -o %t.o
+# RUN: ld.lld -T %t.script %t.o -o %t --no-power10-stubs
+# RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=SYMBOL
+# RUN: llvm-objdump -d --no-show-raw-insn --mcpu=future %t \
+# RUN: | FileCheck %s
+
 # The point of this test is to make sure that when a function with TOC access
 # a local function with st_other=1, a TOC save stub is inserted.
 
Index: lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
===
--- lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
+++ lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s
@@ -11,15 +11,20 @@
 # RUN: llvm-objdump --mcpu=pwr10 --no-show-raw-insn -d %t_be | FileCheck %s
 # RUN: llvm-readelf -s %t_be | FileCheck %s --check-prefix=SYM
 
+# RUN: llvm-mc -filetype=obj -triple=powerpc64le %t/asm -o %t.o
+# RUN: ld.lld -T %t/lts %t.o -o %t_le --no-power10-stubs
+# RUN: llvm-objdump --mcpu=pwr10 --no-show-raw-insn -d %t_le | FileCheck %s --check-prefix=NoP10
+# RUN: llvm-readelf -s %t_le | FileCheck %s --check-prefix=SYM
+
 # SYM:  Symbol table '.symtab' contains 9 entries:
 # SYM:  1: 1001 0 NOTYPE  LOCAL  DEFAULT []   1 callee
 # SYM-NEXT: 2: 10020008 0 NOTYPE  LOCAL  DEFAULT  2 caller_close
 # SYM-NEXT: 3: 20020008 0 NOTYPE  LOCAL  DEFAULT []   3 caller
 # SYM-NEXT: 4: 000520020008 0 NOTYPE  LOCAL  DEFAULT  4 caller_far
-# SYM-NEXT: 5: 000520028038 0 NOTYPE  LOCAL  HIDDEN   6 .TOC.
+# SYM-NEXT: 5: 000520028040 0 NOTYPE  LOCAL  HIDDEN   6 .TOC.
 # SYM-NEXT: 6: 10020020 8 FUNCLOCAL  DEFAULT  2 __toc_save_callee
-# SYM-NEXT: 7: 2002002020 FUNCLOCAL  DEFAULT  3 __toc_save_callee
-# SYM-NEXT: 8: 00052002002020 FUNCLOCAL  DEFAULT  4 __toc_save_callee
+# SYM-NEXT: 7: 2002002032 FUNCLOCAL  DEFAULT  3 __toc_save_callee
+# SYM-NEXT: 8: 00052002002032 FUNCLOCAL  DEFAULT  4 __toc_save_callee
 
 #--- lts
 PHDRS {
@@ -72,6 +77,17 @@
 # CHECK-NEXT:paddi 12, 0, -268501028, 1
 # CHECK-NEXT:mtctr 12
 # CHECK-NEXT:bctr
+
+# NoP10-LABEL: :
+# NoP10: bl 0x20020020
+# NoP10-NEXT:ld 2, 24(1)
+# NoP10-NEXT:blr
+# NoP10-LABEL: <__toc_save_callee>:
+# NoP10-NEXT: std 2, 24(1)
+# NoP10-NEXT:addis 12, 2, -4098
+# NoP10-NEXT:addi 12, 12, 32704
+# NoP10-NEXT:mtctr 12
+# NoP10-NEXT:bctr
 .section .text_caller, "ax", %progbits
 .Lfunc_toc2:
   .quad .TOC.-.Lfunc_gep2
Index: lld/test/ELF/ppc64-tls-pcrel-gd.s
===
--- lld/test/ELF/ppc64-tls-pcrel-gd.s
+++ lld/test/ELF/ppc64-tls-pcrel-gd.s
@@ -42,10 +42,10 @@
 #--- asm
 
 # GD-RELOC: Relocation section '.rela.dyn' at offset 0x100b8 contains 4 entries:
-# GD-RELOC: 01001160  00010044 R_PPC64_DTPMOD64    x + 0
-# GD-RELOC: 01001168  0001004e R_PPC64_DTPREL64   

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

2021-03-04 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment.

Thank you for review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97535

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


[PATCH] D97967: [HIP] do not use -munsafe-fp-atomics by default

2021-03-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a subscriber: jfb.
yaxunl requested review of this revision.

A bug was introduced when adding -munsafe-fp-atomics.
By default it should be off.


https://reviews.llvm.org/D97967

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


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -36,6 +36,10 @@
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck 
-check-prefix=UNSAFE-FP-ATOMICS %s
 // UNSAFE-FP-ATOMICS: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-munsafe-fp-atomics"
 
+// RUN: %clang -### -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck 
-check-prefix=DEFAULT-UNSAFE-FP-ATOMICS %s
+// DEFAULT-UNSAFE-FP-ATOMICS-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" 
{{.*}} "-munsafe-fp-atomics"
+
 // RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib 
-fgpu-exclude-wrong-side-overloads \
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck 
-check-prefix=FIX-OVERLOAD %s
 // FIX-OVERLOAD: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fgpu-exclude-wrong-side-overloads" "-fgpu-defer-diag"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6438,7 +6438,7 @@
 handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs);
 
 if (Args.hasFlag(options::OPT_munsafe_fp_atomics,
- options::OPT_mno_unsafe_fp_atomics))
+ options::OPT_mno_unsafe_fp_atomics, /*Default=*/false))
   CmdArgs.push_back("-munsafe-fp-atomics");
   }
 


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -36,6 +36,10 @@
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=UNSAFE-FP-ATOMICS %s
 // UNSAFE-FP-ATOMICS: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-munsafe-fp-atomics"
 
+// RUN: %clang -### -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=DEFAULT-UNSAFE-FP-ATOMICS %s
+// DEFAULT-UNSAFE-FP-ATOMICS-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-munsafe-fp-atomics"
+
 // RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib -fgpu-exclude-wrong-side-overloads \
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=FIX-OVERLOAD %s
 // FIX-OVERLOAD: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fgpu-exclude-wrong-side-overloads" "-fgpu-defer-diag"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6438,7 +6438,7 @@
 handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs);
 
 if (Args.hasFlag(options::OPT_munsafe_fp_atomics,
- options::OPT_mno_unsafe_fp_atomics))
+ options::OPT_mno_unsafe_fp_atomics, /*Default=*/false))
   CmdArgs.push_back("-munsafe-fp-atomics");
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97902: [Driver] Clarify --gcc-toolchain

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 328227.
MaskRay added a comment.

Add more information to --gcc-toolchain.
Reference --gcc-toolchain in --prefix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/test/Driver/gcc-toolchain.cpp


Index: clang/test/Driver/gcc-toolchain.cpp
===
--- clang/test/Driver/gcc-toolchain.cpp
+++ clang/test/Driver/gcc-toolchain.cpp
@@ -29,3 +29,23 @@
 // CHECK: 
"{{[^"]*}}/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5{{/|}}crtbegin.o"
 // CHECK: "-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5"
 // CHECK: 
"-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5/../../../.."
+//
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN: --target=x86_64-linux-gnu -stdlib=libstdc++ \
+// RUN: --gcc-toolchain=%S/Inputs/ubuntu_13.04_multiarch_tree/usr \
+// RUN: --sysroot= | FileCheck %s --check-prefix=UBUNTU13
+
+/// If both --prefix and --gcc-toolchain are specified, the GCC installation 
with a larger version wins.
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN: --target=i386-unknown-linux -stdlib=libstdc++ \
+// RUN: --prefix=%S/Inputs/ubuntu_13.04_multiarch_tree/usr \
+// RUN: --gcc-toolchain=%S/Inputs/ubuntu_11.04_multiarch_tree/usr \
+// RUN: --sysroot= | FileCheck %s --check-prefix=UBUNTU13
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN: --target=i386-unknown-linux -stdlib=libstdc++ \
+// RUN: --prefix=%S/Inputs/ubuntu_11.04_multiarch_tree/usr \
+// RUN: --gcc-toolchain=%S/Inputs/ubuntu_13.04_multiarch_tree/usr \
+// RUN: --sysroot= | FileCheck %s --check-prefix=UBUNTU13
+
+// UBUNTU13: 
{{.*}}/ubuntu_13.04_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/4.7
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -599,8 +599,20 @@
 def _DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>,
 Flags<[NoXarchOption, CoreOption]>;
 def A : JoinedOrSeparate<["-"], "A">, Flags<[RenderJoined]>, 
Group;
-def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
-HelpText<"Add  to search path for binaries and object files used 
implicitly">;
+def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
+HelpText<"Search $prefix/$triple-$file and $prefix$file for executables, 
libraries, "
+"includes, and data files used by the compiler. If $prefix is a directory, 
the "
+"directory is used to search for GCC installation on targets which 
commonly use GCC. "
+"The directory usually contains 'include' and 'lib'. Note: sysroot is also 
used to "
+"search for GCC installation and the largest version wins. Specify 
--gcc-toolchain "
+"to suppress sysroot. If $prefix is not a directory, the GCC installation 
detection "
+"is skipped but $prefix can still be used to find executables, typically 
for -nostdinc "
+"-nostdlib compiles">;
+def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
+  HelpText<"Search for GCC installation in the specified directory, similar to 
-B. "
+  "The directory usually contains 'lib' and 'include'. If specified, sysroot 
is skipped "
+  "for GCC detection. Note: executables (e.g. ld) used by the compiler are not 
overridden "
+  "by the selected GCC installation">;
 def CC : Flag<["-"], "CC">, Flags<[CC1Option]>, Group,
 HelpText<"Include comments from within macros in preprocessed output">,
 MarshallingInfoFlag>;
@@ -3666,8 +3678,6 @@
   MarshallingInfoFlag>;
 def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias;
 def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias;
-def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
-  HelpText<"Use the gcc toolchain at the given directory">;
 def time : Flag<["-"], "time">,
   HelpText<"Time individual commands">;
 def traditional_cpp : Flag<["-", "--"], "traditional-cpp">, Flags<[CC1Option]>,
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -18,9 +18,9 @@
 
 
 .. program:: clang
-.. option:: -B, --prefix , --prefix=
+.. option:: -B, --prefix , --prefix=
 
-Add  to search path for binaries and object files used implicitly
+Search $prefix/$triple-$file and $prefix$file for executables, libraries, 
includes, and data files used by the compiler. If $prefix is a directory, the 
directory is used to search for GCC installation on targets which commonly use 
GCC. The directory usually contains 'include' and 'lib'. Note: sysroot is also 
used to search for GCC installation and 

[PATCH] D97702: [clang][modules] Use extensible RTTI for ModuleFileExtension

2021-03-04 Thread Artem Chikin via Phabricator via cfe-commits
artemcm accepted this revision.
artemcm added a comment.
This revision is now accepted and ready to land.

This looks good to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97702

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


[PATCH] D97902: [Driver] Clarify --gcc-toolchain

2021-03-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:23
 
-Add  to search path for binaries and object files used implicitly
+Search $prefix/$triple-$file and $prefix$file for executables, libraries, 
includes and data files used by the compiler. If $prefix is a directory, the 
directory is used to search for GCC installation on targets which commonly use 
GCC. The directory usually contains 'include' and 'lib'. If $prefix is not a 
directory, the GCC installation detection is skipped but $prefix can still be 
used to find executables, typically for -nostdinc -nostdlib compiles
 

MaskRay wrote:
> nickdesaulniers wrote:
> > I like how `--gcc-toolchain` references `-B`; can `-B` also reference 
> > `--gcc-toolchain`? That way developers also understand their options when 
> > looking at either? Or should we move `--gcc-toolchain` to be closer to 
> > `-B`, since they're slightly similar?
> `clang-tblgen -I ../llvm/include -I include/clang/Driver -gen-opt-docs 
> include/clang/Driver/ClangOptionDocs.td` sorts the options alphabetically so 
> it is not easy to move them together...
Ok. Can we still refer to `--gcc-toolchain` from here?



Comment at: clang/docs/ClangCommandLineReference.rst:259
 
-Use the gcc toolchain at the given directory
+Search for GCC installation in the specified directory, similar to -B.The 
directory usually contains 'lib' and 'include'.If specified, sysroot is not 
skipped for GCC detection
 

nickdesaulniers wrote:
> Add whitespace after periods.
Looks like there's another instance of this comment on this line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

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


[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

2021-03-04 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX updated this revision to Diff 328222.
ArcsinX added a comment.

Call `FileIndex::updateMain()` with an absolute file path in tests to avoid 
test failures in debug mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97535

Files:
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/test/memory_tree.test
  clang-tools-extra/clangd/unittests/DexTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/IndexTests.cpp

Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -229,7 +229,7 @@
   RefSlab Refs;
   auto Size = Symbols.bytes() + Refs.bytes();
   auto Data = std::make_pair(std::move(Symbols), std::move(Refs));
-  llvm::StringSet<> Files = {testPath("foo.cc"), testPath("bar.cc")};
+  llvm::StringSet<> Files = {"unittest:///foo.cc", "unittest:///bar.cc"};
   MemIndex I(std::move(Data.first), std::move(Data.second), RelationSlab(),
  std::move(Files), IndexContents::All, std::move(Data), Size);
   auto ContainsFile = I.indexedFiles();
@@ -506,7 +506,7 @@
   RefSlab DynRefs;
   auto DynSize = DynSymbols.bytes() + DynRefs.bytes();
   auto DynData = std::make_pair(std::move(DynSymbols), std::move(DynRefs));
-  llvm::StringSet<> DynFiles = {testPath("foo.cc")};
+  llvm::StringSet<> DynFiles = {"unittest:///foo.cc"};
   MemIndex DynIndex(std::move(DynData.first), std::move(DynData.second),
 RelationSlab(), std::move(DynFiles), IndexContents::Symbols,
 std::move(DynData), DynSize);
@@ -514,7 +514,7 @@
   RefSlab StaticRefs;
   auto StaticData =
   std::make_pair(std::move(StaticSymbols), std::move(StaticRefs));
-  llvm::StringSet<> StaticFiles = {testPath("foo.cc"), testPath("bar.cc")};
+  llvm::StringSet<> StaticFiles = {"unittest:///foo.cc", "unittest:///bar.cc"};
   MemIndex StaticIndex(
   std::move(StaticData.first), std::move(StaticData.second), RelationSlab(),
   std::move(StaticFiles), IndexContents::References, std::move(StaticData),
Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -352,14 +352,14 @@
   Test.Code = std::string(MainCode.code());
   Test.Filename = "test.cc";
   auto AST = Test.build();
-  Index.updateMain(Test.Filename, AST);
+  Index.updateMain(testPath(Test.Filename), AST);
   // Add test2.cc
   TestTU Test2;
   Test2.HeaderCode = HeaderCode;
   Test2.Code = std::string(MainCode.code());
   Test2.Filename = "test2.cc";
   AST = Test2.build();
-  Index.updateMain(Test2.Filename, AST);
+  Index.updateMain(testPath(Test2.Filename), AST);
 
   EXPECT_THAT(getRefs(Index, Foo.ID),
   RefsAre({AllOf(RefRange(MainCode.range("foo")),
@@ -387,7 +387,7 @@
   Test.Code = std::string(MainCode.code());
   Test.Filename = "test.cc";
   auto AST = Test.build();
-  Index.updateMain(Test.Filename, AST);
+  Index.updateMain(testPath(Test.Filename), AST);
 
   auto HeaderMacro = findSymbol(Test.headerSymbols(), "HEADER_MACRO");
   EXPECT_THAT(getRefs(Index, HeaderMacro.ID),
Index: clang-tools-extra/clangd/unittests/DexTests.cpp
===
--- clang-tools-extra/clangd/unittests/DexTests.cpp
+++ clang-tools-extra/clangd/unittests/DexTests.cpp
@@ -737,7 +737,7 @@
   RefSlab Refs;
   auto Size = Symbols.bytes() + Refs.bytes();
   auto Data = std::make_pair(std::move(Symbols), std::move(Refs));
-  llvm::StringSet<> Files = {testPath("foo.cc"), testPath("bar.cc")};
+  llvm::StringSet<> Files = {"unittest:///foo.cc", "unittest:///bar.cc"};
   Dex I(std::move(Data.first), std::move(Data.second), RelationSlab(),
 std::move(Files), IndexContents::All, std::move(Data), Size);
   auto ContainsFile = I.indexedFiles();
Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- clang-tools-extra/clangd/test/memory_tree.test
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -23,7 +23,9 @@
 # CHECK-NEXT: "_total": {{[0-9]+}}
 # CHECK-NEXT:   },
 # CHECK-NEXT:   "slabs": {
-# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "test:///main.cpp": {
 # CHECK-NEXT:   "_self": {{[0-9]+}},
 # CHECK-NEXT:   "_total": {{[0-9]+}},
 # CHECK-NEXT:   "references": {
@@ -38,9 

[PATCH] D97902: [Driver] Clarify --gcc-toolchain

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 328220.
MaskRay marked 2 inline comments as done.
MaskRay added a comment.

comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/test/Driver/gcc-toolchain.cpp


Index: clang/test/Driver/gcc-toolchain.cpp
===
--- clang/test/Driver/gcc-toolchain.cpp
+++ clang/test/Driver/gcc-toolchain.cpp
@@ -29,3 +29,23 @@
 // CHECK: 
"{{[^"]*}}/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5{{/|}}crtbegin.o"
 // CHECK: "-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5"
 // CHECK: 
"-L[[TOOLCHAIN]]/usr/lib/i386-linux-gnu/gcc/i686-linux-gnu/4.5/../../../.."
+//
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN: --target=x86_64-linux-gnu -stdlib=libstdc++ \
+// RUN: --gcc-toolchain=%S/Inputs/ubuntu_13.04_multiarch_tree/usr \
+// RUN: --sysroot= | FileCheck %s --check-prefix=UBUNTU13
+
+/// If both --prefix and --gcc-toolchain are specified, the GCC installation 
with a larger version wins.
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN: --target=i386-unknown-linux -stdlib=libstdc++ \
+// RUN: --prefix=%S/Inputs/ubuntu_13.04_multiarch_tree/usr \
+// RUN: --gcc-toolchain=%S/Inputs/ubuntu_11.04_multiarch_tree/usr \
+// RUN: --sysroot= | FileCheck %s --check-prefix=UBUNTU13
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t 2>&1 \
+// RUN: --target=i386-unknown-linux -stdlib=libstdc++ \
+// RUN: --prefix=%S/Inputs/ubuntu_11.04_multiarch_tree/usr \
+// RUN: --gcc-toolchain=%S/Inputs/ubuntu_13.04_multiarch_tree/usr \
+// RUN: --sysroot= | FileCheck %s --check-prefix=UBUNTU13
+
+// UBUNTU13: 
{{.*}}/ubuntu_13.04_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/4.7
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -599,8 +599,16 @@
 def _DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>,
 Flags<[NoXarchOption, CoreOption]>;
 def A : JoinedOrSeparate<["-"], "A">, Flags<[RenderJoined]>, 
Group;
-def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
-HelpText<"Add  to search path for binaries and object files used 
implicitly">;
+def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
+HelpText<"Search $prefix/$triple-$file and $prefix$file for executables, 
libraries, includes, and data files used by the compiler. "
+"If $prefix is a directory, the directory is used to search for GCC 
installation on targets which commonly use GCC. "
+"The directory usually contains 'include' and 'lib'. "
+"If $prefix is not a directory, the GCC installation detection is skipped 
but $prefix can still be used to "
+"find executables, typically for -nostdinc -nostdlib compiles">;
+def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
+  HelpText<"Search for GCC installation in the specified directory, similar to 
-B. "
+  "The directory usually contains 'lib' and 'include'."
+  "If specified, sysroot is not skipped for GCC detection">;
 def CC : Flag<["-"], "CC">, Flags<[CC1Option]>, Group,
 HelpText<"Include comments from within macros in preprocessed output">,
 MarshallingInfoFlag>;
@@ -3666,8 +3674,6 @@
   MarshallingInfoFlag>;
 def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias;
 def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias;
-def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
-  HelpText<"Use the gcc toolchain at the given directory">;
 def time : Flag<["-"], "time">,
   HelpText<"Time individual commands">;
 def traditional_cpp : Flag<["-", "--"], "traditional-cpp">, Flags<[CC1Option]>,
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -18,9 +18,9 @@
 
 
 .. program:: clang
-.. option:: -B, --prefix , --prefix=
+.. option:: -B, --prefix , --prefix=
 
-Add  to search path for binaries and object files used implicitly
+Search $prefix/$triple-$file and $prefix$file for executables, libraries, 
includes, and data files used by the compiler. If $prefix is a directory, the 
directory is used to search for GCC installation on targets which commonly use 
GCC. The directory usually contains 'include' and 'lib'. If $prefix is not a 
directory, the GCC installation detection is skipped but $prefix can still be 
used to find executables, typically for -nostdinc -nostdlib compiles
 
 .. option:: -F
 
@@ -256,7 +256,7 @@
 
 .. option:: --gcc-toolchain=, -gcc-toolchain 
 
-Use the gcc toolchain at the given directory
+Search for GCC installation in the specified 

[PATCH] D97902: [Driver] Clarify --gcc-toolchain

2021-03-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:23
 
-Add  to search path for binaries and object files used implicitly
+Search $prefix/$triple-$file and $prefix$file for executables, libraries, 
includes and data files used by the compiler. If $prefix is a directory, the 
directory is used to search for GCC installation on targets which commonly use 
GCC. The directory usually contains 'include' and 'lib'. If $prefix is not a 
directory, the GCC installation detection is skipped but $prefix can still be 
used to find executables, typically for -nostdinc -nostdlib compiles
 

nickdesaulniers wrote:
> I like how `--gcc-toolchain` references `-B`; can `-B` also reference 
> `--gcc-toolchain`? That way developers also understand their options when 
> looking at either? Or should we move `--gcc-toolchain` to be closer to `-B`, 
> since they're slightly similar?
`clang-tblgen -I ../llvm/include -I include/clang/Driver -gen-opt-docs 
include/clang/Driver/ClangOptionDocs.td` sorts the options alphabetically so it 
is not easy to move them together...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

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


[PATCH] D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg.

2021-03-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

In D97224#2596410 , @rjmccall wrote:

> Do we really consider the existing atomic intrinsics to not impose added 
> alignment restrictions?  I'm somewhat concerned that we're going to produce 
> IR here that's either really suboptimal or, worse, unimplementable, just 
> because we over-interpreted some cue about alignment.  I guess it would only 
> be a significant problem on a target where types are frequently under-aligned 
> for what atomics need, which is not typical, or when the user is doing 
> atomics on a field of something like a packed struct.

For the `__atomic_*` intrinsics, we don't consider those as imposing additional 
alignment restrictions -- currently, we avoid generating the LLVM IR 
instruction if it's below natural alignment, since we couldn't specify 
alignment on the IR instruction. (Now that we have alignment on the LLVM IR 
operations, I'd like to eventually get rid of that logic from Clang, since it's 
also handled by LLVM.)

For other intrinsics (e.g. MSVCIntrin::_InterlockedAnd, 
Builtin::BI__sync_fetch_and_add_4, NVPTX::BI__nvvm_atom_add_gen_i, and the 
others in those 3 function families), we currently entirely ignore the 
alignment, and simply assume the argument is naturally-aligned. So, yes, this 
change could potentially affect the behavior for underaligned types.

So, I could change these to explicitly increase the assumed alignment of their 
arguments, like I did for InterlockedCompareExchange128. My inclination is not 
to do so, however...it doesn't seem like a good idea in general to ignore type 
alignment. But, I'd not be opposed to doing that, if there's a good reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97224

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


[PATCH] D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg.

2021-03-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 328219.
jyknight marked 2 inline comments as done.
jyknight added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97224

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/atomic-ops.c
  clang/test/CodeGen/ms-intrinsics.c
  clang/test/OpenMP/parallel_reduction_codegen.cpp

Index: clang/test/OpenMP/parallel_reduction_codegen.cpp
===
--- clang/test/OpenMP/parallel_reduction_codegen.cpp
+++ clang/test/OpenMP/parallel_reduction_codegen.cpp
@@ -198,7 +198,7 @@
 // LAMBDA: br label %[[REDUCTION_DONE]]
 // LAMBDA: [[CASE2]]
 // LAMBDA: [[G_PRIV_VAL:%.+]] = load i32, i32* [[G_PRIVATE_ADDR]]
-// LAMBDA: atomicrmw add i32* [[G_REF]], i32 [[G_PRIV_VAL]] monotonic, align 4
+// LAMBDA: atomicrmw add i32* [[G_REF]], i32 [[G_PRIV_VAL]] monotonic, align 128
 // LAMBDA: br label %[[REDUCTION_DONE]]
 // LAMBDA: [[REDUCTION_DONE]]
 // LAMBDA: ret void
@@ -255,7 +255,7 @@
 // BLOCKS: br label %[[REDUCTION_DONE]]
 // BLOCKS: [[CASE2]]
 // BLOCKS: [[G_PRIV_VAL:%.+]] = load i32, i32* [[G_PRIVATE_ADDR]]
-// BLOCKS: atomicrmw add i32* [[G_REF]], i32 [[G_PRIV_VAL]] monotonic, align 4
+// BLOCKS: atomicrmw add i32* [[G_REF]], i32 [[G_PRIV_VAL]] monotonic, align 128
 // BLOCKS: br label %[[REDUCTION_DONE]]
 // BLOCKS: [[REDUCTION_DONE]]
 // BLOCKS: ret void
@@ -771,7 +771,7 @@
 // case 2:
 // t_var += t_var_reduction;
 // CHECK: [[T_VAR_PRIV_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[T_VAR_PRIV]]
-// CHECK: atomicrmw add i32* [[T_VAR_REF]], i32 [[T_VAR_PRIV_VAL]] monotonic, align 4
+// CHECK: atomicrmw add i32* [[T_VAR_REF]], i32 [[T_VAR_PRIV_VAL]] monotonic, align 128
 
 // var = var.operator &(var_reduction);
 // CHECK: call void @__kmpc_critical(
@@ -801,7 +801,7 @@
 
 // t_var1 = min(t_var1, t_var1_reduction);
 // CHECK: [[T_VAR1_PRIV_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[T_VAR1_PRIV]]
-// CHECK: atomicrmw min i32* [[T_VAR1_REF]], i32 [[T_VAR1_PRIV_VAL]] monotonic, align 4
+// CHECK: atomicrmw min i32* [[T_VAR1_REF]], i32 [[T_VAR1_PRIV_VAL]] monotonic, align 128
 
 // break;
 // CHECK: br label %[[RED_DONE]]
Index: clang/test/CodeGen/ms-intrinsics.c
===
--- clang/test/CodeGen/ms-intrinsics.c
+++ clang/test/CodeGen/ms-intrinsics.c
@@ -450,10 +450,10 @@
 // CHECK-64: [[EL:%[0-9]+]] = zext i64 %inc1 to i128
 // CHECK-64: [[EHS:%[0-9]+]] = shl nuw i128 [[EH]], 64
 // CHECK-64: [[EXP:%[0-9]+]] = or i128 [[EHS]], [[EL]]
-// CHECK-64: [[ORG:%[0-9]+]] = load i128, i128* [[CNR]], align 16
+// CHECK-64: [[ORG:%[0-9]+]] = load i128, i128* [[CNR]], align 8
 // CHECK-64: [[RES:%[0-9]+]] = cmpxchg volatile i128* [[DST]], i128 [[ORG]], i128 [[EXP]] seq_cst seq_cst, align 16
 // CHECK-64: [[OLD:%[0-9]+]] = extractvalue { i128, i1 } [[RES]], 0
-// CHECK-64: store i128 [[OLD]], i128* [[CNR]], align 16
+// CHECK-64: store i128 [[OLD]], i128* [[CNR]], align 8
 // CHECK-64: [[SUC1:%[0-9]+]] = extractvalue { i128, i1 } [[RES]], 1
 // CHECK-64: [[SUC8:%[0-9]+]] = zext i1 [[SUC1]] to i8
 // CHECK-64: ret i8 [[SUC8]]
Index: clang/test/CodeGen/atomic-ops.c
===
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -655,9 +655,9 @@
   __atomic_load(_a, _b, memory_order_seq_cst);
   // CHECK: store atomic i64 {{.*}}, align 16
   __atomic_store(_a, _b, memory_order_seq_cst);
-  // CHECK: atomicrmw xchg i64* {{.*}}, align 8
+  // CHECK: atomicrmw xchg i64* {{.*}}, align 16
   __atomic_exchange(_a, _b, _c, memory_order_seq_cst);
-  // CHECK: cmpxchg weak i64* {{.*}}, align 8
+  // CHECK: cmpxchg weak i64* {{.*}}, align 16
   __atomic_compare_exchange(_a, _b, _c, 1, memory_order_seq_cst, memory_order_seq_cst);
 }
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -5150,7 +5150,7 @@
 X.getType()->hasSignedIntegerRepresentation());
   }
   llvm::Value *Res =
-  CGF.Builder.CreateAtomicRMW(RMWOp, X.getPointer(CGF), UpdateVal, AO);
+  CGF.Builder.CreateAtomicRMW(RMWOp, X.getAddress(CGF), UpdateVal, AO);
   return std::make_pair(true, RValue::get(Res));
 }
 
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2430,7 +2430,7 @@
   // For atomic bool increment, we just store true and return it for
   // preincrement, do an atomic swap with true for postincrement
   return 

[PATCH] D97902: [Driver] Clarify --gcc-toolchain

2021-03-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

Thanks for the patch! It is a much needed clarification!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

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


[PATCH] D97902: [Driver] Clarify --gcc-toolchain

2021-03-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:23
 
-Add  to search path for binaries and object files used implicitly
+Search $prefix/$triple-$file and $prefix$file for executables, libraries, 
includes and data files used by the compiler. If $prefix is a directory, the 
directory is used to search for GCC installation on targets which commonly use 
GCC. The directory usually contains 'include' and 'lib'. If $prefix is not a 
directory, the GCC installation detection is skipped but $prefix can still be 
used to find executables, typically for -nostdinc -nostdlib compiles
 

I like how `--gcc-toolchain` references `-B`; can `-B` also reference 
`--gcc-toolchain`? That way developers also understand their options when 
looking at either? Or should we move `--gcc-toolchain` to be closer to `-B`, 
since they're slightly similar?



Comment at: clang/docs/ClangCommandLineReference.rst:259
 
-Use the gcc toolchain at the given directory
+Search for GCC installation in the specified directory, similar to -B.The 
directory usually contains 'lib' and 'include'.If specified, sysroot is not 
skipped for GCC detection
 

Add whitespace after periods.



Comment at: clang/include/clang/Driver/Options.td:603
+def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
+HelpText<"Search $prefix/$triple-$file and $prefix$file for executables, 
libraries, includes and data files used by the compiler. "
+"If $prefix is a directory, the directory is used to search for GCC 
installation on targets which commonly use GCC. "

s/includes/includes,/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97902

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


[PATCH] D97965: [clang] Fix crash when creating deduction guide.

2021-03-04 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz updated this revision to Diff 328206.
adamcz added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97965

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -416,6 +416,16 @@
 
 }
 
+namespace no_crash_on_default_arg {
+class A {
+  template  class B {
+B(int c = 1);
+  };
+  // This used to crash due to unparsed default arg above.
+  B(); // expected-error {{deduction guide declaration without trailing return 
type}}
+};
+} // namespace no_crash_on_default_arg
+
 #pragma clang diagnostic push
 #pragma clang diagnostic warning "-Wctad-maybe-unsupported"
 namespace test_implicit_ctad_warning {
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2494,6 +2494,12 @@
 if (!CD || (!FTD && CD->isFunctionTemplateSpecialization()))
   continue;
 
+// Cannot make a deduction guide when unparsed arguments are present.
+if (std::any_of(CD->param_begin(), CD->param_end(), [](ParmVarDecl *P) {
+  return !P || P->hasUnparsedDefaultArg();
+}))
+  continue;
+
 Transform.transformConstructor(FTD, CD);
 AddedAny = true;
   }


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -416,6 +416,16 @@
 
 }
 
+namespace no_crash_on_default_arg {
+class A {
+  template  class B {
+B(int c = 1);
+  };
+  // This used to crash due to unparsed default arg above.
+  B(); // expected-error {{deduction guide declaration without trailing return type}}
+};
+} // namespace no_crash_on_default_arg
+
 #pragma clang diagnostic push
 #pragma clang diagnostic warning "-Wctad-maybe-unsupported"
 namespace test_implicit_ctad_warning {
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2494,6 +2494,12 @@
 if (!CD || (!FTD && CD->isFunctionTemplateSpecialization()))
   continue;
 
+// Cannot make a deduction guide when unparsed arguments are present.
+if (std::any_of(CD->param_begin(), CD->param_end(), [](ParmVarDecl *P) {
+  return !P || P->hasUnparsedDefaultArg();
+}))
+  continue;
+
 Transform.transformConstructor(FTD, CD);
 AddedAny = true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97965: [clang] Fix crash when creating deduction guide.

2021-03-04 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz created this revision.
adamcz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We used to trigger assertion when transforming c-tor with unparsed
default argument. Now we ignore such constructors for this purpose.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97965

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -416,6 +416,16 @@
 
 }
 
+namespace no_crash_on_default_arg {
+class A {
+  template class B {
+B(int c = 1);
+  };
+  // This used to crash due to unparsed default arg above.
+  B();  // expected-error {{deduction guide declaration without trailing 
return type}}
+};
+}
+
 #pragma clang diagnostic push
 #pragma clang diagnostic warning "-Wctad-maybe-unsupported"
 namespace test_implicit_ctad_warning {
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2494,6 +2494,12 @@
 if (!CD || (!FTD && CD->isFunctionTemplateSpecialization()))
   continue;
 
+// Cannot make a deduction guide when unparsed arguments are present.
+if (std::any_of(CD->param_begin(), CD->param_end(), [](ParmVarDecl *P) {
+  return !P || P->hasUnparsedDefaultArg();
+}))
+  continue;
+
 Transform.transformConstructor(FTD, CD);
 AddedAny = true;
   }


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -416,6 +416,16 @@
 
 }
 
+namespace no_crash_on_default_arg {
+class A {
+  template class B {
+B(int c = 1);
+  };
+  // This used to crash due to unparsed default arg above.
+  B();  // expected-error {{deduction guide declaration without trailing return type}}
+};
+}
+
 #pragma clang diagnostic push
 #pragma clang diagnostic warning "-Wctad-maybe-unsupported"
 namespace test_implicit_ctad_warning {
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2494,6 +2494,12 @@
 if (!CD || (!FTD && CD->isFunctionTemplateSpecialization()))
   continue;
 
+// Cannot make a deduction guide when unparsed arguments are present.
+if (std::any_of(CD->param_begin(), CD->param_end(), [](ParmVarDecl *P) {
+  return !P || P->hasUnparsedDefaultArg();
+}))
+  continue;
+
 Transform.transformConstructor(FTD, CD);
 AddedAny = true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97552: [clang][cli] Fix generation of '-fvisibility' with regards to '-mignore-xcoff-visibility'

2021-03-04 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97552

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


[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment.

Could you describe in more detail what problem this patch solves?
Also, since you are adding a new intrinsics, please also update the coroutine 
documentation regarding this new intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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


[PATCH] D97702: [clang][modules] Use extensible RTTI for ModuleFileExtension

2021-03-04 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese added a comment.

Looks fine to me, but it would be good for a Swift person to look at this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97702

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


[PATCH] D97963: [ASTMatchers][Dynamic] Provide Fallback and suggestions for typos.

2021-03-04 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Tests haven't been added yet, wanted to gauge opinions on this first before 
fully committing to it.




Comment at: clang/lib/ASTMatchers/Dynamic/TypoSuggester.cpp:40
+} // namespace clang
\ No newline at end of file


Duly noted :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97963

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


[PATCH] D97512: [clang] removes check against integral-to-pointer conversion...

2021-03-04 Thread Christopher Di Bella via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9830901b341c: [clang] removes check against 
integral-to-pointer conversion... (authored by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97512

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Analysis/free.c
  clang/test/Analysis/free.cpp


Index: clang/test/Analysis/free.cpp
===
--- clang/test/Analysis/free.cpp
+++ clang/test/Analysis/free.cpp
@@ -208,3 +208,39 @@
   // Unknown value
   std::free(x[offset]); // no-warning
 }
+
+struct S {
+  const char* p;
+};
+
+void t18_C_style_C_style_free (S s) {
+  free((void*)(unsigned long long)s.p); // no warning
+}
+
+void t18_C_style_C_style_std_free (S s) {
+  std::free((void*)(unsigned long long)s.p); // no warning
+}
+
+void t18_C_style_reinterpret_free (S s) {
+  free((void*)reinterpret_cast(s.p)); // no warning
+}
+
+void t18_C_style_reinterpret_std_free (S s) {
+  std::free((void*)reinterpret_cast(s.p)); // no warning
+}
+
+void t18_reinterpret_C_style_free (S s) {
+  free(reinterpret_cast((unsigned long long)(s.p))); // no warning
+}
+
+void t18_reinterpret_C_style_std_free (S s) {
+  std::free(reinterpret_cast((unsigned long long)(s.p))); // no warning
+}
+
+void t18_reinterpret_reinterpret_free (S s) {
+  free(reinterpret_cast(reinterpret_cast(s.p))); // 
no warning
+}
+
+void t18_reinterpret_reinterpret_std_free (S s) {
+  std::free(reinterpret_cast(reinterpret_cast(s.p))); // no warning
+}
Index: clang/test/Analysis/free.c
===
--- clang/test/Analysis/free.c
+++ clang/test/Analysis/free.c
@@ -108,3 +108,11 @@
   // expected-warning@-1{{Argument to free() is the address of the function 
'iptr', which is not memory allocated by malloc()}}
   // expected-warning@-2{{attempt to call free on non-heap object 'iptr'}}
 }
+
+struct S {
+  const char* p;
+};
+
+void t18 (struct S s) {
+  free((void*)(unsigned long long)s.p); // no warning
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10316,11 +10316,18 @@
 const CastExpr *Cast) {
   SmallString<128> SizeString;
   llvm::raw_svector_ostream OS(SizeString);
+
+  clang::CastKind Kind = Cast->getCastKind();
+  if (Kind == clang::CK_BitCast &&
+  !Cast->getSubExpr()->getType()->isFunctionPointerType())
+return;
+  if (Kind == clang::CK_IntegralToPointer &&
+  !isa(
+  Cast->getSubExpr()->IgnoreParenImpCasts()->IgnoreParens()))
+return;
+
   switch (Cast->getCastKind()) {
   case clang::CK_BitCast:
-if (!Cast->getSubExpr()->getType()->isFunctionPointerType())
-  return;
-LLVM_FALLTHROUGH;
   case clang::CK_IntegralToPointer:
   case clang::CK_FunctionToPointerDecay:
 OS << '\'';


Index: clang/test/Analysis/free.cpp
===
--- clang/test/Analysis/free.cpp
+++ clang/test/Analysis/free.cpp
@@ -208,3 +208,39 @@
   // Unknown value
   std::free(x[offset]); // no-warning
 }
+
+struct S {
+  const char* p;
+};
+
+void t18_C_style_C_style_free (S s) {
+  free((void*)(unsigned long long)s.p); // no warning
+}
+
+void t18_C_style_C_style_std_free (S s) {
+  std::free((void*)(unsigned long long)s.p); // no warning
+}
+
+void t18_C_style_reinterpret_free (S s) {
+  free((void*)reinterpret_cast(s.p)); // no warning
+}
+
+void t18_C_style_reinterpret_std_free (S s) {
+  std::free((void*)reinterpret_cast(s.p)); // no warning
+}
+
+void t18_reinterpret_C_style_free (S s) {
+  free(reinterpret_cast((unsigned long long)(s.p))); // no warning
+}
+
+void t18_reinterpret_C_style_std_free (S s) {
+  std::free(reinterpret_cast((unsigned long long)(s.p))); // no warning
+}
+
+void t18_reinterpret_reinterpret_free (S s) {
+  free(reinterpret_cast(reinterpret_cast(s.p))); // no warning
+}
+
+void t18_reinterpret_reinterpret_std_free (S s) {
+  std::free(reinterpret_cast(reinterpret_cast(s.p))); // no warning
+}
Index: clang/test/Analysis/free.c
===
--- clang/test/Analysis/free.c
+++ clang/test/Analysis/free.c
@@ -108,3 +108,11 @@
   // expected-warning@-1{{Argument to free() is the address of the function 'iptr', which is not memory allocated by malloc()}}
   // expected-warning@-2{{attempt to call free on non-heap object 'iptr'}}
 }
+
+struct S {
+  const char* p;
+};
+
+void t18 (struct S s) {
+  free((void*)(unsigned long long)s.p); // no warning
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10316,11 +10316,18 @@
 const CastExpr 

[clang] 9830901 - [clang] removes check against integral-to-pointer conversion...

2021-03-04 Thread Christopher Di Bella via cfe-commits

Author: Christopher Di Bella
Date: 2021-03-04T17:00:54Z
New Revision: 9830901b341cfb884cdef00e0335c6e3e62d107a

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

LOG: [clang] removes check against integral-to-pointer conversion...

... unless it's a literal

D94640 was a bit too aggressive in its analysis, considering integers
representing valid addresses as invalid. This change rolls back some of
the check, so that only the most obvious case is still flagged.

Before:

```cpp
free((void*)1000);   // literal converted to `void*`: warning good
free((void*)an_int); // `int` object converted to `void*`: warning might
 //  be a false positive
```

After

```cpp
free((void*)1000);   // literal converted to `void*`: warning good
free((void*)an_int); // doesn't warn
```

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/Analysis/free.c
clang/test/Analysis/free.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 7e6dd354caace..22dd634c5031a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10316,11 +10316,18 @@ void CheckFreeArgumentsCast(Sema , const 
std::string ,
 const CastExpr *Cast) {
   SmallString<128> SizeString;
   llvm::raw_svector_ostream OS(SizeString);
+
+  clang::CastKind Kind = Cast->getCastKind();
+  if (Kind == clang::CK_BitCast &&
+  !Cast->getSubExpr()->getType()->isFunctionPointerType())
+return;
+  if (Kind == clang::CK_IntegralToPointer &&
+  !isa(
+  Cast->getSubExpr()->IgnoreParenImpCasts()->IgnoreParens()))
+return;
+
   switch (Cast->getCastKind()) {
   case clang::CK_BitCast:
-if (!Cast->getSubExpr()->getType()->isFunctionPointerType())
-  return;
-LLVM_FALLTHROUGH;
   case clang::CK_IntegralToPointer:
   case clang::CK_FunctionToPointerDecay:
 OS << '\'';

diff  --git a/clang/test/Analysis/free.c b/clang/test/Analysis/free.c
index 84d53472158cd..59767b5917d79 100644
--- a/clang/test/Analysis/free.c
+++ b/clang/test/Analysis/free.c
@@ -108,3 +108,11 @@ void t17(void) {
   // expected-warning@-1{{Argument to free() is the address of the function 
'iptr', which is not memory allocated by malloc()}}
   // expected-warning@-2{{attempt to call free on non-heap object 'iptr'}}
 }
+
+struct S {
+  const char* p;
+};
+
+void t18 (struct S s) {
+  free((void*)(unsigned long long)s.p); // no warning
+}

diff  --git a/clang/test/Analysis/free.cpp b/clang/test/Analysis/free.cpp
index 2559770d6ddb7..85b0935a51992 100644
--- a/clang/test/Analysis/free.cpp
+++ b/clang/test/Analysis/free.cpp
@@ -208,3 +208,39 @@ void t17b (char **x, int offset) {
   // Unknown value
   std::free(x[offset]); // no-warning
 }
+
+struct S {
+  const char* p;
+};
+
+void t18_C_style_C_style_free (S s) {
+  free((void*)(unsigned long long)s.p); // no warning
+}
+
+void t18_C_style_C_style_std_free (S s) {
+  std::free((void*)(unsigned long long)s.p); // no warning
+}
+
+void t18_C_style_reinterpret_free (S s) {
+  free((void*)reinterpret_cast(s.p)); // no warning
+}
+
+void t18_C_style_reinterpret_std_free (S s) {
+  std::free((void*)reinterpret_cast(s.p)); // no warning
+}
+
+void t18_reinterpret_C_style_free (S s) {
+  free(reinterpret_cast((unsigned long long)(s.p))); // no warning
+}
+
+void t18_reinterpret_C_style_std_free (S s) {
+  std::free(reinterpret_cast((unsigned long long)(s.p))); // no warning
+}
+
+void t18_reinterpret_reinterpret_free (S s) {
+  free(reinterpret_cast(reinterpret_cast(s.p))); // 
no warning
+}
+
+void t18_reinterpret_reinterpret_std_free (S s) {
+  std::free(reinterpret_cast(reinterpret_cast(s.p))); // no warning
+}



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


[PATCH] D97963: [ASTMatchers][Dynamic] Provide Fallback and suggestions for typos.

2021-03-04 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
njames93 added reviewers: aaron.ballman, klimek, steveire.
Herald added a subscriber: mgorny.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If there is a typo in parsing and we can see a close match, offer a suggestion 
hint and in the case of matchers carry on parsing to see if more diags are 
generated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97963

Files:
  clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
  clang/include/clang/ASTMatchers/Dynamic/Parser.h
  clang/include/clang/ASTMatchers/Dynamic/Registry.h
  clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
  clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
  clang/lib/ASTMatchers/Dynamic/Parser.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/ASTMatchers/Dynamic/TypoSuggester.cpp
  clang/lib/ASTMatchers/Dynamic/TypoSuggester.h

Index: clang/lib/ASTMatchers/Dynamic/TypoSuggester.h
===
--- /dev/null
+++ clang/lib/ASTMatchers/Dynamic/TypoSuggester.h
@@ -0,0 +1,54 @@
+//===- TypoSuggester.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_ASTMATCHERS_DYNAMIC_TYPOSUGGESTER_H
+#define LLVM_CLANG_ASTMATCHERS_DYNAMIC_TYPOSUGGESTER_H
+
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace ast_matchers {
+namespace dynamic {
+// Helper class for getting the closest match for a typo string.
+// If OwnSuggestions is true, temporary strings can be passed to addSuggestion.
+class TypoSuggester {
+  llvm::StringRef Current;
+  const llvm::StringRef Value;
+  unsigned BestDistance;
+  bool HasMatch = false;
+
+public:
+  enum AutoMaxEditT { AutoMaxEdit };
+  TypoSuggester(llvm::StringRef Value, AutoMaxEditT)
+  : Value(Value), BestDistance((Value.size() + 3U) / 5U) {
+// This BestDistance is rather arbitrary but prevents generating suggestions
+// for single character values and grows more lenient as the value grows.
+  }
+  TypoSuggester(llvm::StringRef Target,
+unsigned MaxEdit = std::numeric_limits::max())
+  : Value(Target), BestDistance(MaxEdit) {
+assert(MaxEdit > 0);
+  }
+
+  // Returns true if we reach a state where we can never find a unique closest
+  // match.
+  bool addSuggestion(llvm::StringRef Suggest);
+  // If true, we are in a state where we can never find a unique closest match.
+  bool hasFailed() const { return BestDistance == 0; }
+  bool foundSuggestion() const { return HasMatch; }
+  llvm::StringRef getSuggestion() const {
+assert(foundSuggestion());
+return Current;
+  }
+};
+
+} // namespace dynamic
+} // namespace ast_matchers
+} // namespace clang
+
+#endif // LLVM_CLANG_ASTMATCHERS_DYNAMIC_TYPOSUGGESTER_H
Index: clang/lib/ASTMatchers/Dynamic/TypoSuggester.cpp
===
--- /dev/null
+++ clang/lib/ASTMatchers/Dynamic/TypoSuggester.cpp
@@ -0,0 +1,39 @@
+//===- TypoSuggester.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TypoSuggester.h"
+
+namespace clang {
+namespace ast_matchers {
+namespace dynamic {
+bool TypoSuggester::addSuggestion(llvm::StringRef Suggest) {
+  unsigned Dist = Value.edit_distance(Suggest, true, BestDistance);
+  if (Dist > BestDistance)
+return false;
+  if (Dist == BestDistance) {
+if (HasMatch) {
+  // If we already have a completion candidate with the same edit
+  // distance, we can't be sure so disregard both.
+  HasMatch = false;
+  --BestDistance;
+  // As we assume Target will never appear in Suggest, once we reach a
+  // case where we need 0 edits, we can stop work.
+  return BestDistance == 0;
+}
+Current = Suggest;
+HasMatch = true;
+return false;
+  }
+  Current = Suggest;
+  HasMatch = true;
+  BestDistance = Dist;
+  return false;
+}
+} // namespace dynamic
+} // namespace ast_matchers
+} // namespace clang
\ No newline at end of file
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/ASTMatchers/Dynamic/Registry.h"
 #include "Marshallers.h"
+#include "TypoSuggester.h"
 #include "clang/AST/ASTTypeTraits.h"
 

[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

2021-03-04 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95244

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


[PATCH] D97125: Stop traping on sNaN in __builtin_isinf

2021-03-04 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre requested review of this revision.
thopre added a comment.

Requesting review since the logic has changed. This time I've also tested 
isfinite against glibc's result. All looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97125

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


[PATCH] D97960: [clang-tidy] bugprone-signal-handler improvements: display call chain

2021-03-04 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: martong, gamesh411, Szelethus, dkrupp, xazax.hun, 
whisperity.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add a new feature to display the call chain if an unsafe function is
found in a signal handler or function called from it.
Warning messages are improved too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97960

Files:
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-minimal.c
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-posix.c
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
@@ -19,11 +19,11 @@
   S(int) {
 std::other_call();
 // Should be fixed.
-// CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+// CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
   };
   int operator-() const {
 std::other_call();
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
   }
 };
 
@@ -31,14 +31,14 @@
   static int memberInit() {
 std::other_call();
 // Should be fixed.
-// CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+// CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
   }
   int M = memberInit();
 };
 
 int defaultInit() {
   std::other_call();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 
 void testDefaultInit(int I = defaultInit()) {
@@ -70,13 +70,13 @@
 
 void handler_bad1(int) {
   std::other_call();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 
 void handler_bad2(int) {
   std::SysStruct S;
   S << 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator<<' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'operator<<' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 
 void handler_bad3(int) {
@@ -98,7 +98,7 @@
 
 void handler_bad7(int) {
   int I = []() { std::other_call(); return 2; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 }
 
@@ -110,16 +110,23 @@
   std::signal(SIGINT, handler_abort);
   std::signal(SIGINT, handler__Exit);
   std::signal(SIGINT, handler_signal);
+
   std::signal(SIGINT, handler_bad1);
+
   std::signal(SIGINT, handler_bad2);
+
   // test call of user-defined operator
   std::signal(SIGINT, handler_bad3);
+
   // test call of constructor
   std::signal(SIGINT, handler_bad4);
+
   // test call of default member initializer
   std::signal(SIGINT, handler_bad5);
+
   // test call of default argument initializer
   std::signal(SIGINT, handler_bad6);
+
   // test lambda call in handler
   std::signal(SIGINT, handler_bad7);
 
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c

  1   2   >