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

[Bing Yu 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 rG8198d83965ba: [X86] Pass to transform amx intrinsics to 
scalar operation. (authored by LuoYuanke, committed by yubing).

Changed prior to commit:
  https://reviews.llvm.org/D93594?vs=328408&id=328412#toc

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 &P : 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

[PATCH] D98012: [RFC][doc] Document that RISC-V's __fp16 has different behavior

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng created this revision.
Herald added subscribers: vkmr, luismarques, sameer.abuasal, s.egerton, Jim, 
PkmX, rogfer01, shiva0217, simoncook.
kito-cheng requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

RISC-V has a draft extension for half-precision proposed last year[1],
so we think adding a new type for that would be great to make this
easier to use that extension.

We found there is `_Float16` and `__fp16` types are supported on GCC and
Clang, but `_Float16` has C++ supporting issue on GCC site, so we think
support both type is a reasonable choice to us.

However we would like have slight different behavior for `__fp16` other
than ACLE: The evaluation format of `__fp16` set same as `_Float16`,
which means no promotion are performed if there is no hardware half-precision
supported.

The only concern to us is it's defined differnt with clang's document,
so we would put this RFC patch here, to make sure it's OK for make
`__fp16` has differnt behavior between differnt targets.

This patch contain document change only, implementation would be in
another patches.

[1] https://github.com/riscv/riscv-isa-manual/pull/496
[2] https://github.com/riscv/riscv-elf-psabi-doc/pull/172

Co-authored-by: Evandro Menezes 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98012

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] D97950: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97950

___
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

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

steakhal added a comment.

In D96976#2605313 , @RedDocMD wrote:

> 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).

What is that 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.

Do you have anything specific in mind about implementing the 3rd option?
Why do you think it's //significantly complex//?

Leaving assertion failures is not an option.
IMO if this feature is important to you, then you should pursue the 3rd.


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.

[Zakk Chen via Phabricator via cfe-commits]

khchen added a comment.

Overall LGTM.




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());

khchen wrote:
> if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain))
Sorry, I forget this rule: 
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return



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

kito-cheng wrote:
> khchen wrote:
> > ```
> > if (!File)
> >   return;
> > ```
> Good suggestion, thanks! 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


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] D97928: [AMDGPU] Restore the s_memtime instruction in gfx1030

[Jay Foad via Phabricator via cfe-commits]

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

Add clang changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97928

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGenOpenCL/amdgpu-features.cl
  clang/test/SemaOpenCL/builtins-amdgcn-error-gfx1030.cl
  llvm/lib/Target/AMDGPU/AMDGPU.td
  llvm/lib/Target/AMDGPU/GCNSubtarget.h
  llvm/lib/Target/AMDGPU/SMInstructions.td
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.memtime.ll
  llvm/test/MC/AMDGPU/gfx1030_err.s

Index: llvm/test/MC/AMDGPU/gfx1030_err.s
===
--- llvm/test/MC/AMDGPU/gfx1030_err.s
+++ llvm/test/MC/AMDGPU/gfx1030_err.s
@@ -21,9 +21,6 @@
 s_get_waveid_in_workgroup s0
 // GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 
-s_memtime s[0:1]
-// GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
 s_getreg_b32 s2, hwreg(HW_REG_XNACK_MASK)
 // GFX10: :[[@LINE-1]]:{{[0-9]+}}: error: specified hardware register is not supported on this GPU
 
Index: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.memtime.ll
===
--- llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.memtime.ll
+++ llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.memtime.ll
@@ -1,7 +1,7 @@
 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck --check-prefixes=SIVI,GCN %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global -verify-machineinstrs < %s | FileCheck --check-prefixes=SIVI,GCN %s
 ; RUN: llc -march=amdgcn -mcpu=gfx1010 -mattr=-flat-for-global -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
-; RUN: not --crash llc -march=amdgcn -mcpu=gfx1030 -mattr=-flat-for-global -verify-machineinstrs < %s 2>&1 | FileCheck -check-prefix=GFX1030-ERR %s
+; RUN: llc -march=amdgcn -mcpu=gfx1030 -mattr=-flat-for-global -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 
 declare i64 @llvm.amdgcn.s.memtime() #0
 
@@ -13,7 +13,6 @@
 ; SIVI-NOT: lgkmcnt
 ; GCN: s_memtime s{{\[[0-9]+:[0-9]+\]}}
 ; GCN: {{buffer|global}}_store_dwordx2
-; GFX1030-ERR: ERROR
 define amdgpu_kernel void @test_s_memtime(i64 addrspace(1)* %out) #0 {
   %cycle0 = call i64 @llvm.amdgcn.s.memtime()
   store volatile i64 %cycle0, i64 addrspace(1)* %out
Index: llvm/lib/Target/AMDGPU/SMInstructions.td
===
--- llvm/lib/Target/AMDGPU/SMInstructions.td
+++ llvm/lib/Target/AMDGPU/SMInstructions.td
@@ -866,14 +866,16 @@
 >;
 } // let OtherPredicates = [HasSMemTimeInst]
 
-let OtherPredicates = [HasNoSMemTimeInst] in {
+let OtherPredicates = [HasShaderCyclesRegister] in {
 def : GCNPat <
   (i64 (readcyclecounter)),
   (REG_SEQUENCE SReg_64,
 (S_GETREG_B32 getHwRegImm.ret), sub0,
-(S_MOV_B32 (i32 0)), sub1)
->;
-} // let OtherPredicates = [HasNoSMemTimeInst]
+(S_MOV_B32 (i32 0)), sub1)> {
+  // Prefer this to s_memtime because it has lower and more predictable latency.
+  let AddedComplexity = 1;
+}
+} // let OtherPredicates = [HasShaderCyclesRegister]
 
 //===--===//
 // GFX10.
Index: llvm/lib/Target/AMDGPU/GCNSubtarget.h
===
--- llvm/lib/Target/AMDGPU/GCNSubtarget.h
+++ llvm/lib/Target/AMDGPU/GCNSubtarget.h
@@ -163,6 +163,7 @@
   bool HasVscnt;
   bool HasGetWaveIdInst;
   bool HasSMemTimeInst;
+  bool HasShaderCyclesRegister;
   bool HasRegisterBanking;
   bool HasVOP3Literal;
   bool HasNoDataDepHazard;
@@ -714,6 +715,10 @@
 return HasSMemTimeInst;
   }
 
+  bool hasShaderCyclesRegister() const {
+return HasShaderCyclesRegister;
+  }
+
   bool hasRegisterBanking() const {
 return HasRegisterBanking;
   }
Index: llvm/lib/Target/AMDGPU/AMDGPU.td
===
--- llvm/lib/Target/AMDGPU/AMDGPU.td
+++ llvm/lib/Target/AMDGPU/AMDGPU.td
@@ -563,6 +563,12 @@
   "Has s_memtime instruction"
 >;
 
+def FeatureShaderCyclesRegister : SubtargetFeature<"shader-cycles-register",
+  "HasShaderCyclesRegister",
+  "true",
+  "Has SHADER_CYCLES hardware register"
+>;
+
 def FeatureMadMacF32Insts : SubtargetFeature<"mad-mac-f32-insts",
   "HasMadMacF32Insts",
   "true",
@@ -777,7 +783,7 @@
FeatureNoSdstCMPX, FeatureVscnt, FeatureRegisterBanking,
FeatureVOP3Literal, FeatureDPP8, FeatureExtendedImageInsts,
FeatureNoDataDepHazard, FeaturePkFmacF16Inst,
-   FeatureGFX10A16, FeatureFastDenormalF32, FeatureG16,
+   FeatureGFX10A16, FeatureSMemTimeInst, FeatureFastDenormalF32, FeatureG16,
FeatureUnalignedBufferAccess, FeatureUnalignedDSAccess
   ]
 >;
@@ -988,7 +994,6 @@
  FeatureScalarAtomics,
  FeatureScalarFlatScratchInsts,
  FeatureGetWaveIdInst,
- FeatureSMemTimeInst,
 

[clang-tools-extra] a60d06d - [clangd] Rename Module -> FeatureModule to avoid confusion. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-03-05T10:04:00+01:00
New Revision: a60d06d8b757b656726b70e83e76417297c7057b

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

LOG: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC

As pointed out in D96244, "Module" is already pretty overloaded to refer
to clang and llvm modules. (And clangd deals directly with the former).

FeatureModule is a bit of a mouthful but it's pretty self-descriptive.
I think it might be better than "Component" which doesn't really capture
the "common interface" aspect - it's IMO confusing to refer to
"components" but exclude CDB for example.

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

Added: 
clang-tools-extra/clangd/FeatureModule.cpp
clang-tools-extra/clangd/FeatureModule.h

Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Removed: 
clang-tools-extra/clangd/Module.cpp
clang-tools-extra/clangd/Module.h



diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index bf654a23e00c..a6229a229002 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -62,6 +62,7 @@ add_clang_library(clangDaemon
   DraftStore.cpp
   DumpAST.cpp
   ExpectedTypes.cpp
+  FeatureModule.cpp
   FindSymbols.cpp
   FindTarget.cpp
   FileDistance.cpp
@@ -75,7 +76,6 @@ add_clang_library(clangDaemon
   Hover.cpp
   IncludeFixer.cpp
   JSONTransport.cpp
-  Module.cpp
   PathMapping.cpp
   Protocol.cpp
   Quality.cpp

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index ba263d123f1d..df5377d77f2b 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -580,8 +580,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   {
 LSPBinder Binder(Handlers, *this);
 bindMethods(Binder, Params.capabilities);
-if (Opts.Modules)
-  for (auto &Mod : *Opts.Modules)
+if (Opts.FeatureModules)
+  for (auto &Mod : *Opts.FeatureModules)
 Mod.initializeLSP(Binder, Params.rawCapabilities, ServerCaps);
   }
 

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index d24271aafeae..cbd9677f340c 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -129,7 +129,7 @@ ClangdServer::Options::operator TUScheduler::Options() 
const {
 ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
const ThreadsafeFS &TFS, const Options &Opts,
Callbacks *Callbacks)
-: Modules(Opts.Modules), CDB(CDB), TFS(TFS),
+: FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
   DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
   ClangTidyProvider(Opts.ClangTidyProvider),
   WorkspaceRoot(Opts.WorkspaceRoot) {
@@ -168,13 +168,13 @@ ClangdServer::ClangdServer(const 
GlobalCompilationDatabase &CDB,
   if (DynamicIdx)
 AddIndex(DynamicIdx.get());
 
-  if (Opts.Modules) {
-Module::Facilities F{
+  if (Opts.FeatureModules) {
+FeatureModule::Facilities F{
 *this->WorkScheduler,
 this->Index,
 this->TFS,
 };
-for (auto &Mod : *Opts.Modules)
+for (auto &Mod : *Opts.FeatureModules)
   Mod.initialize(F);
   }
 }
@@ -184,11 +184,11 @@ ClangdServer::~ClangdServer() {
   // otherwise access members concurrently.
   // (Nobody can be using TUScheduler because we're on the main thread).
   WorkScheduler.reset();
-  // Now requests have stopped, we can shut down modules.
-  if (Modules) {
-for (auto &Mod : *Modules)
+  // Now requests have stopped, we can shut down feature modules.
+  if (FeatureModules) {
+for (auto &Mod : *FeatureModules)
   Mod.stop();
-for (auto &Mod : *Modules)
+for (auto &Mod : *FeatureModules)
   Mod.blockUntilIdle(Deadline::infinity());
   }
 }
@@ -920,7 +920,7 @@ ClangdServer::blockUntilIdleForTest(llvm::Optional 
TimeoutSeconds) {
   return false;
 if (BackgroundIdx && !BackgroundIdx->blockUntilIdleForTest(Timeout))
   return false;
-if (Modules && llvm::any_of(*Modules, [&](Module &M) {
+if (FeatureModules && llvm::any_of(*FeatureModules, [&](FeatureModule &M) {
   return !M.blockUntilIdle(timeoutSeconds(Timeout));
 }))
   return false;

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index fc710ae4724d..5254bfe78b71 100644
--- a/clang-

[PATCH] D97950: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC

[Sam McCall 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 rGa60d06d8b757: [clangd] Rename Module -> FeatureModule to 
avoid confusion. NFC (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97950

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FeatureModule.cpp
  clang-tools-extra/clangd/FeatureModule.h
  clang-tools-extra/clangd/Module.cpp
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -42,7 +42,7 @@
 Base = ClangdServer::optsForTest();
 // This is needed to we can test index-based operations like call hierarchy.
 Base.BuildDynamicSymbolIndex = true;
-Base.Modules = &Modules;
+Base.FeatureModules = &FeatureModules;
   }
 
   LSPClient &start() {
@@ -70,7 +70,7 @@
 
   MockFS FS;
   ClangdLSPServer::Options Opts;
-  ModuleSet Modules;
+  FeatureModuleSet FeatureModules;
 
 private:
   // Color logs so we can distinguish them from test output.
@@ -227,7 +227,7 @@
 }
 
 TEST_F(LSPTest, ModulesTest) {
-  class MathModule final : public Module {
+  class MathModule final : public FeatureModule {
 OutgoingNotification Changed;
 void initializeLSP(LSPBinder &Bind, const llvm::json::Object &ClientCaps,
llvm::json::Object &ServerCaps) override {
@@ -248,7 +248,7 @@
   [Reply(std::move(Reply)), Value(Value)]() mutable { Reply(Value); });
 }
   };
-  Modules.add(std::make_unique());
+  FeatureModules.add(std::make_unique());
 
   auto &Client = start();
   Client.notify("add", 2);
@@ -266,10 +266,10 @@
   return [&Out](llvm::Expected V) { Out.emplace(std::move(V)); };
 }
 
-TEST_F(LSPTest, ModulesThreadingTest) {
-  // A module that does its work on a background thread, and so exercises the
-  // block/shutdown protocol.
-  class AsyncCounter final : public Module {
+TEST_F(LSPTest, FeatureModulesThreadingTest) {
+  // A feature module that does its work on a background thread, and so
+  // exercises the block/shutdown protocol.
+  class AsyncCounter final : public FeatureModule {
 bool ShouldStop = false;
 int State = 0;
 std::deque> Queue; // null = increment, non-null = read.
@@ -347,19 +347,19 @@
 }
   };
 
-  Modules.add(std::make_unique());
+  FeatureModules.add(std::make_unique());
   auto &Client = start();
 
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
   EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
-  EXPECT_EQ(3, Modules.get()->getSync());
+  EXPECT_EQ(3, FeatureModules.get()->getSync());
   // Throw some work on the queue to make sure shutdown blocks on it.
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
-  // And immediately shut down. Module destructor verifies that we blocked.
+  // And immediately shut down. FeatureModule destructor verifies we blocked.
 }
 
 } // namespace
Index: clang-tools-extra/clangd/FeatureModule.h
===
--- clang-tools-extra/clangd/FeatureModule.h
+++ clang-tools-extra/clangd/FeatureModule.h
@@ -1,4 +1,4 @@
-//===--- Module.h - Plugging features into clangd -*-C++-*-===//
+//===--- FeatureModule.h - Plugging features into clangd --*-C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FEATUREMODULE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FEATUREMODULE_H
 
 #include "support/Function.h"
 #include "support/Threading.h"
@@ -26,11 +26,11 @@
 class ThreadsafeFS;
 class TUScheduler;
 
-/// A Module contributes a vertical feature to clangd.
+/// A FeatureModule contributes a vertical feature to clangd.
 ///
 /// The lifetime of a module is roughly:
-///  - modules are created before the LSP server, in ClangdMain.cpp
-///  - these modules are then passed to ClangdLSPServer in a ModuleSet
+///  - feature modules are created before the LSP server, in ClangdMain.cpp
+///  - these modules are then passed to ClangdLSPServer in a FeatureModuleSet
 ///  - 

[PATCH] D98018: [Fuchsia] Enable Polly for first stage as well

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added a reviewer: leonardchan.
Herald added a subscriber: mgorny.
Herald added a reviewer: bollu.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We want Polly enabled for both stages of the toolchain build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98018

Files:
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm;polly" CACHE STRING 
"")
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm;polly" CACHE STRING "")
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kent Sommer via Phabricator via cfe-commits]

kentsommer accepted this revision.
kentsommer added a comment.

LGTM

Sorry for the slow response!


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] D98022: [clang] Fix typos in the default logic for CLANG_DEFAULT_UNWINDLIB

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: saugustine, MaskRay, phosek.
Herald added a subscriber: mgorny.
mstorsjo requested review of this revision.
Herald added a project: clang.

CLANG_DEFAULT_RTLIB had a typo, and libunwind isn't a valid option for it.

This keeps the practical behaviour from before, defaulting to none if using 
compiler-rt as rtlib.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98022

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -262,7 +262,7 @@
 if (CLANG_DEFAULT_UNWINDLIB STREQUAL "")
   if (CLANG_DEFAULT_RTLIB STREQUAL "libgcc")
 set (CLANG_DEFAULT_UNWINDLIB "libgcc" CACHE STRING "" FORCE)
-  elseif (CLANG_DEFAULT_RTLIBS STREQUAL "libunwind")
+  elseif (CLANG_DEFAULT_RTLIB STREQUAL "compiler-rt")
 set (CLANG_DEFAULT_UNWINDLIB "none" CACHE STRING "" FORCE)
   endif()
 endif()


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -262,7 +262,7 @@
 if (CLANG_DEFAULT_UNWINDLIB STREQUAL "")
   if (CLANG_DEFAULT_RTLIB STREQUAL "libgcc")
 set (CLANG_DEFAULT_UNWINDLIB "libgcc" CACHE STRING "" FORCE)
-  elseif (CLANG_DEFAULT_RTLIBS STREQUAL "libunwind")
+  elseif (CLANG_DEFAULT_RTLIB STREQUAL "compiler-rt")
 set (CLANG_DEFAULT_UNWINDLIB "none" CACHE STRING "" FORCE)
   endif()
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-03-05T10:57:06+01:00
New Revision: fcf75ae6ce20e3575b1464ce724619f38c43edd2

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

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

This patch fixes failure of the `CodeGen/aix-ignore-xcoff-visibility.cpp` test 
with command line round-trip.

The absence of '-fvisibility' implies '-mignore-xcoff-visibility'.

The problem is that when '-fvisibility default' is passed to -cc1, it isn't 
being generated. (This adheres to the principle that generation doesn't produce 
arguments with default values.)

However, that caused '-mignore-xcoff-visibility' to be implied in the generated 
command line (without '-fvisibility'), while it wasn't implied in the original 
command line (with '-fvisibility').

This patch fixes that by always generating '-fvisibility' and explains the 
situation in comment.

(The '-mginore-xcoff-visibility' option was added in D87451).

Reviewed By: Bigcheese

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d4d48deb649f..299d4619c7e1 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5321,7 +5321,9 @@ def stack_protector_buffer_size : Separate<["-"], 
"stack-protector-buffer-size">
   MarshallingInfoInt, "8">;
 def fvisibility : Separate<["-"], "fvisibility">,
   HelpText<"Default type and symbol visibility">,
-  MarshallingInfoVisibility, 
"DefaultVisibility">;
+  MarshallingInfoVisibility, 
"DefaultVisibility">,
+  // Always emitting because of the relation to `-mignore-xcoff-visibility`.
+  AlwaysEmit;
 def ftype_visibility : Separate<["-"], "ftype-visibility">,
   HelpText<"Default type visibility">,
   MarshallingInfoVisibility, 
fvisibility.KeyPath>;

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index b0654f9d706e..89e485a29384 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1831,6 +1831,23 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions 
&Opts, ArgList &Args,
 }
   }
 
+  // This is the reason why '-fvisibility' needs to be always generated:
+  // its absence implies '-mignore-xcoff-visibility'.
+  //
+  // Suppose the original cc1 command line does contain '-fvisibility default':
+  // '-mignore-xcoff-visibility' should not be implied.
+  // * If '-fvisibility' is not generated (as most options with default values
+  //   don't), its absence would imply '-mignore-xcoff-visibility'. This 
changes
+  //   the command line semantics.
+  // * If '-fvisibility' is generated regardless of its presence and value,
+  //   '-mignore-xcoff-visibility' won't be implied and the command line
+  //   semantics are kept intact.
+  //
+  // When the original cc1 command line does **not** contain '-fvisibility',
+  // '-mignore-xcoff-visibility' is implied. The generated command line will
+  // contain both '-fvisibility default' and '-mignore-xcoff-visibility' and
+  // subsequent calls to `CreateFromArgs`/`generateCC1CommandLine` will always
+  // produce the same arguments.
   if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility) ||
   !Args.hasArg(OPT_fvisibility)))
 Opts.IgnoreXCOFFVisibility = 1;



___
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'

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfcf75ae6ce20: [clang][cli] Fix generation of 
'-fvisibility' with regards to '-mignore-xcoff… (authored by 
jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D97552?vs=326676&id=328448#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97552

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1831,6 +1831,23 @@
 }
   }
 
+  // This is the reason why '-fvisibility' needs to be always generated:
+  // its absence implies '-mignore-xcoff-visibility'.
+  //
+  // Suppose the original cc1 command line does contain '-fvisibility default':
+  // '-mignore-xcoff-visibility' should not be implied.
+  // * If '-fvisibility' is not generated (as most options with default values
+  //   don't), its absence would imply '-mignore-xcoff-visibility'. This 
changes
+  //   the command line semantics.
+  // * If '-fvisibility' is generated regardless of its presence and value,
+  //   '-mignore-xcoff-visibility' won't be implied and the command line
+  //   semantics are kept intact.
+  //
+  // When the original cc1 command line does **not** contain '-fvisibility',
+  // '-mignore-xcoff-visibility' is implied. The generated command line will
+  // contain both '-fvisibility default' and '-mignore-xcoff-visibility' and
+  // subsequent calls to `CreateFromArgs`/`generateCC1CommandLine` will always
+  // produce the same arguments.
   if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility) ||
   !Args.hasArg(OPT_fvisibility)))
 Opts.IgnoreXCOFFVisibility = 1;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5321,7 +5321,9 @@
   MarshallingInfoInt, "8">;
 def fvisibility : Separate<["-"], "fvisibility">,
   HelpText<"Default type and symbol visibility">,
-  MarshallingInfoVisibility, 
"DefaultVisibility">;
+  MarshallingInfoVisibility, 
"DefaultVisibility">,
+  // Always emitting because of the relation to `-mignore-xcoff-visibility`.
+  AlwaysEmit;
 def ftype_visibility : Separate<["-"], "ftype-visibility">,
   HelpText<"Default type visibility">,
   MarshallingInfoVisibility, 
fvisibility.KeyPath>;


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1831,6 +1831,23 @@
 }
   }
 
+  // This is the reason why '-fvisibility' needs to be always generated:
+  // its absence implies '-mignore-xcoff-visibility'.
+  //
+  // Suppose the original cc1 command line does contain '-fvisibility default':
+  // '-mignore-xcoff-visibility' should not be implied.
+  // * If '-fvisibility' is not generated (as most options with default values
+  //   don't), its absence would imply '-mignore-xcoff-visibility'. This changes
+  //   the command line semantics.
+  // * If '-fvisibility' is generated regardless of its presence and value,
+  //   '-mignore-xcoff-visibility' won't be implied and the command line
+  //   semantics are kept intact.
+  //
+  // When the original cc1 command line does **not** contain '-fvisibility',
+  // '-mignore-xcoff-visibility' is implied. The generated command line will
+  // contain both '-fvisibility default' and '-mignore-xcoff-visibility' and
+  // subsequent calls to `CreateFromArgs`/`generateCC1CommandLine` will always
+  // produce the same arguments.
   if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility) ||
   !Args.hasArg(OPT_fvisibility)))
 Opts.IgnoreXCOFFVisibility = 1;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5321,7 +5321,9 @@
   MarshallingInfoInt, "8">;
 def fvisibility : Separate<["-"], "fvisibility">,
   HelpText<"Default type and symbol visibility">,
-  MarshallingInfoVisibility, "DefaultVisibility">;
+  MarshallingInfoVisibility, "DefaultVisibility">,
+  // Always emitting because of the relation to `-mignore-xcoff-visibility`.
+  AlwaysEmit;
 def ftype_visibility : Separate<["-"], "ftype-visibility">,
   HelpText<"Default type visibility">,
   MarshallingInfoVisibility, fvisibility.KeyPath>;
___
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.

[Nicolas Vasilache via Phabricator via cfe-commits]

nicolasvasilache added a comment.

This seems to break the build 
https://buildkite.com/mlir/mlir-core/builds/12026#91ec4dfe-542f-4312-92db-7d555f05ce06.

I could repro locally, reverting locally fixes the build.

Please address, thanks!


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

[clang] 4295ae9 - [clang][modules] Use extensible RTTI for ModuleFileExtension

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-03-05T11:11:05+01:00
New Revision: 4295ae96cdf275cdda8bded9271960a4cac11fb2

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

LOG: [clang][modules] Use extensible RTTI for ModuleFileExtension

Clang exposes an interface for extending the PCM/PCH file format: 
`ModuleFileExtension`.

Clang itself has only a single implementation of the interface: 
`TestModuleFileExtension` that can be instantiated via the 
`-ftest-module-file_extension=` command line argument (and is stored in 
`FrontendOptions::ModuleFileExtensions`).

Clients of the Clang library can extend the PCM/PCH file format by pushing an 
instance of their extension class to the 
`FrontendOptions::ModuleFileExtensions` vector.

When generating the `-ftest-module-file_extension=` command line argument from 
`FrontendOptions`, a downcast is used to distinguish between the Clang's 
testing extension and other (client) extensions.

This functionality is enabled by LLVM-style RTTI. However, this style of RTTI 
is hard to extend, as it requires patching Clang (adding new case to the 
`ModuleFileExtensionKind` enum).

This patch switches to the LLVM RTTI for open class hierarchies, which allows 
libClang users (e.g. Swift) to create implementations of `ModuleFileExtension` 
without patching Clang. (Documentation of the feature: 
https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html#rtti-for-open-class-hierarchies)

Reviewed By: artemcm

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

Added: 


Modified: 
clang/include/clang/Serialization/ModuleFileExtension.h
clang/lib/Frontend/TestModuleFileExtension.cpp
clang/lib/Frontend/TestModuleFileExtension.h
clang/lib/Serialization/ModuleFileExtension.cpp
clang/unittests/Frontend/CompilerInvocationTest.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ModuleFileExtension.h 
b/clang/include/clang/Serialization/ModuleFileExtension.h
index e9ac4c22520e..34ea870724a4 100644
--- a/clang/include/clang/Serialization/ModuleFileExtension.h
+++ b/clang/include/clang/Serialization/ModuleFileExtension.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_SERIALIZATION_MODULEFILEEXTENSION_H
 
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/ExtensibleRTTI.h"
 #include 
 #include 
 
@@ -59,21 +60,16 @@ class ModuleFileExtensionWriter;
 /// compiled module files (.pcm) and precompiled headers (.pch) via a
 /// custom writer that can then be accessed via a custom reader when
 /// the module file or precompiled header is loaded.
-class ModuleFileExtension {
-protected:
-  /// Discriminator for LLVM-style RTTI.
-  enum ModuleFileExtensionKind {
-MFEK_Test,
-  };
-
-  const ModuleFileExtensionKind Kind;
+///
+/// Subclasses must use LLVM RTTI for open class hierarchies.
+class ModuleFileExtension
+: public llvm::RTTIExtends {
 public:
-  ModuleFileExtension(ModuleFileExtensionKind Kind) : Kind(Kind) {}
+  /// Discriminator for LLVM RTTI.
+  static char ID;
 
   virtual ~ModuleFileExtension();
 
-  ModuleFileExtensionKind getKind() const { return Kind; }
-
   /// Retrieves the metadata for this module file extension.
   virtual ModuleFileExtensionMetadata getExtensionMetadata() const = 0;
 

diff  --git a/clang/lib/Frontend/TestModuleFileExtension.cpp 
b/clang/lib/Frontend/TestModuleFileExtension.cpp
index ec147e16c569..7d4026a7efc6 100644
--- a/clang/lib/Frontend/TestModuleFileExtension.cpp
+++ b/clang/lib/Frontend/TestModuleFileExtension.cpp
@@ -15,6 +15,8 @@
 using namespace clang;
 using namespace clang::serialization;
 
+char TestModuleFileExtension::ID = 0;
+
 TestModuleFileExtension::Writer::~Writer() { }
 
 void TestModuleFileExtension::Writer::writeExtensionContents(

diff  --git a/clang/lib/Frontend/TestModuleFileExtension.h 
b/clang/lib/Frontend/TestModuleFileExtension.h
index ee7312502e24..c8ca4cd4f210 100644
--- a/clang/lib/Frontend/TestModuleFileExtension.h
+++ b/clang/lib/Frontend/TestModuleFileExtension.h
@@ -17,7 +17,8 @@
 namespace clang {
 
 /// A module file extension used for testing purposes.
-class TestModuleFileExtension : public ModuleFileExtension {
+class TestModuleFileExtension
+: public llvm::RTTIExtends {
   std::string BlockName;
   unsigned MajorVersion;
   unsigned MinorVersion;
@@ -43,15 +44,13 @@ class TestModuleFileExtension : public ModuleFileExtension {
   };
 
 public:
-  TestModuleFileExtension(StringRef BlockName,
-  unsigned MajorVersion,
-  unsigned MinorVersion,
-  bool Hashed,
+  static char ID;
+
+  TestModuleFileExtension(StringRef BlockName, unsigned MajorVersion,
+  unsigned MinorVersion, bool Hashed,
   StringRef UserInfo)
-: ModuleFileExtension(Module

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

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4295ae96cdf2: [clang][modules] Use extensible RTTI for 
ModuleFileExtension (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D97702?vs=327163&id=328453#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97702

Files:
  clang/include/clang/Serialization/ModuleFileExtension.h
  clang/lib/Frontend/TestModuleFileExtension.cpp
  clang/lib/Frontend/TestModuleFileExtension.h
  clang/lib/Serialization/ModuleFileExtension.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -11,6 +11,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Serialization/ModuleFileExtension.h"
 #include "llvm/Support/Host.h"
 
 #include "gmock/gmock.h"
@@ -743,6 +744,58 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdigraphs")));
 }
 
+struct DummyModuleFileExtension
+: public llvm::RTTIExtends {
+  static char ID;
+
+  ModuleFileExtensionMetadata getExtensionMetadata() const override {
+return {};
+  };
+
+  llvm::hash_code hashExtension(llvm::hash_code Code) const override {
+return {};
+  }
+
+  std::unique_ptr
+  createExtensionWriter(ASTWriter &Writer) override {
+return {};
+  }
+
+  std::unique_ptr
+  createExtensionReader(const ModuleFileExtensionMetadata &Metadata,
+ASTReader &Reader, serialization::ModuleFile &Mod,
+const llvm::BitstreamCursor &Stream) override {
+return {};
+  }
+};
+
+char DummyModuleFileExtension::ID = 0;
+
+TEST_F(CommandLineTest, TestModuleFileExtension) {
+  const char *Args[] = {"-ftest-module-file-extension=first:2:1:0:first",
+"-ftest-module-file-extension=second:3:2:1:second"};
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+  ASSERT_THAT(Invocation.getFrontendOpts().ModuleFileExtensions.size(), 2);
+
+  // Exercise the check that only serializes instances of
+  // TestModuleFileExtension by providing an instance of another
+  // ModuleFileExtension subclass.
+  Invocation.getFrontendOpts().ModuleFileExtensions.push_back(
+  std::make_shared());
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs,
+  ContainsN(HasSubstr("-ftest-module-file-extension="), 2));
+  ASSERT_THAT(
+  GeneratedArgs,
+  Contains(StrEq("-ftest-module-file-extension=first:2:1:0:first")));
+  ASSERT_THAT(
+  GeneratedArgs,
+  Contains(StrEq("-ftest-module-file-extension=second:3:2:1:second")));
+}
+
 TEST_F(CommandLineTest, RoundTrip) {
   // Testing one marshalled and one manually generated option from each
   // CompilerInvocation member.
Index: clang/lib/Serialization/ModuleFileExtension.cpp
===
--- clang/lib/Serialization/ModuleFileExtension.cpp
+++ clang/lib/Serialization/ModuleFileExtension.cpp
@@ -9,6 +9,8 @@
 #include "llvm/ADT/Hashing.h"
 using namespace clang;
 
+char ModuleFileExtension::ID = 0;
+
 ModuleFileExtension::~ModuleFileExtension() { }
 
 llvm::hash_code ModuleFileExtension::hashExtension(llvm::hash_code Code) const {
Index: clang/lib/Frontend/TestModuleFileExtension.h
===
--- clang/lib/Frontend/TestModuleFileExtension.h
+++ clang/lib/Frontend/TestModuleFileExtension.h
@@ -17,7 +17,8 @@
 namespace clang {
 
 /// A module file extension used for testing purposes.
-class TestModuleFileExtension : public ModuleFileExtension {
+class TestModuleFileExtension
+: public llvm::RTTIExtends {
   std::string BlockName;
   unsigned MajorVersion;
   unsigned MinorVersion;
@@ -43,15 +44,13 @@
   };
 
 public:
-  TestModuleFileExtension(StringRef BlockName,
-  unsigned MajorVersion,
-  unsigned MinorVersion,
-  bool Hashed,
+  static char ID;
+
+  TestModuleFileExtension(StringRef BlockName, unsigned MajorVersion,
+  unsigned MinorVersion, bool Hashed,
   StringRef UserInfo)
-: ModuleFileExtension(ModuleFileExtensionKind::MFEK_Test),
-  BlockName(BlockName),
-  MajorVersion(MajorVersion), MinorVersion(MinorVersion),
-  Hashed(Hashed), UserInfo(UserInfo) { }
+  : BlockName(BlockName), MajorVersion(MajorVersion),
+MinorVersion(MinorVersion), Hashed(Hashed), UserInfo(UserInfo) {}
   ~TestModuleFileExtension() override;
 
   ModuleFileExtensionMetadata getExtensionMetadata() const override;
@@ -66,

[PATCH] D98023: [clang] Don't make the g++ driver imply an explicitly shared libunwind

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: mati865, rnk, saugustine, phosek.
mstorsjo requested review of this revision.
Herald added a project: clang.

The logic in getLibGccType, to have the g++ driver mode imply a shared libgcc, 
originally was to match GCC driver behaviour for libgcc.

For cases when using -rtlib=compiler-rt or -unwindlib=libunwind, there's no 
distinction between SharedLibGcc and UnspecifiedLibGcc for platforms other than 
MinGW.

For MinGW targets, we distinguish between an explicitly shared unwinder library 
(requested via -shared-libgcc), an explicitly static one (requested via 
-static-libgcc or -static) and the default case (which just passes -lunwind to 
the linker, which will pick either shared or static depending on what's 
available, with the normal linker logic).

This makes the implicit default case (as added in D79995 
) actually work as it was intended, when using 
the g++ driver (which is the main usecase for libunwind as far as I know).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98023

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/compiler-rt-unwind.c


Index: clang/test/Driver/compiler-rt-unwind.c
===
--- clang/test/Driver/compiler-rt-unwind.c
+++ clang/test/Driver/compiler-rt-unwind.c
@@ -69,5 +69,9 @@
 // RUN: --target=x86_64-w64-mingw32 -rtlib=compiler-rt 
--unwindlib=libunwind \
 // RUN: --gcc-toolchain="" \
 // RUN:   | FileCheck 
--check-prefix=MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT %s
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-w64-mingw32 -rtlib=compiler-rt 
--unwindlib=libunwind \
+// RUN: --gcc-toolchain="" \
+// RUN:   | FileCheck 
--check-prefix=MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT %s
 // MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT: 
"{{.*}}libclang_rt.builtins-x86_64.a"
 // MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT: "{{.*}}lunwind"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1382,7 +1382,8 @@
   // The Android NDK only provides libunwind.a, not libunwind.so.
   if (TC.getTriple().isAndroid())
 return LibGccType::StaticLibGcc;
-  if (D.CCCIsCXX())
+  ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(Args);
+  if (D.CCCIsCXX() && RLT == ToolChain::RLT_Libgcc)
 return LibGccType::SharedLibGcc;
   return LibGccType::UnspecifiedLibGcc;
 }


Index: clang/test/Driver/compiler-rt-unwind.c
===
--- clang/test/Driver/compiler-rt-unwind.c
+++ clang/test/Driver/compiler-rt-unwind.c
@@ -69,5 +69,9 @@
 // RUN: --target=x86_64-w64-mingw32 -rtlib=compiler-rt --unwindlib=libunwind \
 // RUN: --gcc-toolchain="" \
 // RUN:   | FileCheck --check-prefix=MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT %s
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-w64-mingw32 -rtlib=compiler-rt --unwindlib=libunwind \
+// RUN: --gcc-toolchain="" \
+// RUN:   | FileCheck --check-prefix=MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT %s
 // MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT: "{{.*}}libclang_rt.builtins-x86_64.a"
 // MINGW-RTLIB-COMPILER-RT-UNWINDLIB-COMPILER-RT: "{{.*}}lunwind"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1382,7 +1382,8 @@
   // The Android NDK only provides libunwind.a, not libunwind.so.
   if (TC.getTriple().isAndroid())
 return LibGccType::StaticLibGcc;
-  if (D.CCCIsCXX())
+  ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(Args);
+  if (D.CCCIsCXX() && RLT == ToolChain::RLT_Libgcc)
 return LibGccType::SharedLibGcc;
   return LibGccType::UnspecifiedLibGcc;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

FYI, this might need rebasing on top of D97552 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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

[PATCH] D97462: [clang][cli] Round-trip cc1 arguments in assert builds

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 328460.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97462

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -465,7 +465,8 @@
 
 option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
 
-option(CLANG_ROUND_TRIP_CC1_ARGS "Round-trip command line arguments in -cc1." 
OFF)
+option(CLANG_ROUND_TRIP_CC1_ARGS
+  "Round-trip command line arguments in -cc1." ${LLVM_ENABLE_ASSERTIONS})
 
 if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
   message(FATAL_ERROR "Cannot disable static analyzer while enabling ARCMT or 
Z3")


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -465,7 +465,8 @@
 
 option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
 
-option(CLANG_ROUND_TRIP_CC1_ARGS "Round-trip command line arguments in -cc1." OFF)
+option(CLANG_ROUND_TRIP_CC1_ARGS
+  "Round-trip command line arguments in -cc1." ${LLVM_ENABLE_ASSERTIONS})
 
 if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
   message(FATAL_ERROR "Cannot disable static analyzer while enabling ARCMT or Z3")
___
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.

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@yubing I've reverted this as it was failing on a lot of buildbots: 
http://lab.llvm.org:8011/#/builders/109/builds/9867


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] D98029: [clangd] Introduce a CommandLineConfigProvider

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

This enables unifying command line flags with config options in clangd
internals. This patch changes behaviour in 2 places:

- BackgroundIndex was previously disabled when -remote-index was

provided. After this patch, it will be enabled but all files will have
bkgindex policy set to Skip.

- -index-file was loaded at startup (at least load was initiated), now

the load will happen through ProjectAwareIndex with first index query.

Unfortunately this doesn't simplify any options initially, as

- CompileCommandsDir is also used by clangd --check workflow, which

doesn't use configs.

- EnableBackgroundIndex option controls whether the component will be

created at all, which implies creation of extra threads registering a
listener for compilation database discoveries.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98029

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

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -8,6 +8,8 @@
 
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
+#include "Config.h"
+#include "ConfigProvider.h"
 #include "Features.inc"
 #include "PathMapping.h"
 #include "Protocol.h"
@@ -43,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #ifndef _WIN32
@@ -544,15 +547,55 @@
 log("Associating {0} with monolithic index at {1}.", External.MountPoint,
 External.Location);
 auto NewIndex = std::make_unique(std::make_unique());
-Tasks.runAsync("Load-index:" + External.Location,
-   [File = External.Location, PlaceHolder = NewIndex.get()] {
- if (auto Idx = loadIndex(File, /*UseDex=*/true))
-   PlaceHolder->reset(std::move(Idx));
-   });
+auto IndexLoadTask = [File = External.Location,
+  PlaceHolder = NewIndex.get()] {
+  if (auto Idx = loadIndex(File, /*UseDex=*/true))
+PlaceHolder->reset(std::move(Idx));
+};
+if (Sync) {
+  IndexLoadTask();
+} else {
+  Tasks.runAsync("Load-index:" + External.Location,
+ std::move(IndexLoadTask));
+}
 return std::move(NewIndex);
   }
   llvm_unreachable("Invalid ExternalIndexKind.");
 }
+
+class CommandLineFlagsConfigProvider : public config::Provider {
+  std::vector
+  getFragments(const config::Params &,
+   config::DiagnosticCallback) const override {
+config::CompiledFragment Frag = [](const config::Params &, Config &C) {
+  if (!CompileCommandsDir.empty()) {
+C.CompileFlags.CDBSearch = {Config::CDBSearchSpec::FixedDir,
+CompileCommandsDir.getValue()};
+  }
+  if (!IndexFile.empty()) {
+Config::ExternalIndexSpec Spec;
+Spec.Kind = Spec.File;
+Spec.Location = IndexFile;
+Spec.MountPoint = "";
+C.Index.External = std::move(Spec);
+  }
+  if (!RemoteIndexAddress.empty()) {
+assert(!ProjectRoot.empty() && IndexFile.empty());
+Config::ExternalIndexSpec Spec;
+Spec.Kind = Spec.Server;
+Spec.Location = RemoteIndexAddress;
+Spec.MountPoint = ProjectRoot;
+C.Index.External = std::move(Spec);
+C.Index.Background = Config::BackgroundPolicy::Skip;
+  }
+  if (!EnableBackgroundIndex) {
+C.Index.Background = Config::BackgroundPolicy::Skip;
+  }
+  return true;
+};
+return {Frag};
+  }
+};
 } // namespace
 } // namespace clangd
 } // namespace clang
@@ -707,12 +750,17 @@
  "--compile-commands-dir to an absolute path: {0}. The argument "
  "will be ignored.",
  EC.message());
+CompileCommandsDir.clear();
   } else {
-Opts.CompileCommandsDir = std::string(Path.str());
+CompileCommandsDir = Path.str().str();
+// FIXME: Still set for clangd --check. Use config in --check workflow
+// and get rid of these options?
+Opts.CompileCommandsDir = CompileCommandsDir;
   }
 } else {
   elog("Path specified by --compile-commands-dir does not exist. The "
"argument will be ignored.");
+  CompileCommandsDir.clear();
 }
   }
 
@@ -729,18 +777,6 @@
   Opts.BuildDynamicSymbolIndex = true;
   std::vector> IdxStack;
   std::unique_ptr StaticIdx;
-  std::future AsyncIndexLoad; // Block exit while loading the index.
-  if (!IndexFile.empty()) {
-// Load the index asynchronously. Meanwhile SwapIndex returns no results.
-SwapInd

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D89909#2600859 , @aaron.ballman 
wrote:

> Just a few minor nits from me, but I'm mostly wondering: where are we at with 
> this and are there still substantive changes required? (I looked through the 
> comments, but there's a lot of back-and-forth since Oct and I'm not certain 
> what's holding the patch back currently.)

To make it short, from my side I am not very clear about the overall design. 
From the SYCL spec side, there is no indication of what compiler extensions are 
needed and if at all. As a result, some of the design choices are unclear to me 
- in particular why SPIR target would need a separate address space map for 
SYCL. This is not how it was intended originally and I am worried that this 
will create issues for the consumers of IR to handle two different formats. But 
in general, if the community is now to maintain this code we should at least 
have some deeper understanding of it.

I would suggest starting from some high-level documentation that provides the 
details of the compiler extension being implemented. Perhaps the documentation 
that @bader has linked earlier could be used as a starting point with some more 
details that would allow assessing and reviewing the changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89909

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

[PATCH] D98030: [IR] Add vscale_range IR function attribute

[Bradley Smith via Phabricator via cfe-commits]

bsmith created this revision.
bsmith added reviewers: paulwalker-arm, joechrisellis, peterwaller-arm.
Herald added subscribers: dexonsmith, jdoerfert, steven_wu, hiraditya.
bsmith requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This attribute represents the minimum and maximum values vscale can
take. For now this attribute is not hooked up to anything during
codegen, this will be added in the future when such codegen is
considered stable.

Additionally hook up the -msve-vector-bits= clang option to emit this
attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98030

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/AttributeImpl.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- /dev/null
+++ llvm/test/Verifier/vscale_range.ll
@@ -0,0 +1,7 @@
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+; CHECK-NOT: 'vscale_range' minimum cannot be greater than maximum
+declare i8* @a(i32) vscale_range(1, 0)
+
+; CHECK: 'vscale_range' minimum cannot be greater than maximum
+declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -422,6 +422,18 @@
   ret void
 }
 
+; CHECK: define void @f72() #45
+define void @f72() vscale_range(8)
+{
+  ret void
+}
+
+; CHECK: define void @f73() #46
+define void @f73() vscale_range(1,8)
+{
+  ret void
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
@@ -467,4 +479,6 @@
 ; CHECK: attributes #42 = { nocallback }
 ; CHECK: attributes #43 = { cold }
 ; CHECK: attributes #44 = { hot }
+; CHECK: attributes #45 = { vscale_range(8,8) }
+; CHECK: attributes #46 = { vscale_range(1,8) }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -970,6 +970,7 @@
   case Attribute::StackProtectStrong:
   case Attribute::StrictFP:
   case Attribute::UWTable:
+  case Attribute::VScaleRange:
   case Attribute::NoCfCheck:
   case Attribute::MustProgress:
   case Attribute::NoProfile:
Index: llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
===
--- llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
@@ -73,6 +73,7 @@
   .Case("sspstrong", Attribute::StackProtectStrong)
   .Case("strictfp", Attribute::StrictFP)
   .Case("uwtable", Attribute::UWTable)
+  .Case("vscale_range", Attribute::VScaleRange)
   .Default(Attribute::None);
 }
 
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1622,6 +1622,7 @@
   case Attribute::InlineHint:
   case Attribute::StackAlignment:
   case Attribute::UWTable:
+  case Attribute::VScaleRange:
   case Attribute::NonLazyBind:
   case Attribute::ReturnsTwice:
   case Attribute::SanitizeAddress:
@@ -1980,6 +1981,14 @@
   return;
   }
 
+  if (Attrs.hasFnAttribute(Attribute::VScaleRange)) {
+std::pair Args =
+Attrs.getVScaleRangeArgs(AttributeList::FunctionIndex);
+
+if (Args.first > Args.second && Args.second != 0)
+  CheckFailed("'vscale_range' minimum cannot be greater than maximum", V);
+  }
+
   if (Attrs.hasFnAttribute("frame-pointer")) {
 StringRef FP = Attrs.getAttribute(AttributeList::FunctionIndex,
   "frame-pointer").getValueAsString();
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -78,6 +78,17 @@
   return std::make_pair(ElemSizeArg, NumElemsArg);
 }
 
+static uint64_t packVScaleRangeArgs(unsigned MinValue, unsigned MaxValue) {
+  return uint64_t(MinValue) << 32 | MaxValue;
+}
+
+static std::pair unpackVScaleRangeArgs(uint64_t Value) {
+  unsi

[clang] 9b30251 - [AArch64] Add missing intrinsics for vrnd

[Jingu Kang via cfe-commits]

Author: Jingu Kang
Date: 2021-03-05T11:26:12Z
New Revision: 9b302513f6d82f0ca989b3bb1f5ffc592ed866b7

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

LOG: [AArch64] Add missing intrinsics for vrnd

Added: 
clang/test/CodeGen/aarch64-v8.5a-neon-frint3264-intrinsic.c
llvm/test/CodeGen/AArch64/v8.5a-neon-frint3264-intrinsic.ll

Modified: 
clang/include/clang/Basic/arm_neon.td
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/Preprocessor/aarch64-target-features.c
llvm/include/llvm/IR/IntrinsicsAArch64.td
llvm/lib/Target/AArch64/AArch64InstrInfo.td

Removed: 




diff  --git a/clang/include/clang/Basic/arm_neon.td 
b/clang/include/clang/Basic/arm_neon.td
index 5360729db307..6e3ed6ebbd39 100644
--- a/clang/include/clang/Basic/arm_neon.td
+++ b/clang/include/clang/Basic/arm_neon.td
@@ -1210,6 +1210,13 @@ def FRINTZ_S64 : SInst<"vrnd", "..", "dQd">;
 def FRINTI_S64 : SInst<"vrndi", "..", "dQd">;
 }
 
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__aarch64__) && 
defined(__ARM_FEATURE_FRINT)" in {
+def FRINT32X_S32 : SInst<"vrnd32x", "..", "fQf">;
+def FRINT32Z_S32 : SInst<"vrnd32z", "..", "fQf">;
+def FRINT64X_S32 : SInst<"vrnd64x", "..", "fQf">;
+def FRINT64Z_S32 : SInst<"vrnd64z", "..", "fQf">;
+}
+
 

 // MaxNum/MinNum Floating Point
 

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index f17134623b8b..5b7981ea5256 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -182,6 +182,7 @@ void AArch64TargetInfo::getTargetDefinesARMV84A(const 
LangOptions &Opts,
 
 void AArch64TargetInfo::getTargetDefinesARMV85A(const LangOptions &Opts,
 MacroBuilder &Builder) const {
+  Builder.defineMacro("__ARM_FEATURE_FRINT", "1");
   // Also include the Armv8.4 defines
   getTargetDefinesARMV84A(Opts, Builder);
 }

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 35717168b2e0..eb5c430e4df0 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -5823,6 +5823,14 @@ static const ARMVectorIntrinsicInfo 
AArch64SIMDIntrinsicMap[] = {
   NEONMAP1(vrecpsq_v, aarch64_neon_frecps, Add1ArgType),
   NEONMAP2(vrhadd_v, aarch64_neon_urhadd, aarch64_neon_srhadd, Add1ArgType | 
UnsignedAlts),
   NEONMAP2(vrhaddq_v, aarch64_neon_urhadd, aarch64_neon_srhadd, Add1ArgType | 
UnsignedAlts),
+  NEONMAP1(vrnd32x_v, aarch64_neon_frint32x, Add1ArgType),
+  NEONMAP1(vrnd32xq_v, aarch64_neon_frint32x, Add1ArgType),
+  NEONMAP1(vrnd32z_v, aarch64_neon_frint32z, Add1ArgType),
+  NEONMAP1(vrnd32zq_v, aarch64_neon_frint32z, Add1ArgType),
+  NEONMAP1(vrnd64x_v, aarch64_neon_frint64x, Add1ArgType),
+  NEONMAP1(vrnd64xq_v, aarch64_neon_frint64x, Add1ArgType),
+  NEONMAP1(vrnd64z_v, aarch64_neon_frint64z, Add1ArgType),
+  NEONMAP1(vrnd64zq_v, aarch64_neon_frint64z, Add1ArgType),
   NEONMAP0(vrndi_v),
   NEONMAP0(vrndiq_v),
   NEONMAP2(vrshl_v, aarch64_neon_urshl, aarch64_neon_srshl, Add1ArgType | 
UnsignedAlts),
@@ -10539,6 +10547,30 @@ Value 
*CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID,
   : Intrinsic::trunc;
 return EmitNeonCall(CGM.getIntrinsic(Int, HalfTy), Ops, "vrndz");
   }
+  case NEON::BI__builtin_neon_vrnd32x_v:
+  case NEON::BI__builtin_neon_vrnd32xq_v: {
+Ops.push_back(EmitScalarExpr(E->getArg(0)));
+Int = Intrinsic::aarch64_neon_frint32x;
+return EmitNeonCall(CGM.getIntrinsic(Int, Ty), Ops, "vrnd32x");
+  }
+  case NEON::BI__builtin_neon_vrnd32z_v:
+  case NEON::BI__builtin_neon_vrnd32zq_v: {
+Ops.push_back(EmitScalarExpr(E->getArg(0)));
+Int = Intrinsic::aarch64_neon_frint32z;
+return EmitNeonCall(CGM.getIntrinsic(Int, Ty), Ops, "vrnd32z");
+  }
+  case NEON::BI__builtin_neon_vrnd64x_v:
+  case NEON::BI__builtin_neon_vrnd64xq_v: {
+Ops.push_back(EmitScalarExpr(E->getArg(0)));
+Int = Intrinsic::aarch64_neon_frint64x;
+return EmitNeonCall(CGM.getIntrinsic(Int, Ty), Ops, "vrnd64x");
+  }
+  case NEON::BI__builtin_neon_vrnd64z_v:
+  case NEON::BI__builtin_neon_vrnd64zq_v: {
+Ops.push_back(EmitScalarExpr(E->getArg(0)));
+Int = Intrinsic::aarch64_neon_frint64z;
+return EmitNeonCall(CGM.getIntrinsic(Int, Ty), Ops, "vrnd64z");
+  }
   case NEON::BI__builtin_neon_vrnd_v:
   case NEON::BI__builtin_neon_vrndq_v: {
 Int = Builder.getIsFPConstrained()

diff  --git a/clang/test/CodeGen/aarch64-v8.5a-neon-frint3264-intrinsic.c 
b/clang/test/CodeGen/aarch64-v8.5a-neon-frint3264-intrinsic.c
new file mode 100644
index ..a0694ee29af2
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-v8.5a-neon-frint3264-intrinsic.c
@@ -0

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

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

In D93594#2606157 , @RKSimon wrote:

> @yubing I've reverted this as it was failing on a lot of buildbots: 
> http://lab.llvm.org:8011/#/builders/109/builds/9867

Thanks – I was just about to point out this broke downstream testing 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] D98032: [clang-format] Improve clang-format-diff.py error message

[David Spickett via Phabricator via cfe-commits]

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

Previously if we couldn't run the clang-format command
for some reason, you'd get an unhelpful error message:

  OSError: [Errno 2] No such file or directory

Which doesn't tell you what was happening to cause this.

Catch the error and add the command we were attempting to run:

  RuntimeError: Failed to run "<...>/clang-food <...>" - No such file or 
directory"
  RuntimeError: Failed to run "<...>/clang-format <...>" - Permission denied"


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98032

Files:
  clang/tools/clang-format/clang-format-diff.py


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -103,11 +103,19 @@
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command,
- stdout=subprocess.PIPE,
- stderr=None,
- stdin=subprocess.PIPE,
- universal_newlines=True)
+
+try:
+  p = subprocess.Popen(command,
+   stdout=subprocess.PIPE,
+   stderr=None,
+   stdin=subprocess.PIPE,
+   universal_newlines=True)
+except OSError as e:
+  # Give the user more context when clang-format isn't
+  # found/isn't executable, etc.
+  raise RuntimeError(
+'Failed to run "%s" - %s"' % (" ".join(command), e.strerror))
+
 stdout, stderr = p.communicate()
 if p.returncode != 0:
   sys.exit(p.returncode)


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -103,11 +103,19 @@
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command,
- stdout=subprocess.PIPE,
- stderr=None,
- stdin=subprocess.PIPE,
- universal_newlines=True)
+
+try:
+  p = subprocess.Popen(command,
+   stdout=subprocess.PIPE,
+   stderr=None,
+   stdin=subprocess.PIPE,
+   universal_newlines=True)
+except OSError as e:
+  # Give the user more context when clang-format isn't
+  # found/isn't executable, etc.
+  raise RuntimeError(
+'Failed to run "%s" - %s"' % (" ".join(command), e.strerror))
+
 stdout, stderr = p.communicate()
 if p.returncode != 0:
   sys.exit(p.returncode)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98012: [RFC][doc] Document that RISC-V's __fp16 has different behavior

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

> However we would like have slight different behavior for __fp16 other

than ACLE: The evaluation format of __fp16 set same as _Float16,
which means no promotion are performed if there is no hardware half-precision
supported.

Well, this is really problematic, because you're giving `__fp16` semantics, 
you're defining it to behave as `_Float16`.
Redefining an existing type, let it differentiate from other targets can never 
be a good thing.
I don't see how this is going to help anyone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98012

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

[PATCH] D98030: [IR] Add vscale_range IR function attribute

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Thanks for this patch, it's great to have an IR attribute for this. The 
mechanics of this patch look mostly fine to me.
I added a few more subscribers to give this some wider visibility.




Comment at: llvm/docs/LangRef.rst:1998
+function. A value of 0 means unbounded. If the optional max value is 
omitted
+then max is set to the value of min.
 

Do you need to say anything about a default if the attribute is not set, e.g. 
if `vscale_range` is not set, no assumptions are made about the range of vscale 
and the compiler falls back on the architectural maximum (if available).



Comment at: llvm/lib/IR/Attributes.cpp:564-565
+  if (hasAttribute(Attribute::VScaleRange)) {
+unsigned MinValue;
+unsigned MaxValue;
+std::tie(MinValue, MaxValue) = getVScaleRangeArgs();

nit: `unsigned MinValue, MaxValue;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98030

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

[PATCH] D98029: [clangd] Introduce a CommandLineConfigProvider

[Sam McCall via Phabricator via cfe-commits]

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

Both side-effects seem fine to me.

> EnableBackgroundIndex option controls whether the component will be created 
> at all

Do you think we should eventually switch to always setting 
ClangdServer::Options::BackgroundIndex to true in ClangdServerMain (or removing 
the option?)
If not, I'm not sure if there's much benefit in also setting the config flag...

> CompileCommandsDir is also used by clangd --check workflow, which doesn't use 
> configs

We can/should fix this though, right?




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555
+};
+if (Sync) {
+  IndexLoadTask();

(This seems a little weird, get passed in a task runner and maybe ignore it. 
Seems like the asyncprojectindex should know whether it's supposed to be sync 
or not, and either pass in a null runner or it should handle the asynchrony 
itself... not something for this patch)



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:566
+
+class CommandLineFlagsConfigProvider : public config::Provider {
+  std::vector

or just FlagsConfigProvider?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:570
+   config::DiagnosticCallback) const override {
+config::CompiledFragment Frag = [](const config::Params &, Config &C) {
+  if (!CompileCommandsDir.empty()) {

Hmm, it seems natural to move assembling some of these objects into the 
constructor, I'm not sure it matters much, at least for now, but putting the 
actual flag-inspection on the hot-ish path seems constraining.

(Lowest-ceremony I can come up with is just making them e.g. 
Optional members of the provider, and then having a single 
fragment which capture the provider `this` and keep the conditionals inside it)



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:756
+CompileCommandsDir = Path.str().str();
+// FIXME: Still set for clangd --check. Use config in --check workflow
+// and get rid of these options?

If we fix this, we can move the rest of the logic into the flags config 
provider constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98029

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

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

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

mboehme created this revision.
mboehme added a reviewer: sammccall.
mboehme added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We have no way to reason about the bool returned by try_emplace, so we
simply ignore any std::move()s that happen in a try_emplace argument.
A lot of the time in this situation, the code will be checking the
bool and doing something else if it turns out the value wasn't moved
into the map, and this has been causing false positives so far.

I don't currently have any intentions of handling "maybe move" functions
more generally.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98034

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -32,6 +32,31 @@
   bool expired() const;
 };
 
+template 
+struct pair {};
+
+template 
+struct map {
+  struct iterator {};
+
+  map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
+template 
+struct unordered_map {
+  struct iterator {};
+
+  unordered_map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
 #define DECLARE_STANDARD_CONTAINER(name) \
   template   \
   struct name {  \
@@ -55,11 +80,9 @@
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
 DECLARE_STANDARD_CONTAINER(set);
-DECLARE_STANDARD_CONTAINER(map);
 DECLARE_STANDARD_CONTAINER(multiset);
 DECLARE_STANDARD_CONTAINER(multimap);
 DECLARE_STANDARD_CONTAINER(unordered_set);
-DECLARE_STANDARD_CONTAINER(unordered_map);
 DECLARE_STANDARD_CONTAINER(unordered_multiset);
 DECLARE_STANDARD_CONTAINER(unordered_multimap);
 
@@ -483,6 +506,35 @@
   }
 };
 
+// Ignore moves that happen in a try_emplace.
+void try_emplace() {
+  {
+std::map amap;
+A a;
+amap.try_emplace(1, std::move(a));
+a.foo();
+  }
+  {
+std::unordered_map amap;
+A a;
+amap.try_emplace(1, std::move(a));
+a.foo();
+  }
+  {
+// Don't make any assumptions about methods called try_emplace that don't
+// belong to a standard container.
+struct map {
+  void try_emplace(int, A &&);
+};
+map mymap;
+A a;
+mymap.try_emplace(1, std::move(a));
+a.foo();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:11: note: move occurred here
+  }
+}
+
 
 // Tests involving control flow.
 
@@ -776,7 +828,7 @@
 container.empty();
   }
   {
-std::map container;
+std::map container;
 std::move(container);
 container.clear();
 container.empty();
@@ -800,7 +852,7 @@
 container.empty();
   }
   {
-std::unordered_map container;
+std::unordered_map container;
 std::move(container);
 container.clear();
 container.empty();
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -169,6 +169,10 @@
 The check will assume that the last line causes a move, even though, in this
 particular case, it does not. Again, this is intentional.
 
+There is one special case: A call to ``std::move`` inside a ``try_emplace`` call
+on a standard map type is ignored. This is to avoid spurious warnings, as the
+check has no way to reason about the ``bool`` returned by ``try_emplace``.
+
 When analyzing the order in which moves, uses and reinitializations happen (see
 section `Unsequenced moves, uses, and reinitializations`_), the move is assumed
 to occur in whichever function the result of the ``std::move`` is passed to.
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -393,12 +393,22 @@
 }
 
 void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
+  auto StandardMapMatcher =
+  hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+  cxxRecordDecl(hasAnyName("::std::map", "::std::unordered_map"));
+
   auto CallMoveMatcher =
   callExp

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGValue.h:18
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/Sema.h"
 #include "clang/AST/Type.h"

rnk wrote:
> This includes Sema.h into every codegen file that uses CGValue.h (most of 
> them). That seems bad for build time. :(
> 
> This also seems like a layering violation. CodeGen has no dependency on Sema:
> https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CMakeLists.txt#L104
I agree that this is a layering violation (Sema relies on CodeGen which now 
relies on Sema due to this change). We just ran into it in a downstream fork 
when we had to add `clangSema` to the codegen linker input to avoid linking 
errors. I'm a bit surprised given that the only use appears to be a 
`static_assert` that shouldn't require anything to be linked in, but here we 
are just the same.

I think this should be rolled back so that we don't get additional dependencies 
on Sema within CodeGen by accident. It helps that @rnk moved this change into 
CGDecl.cpp (limits the scope of where we may introduce accidental 
dependencies), but I don't think we should be including anything from Sema.h 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73363

___
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.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks all for reporting and reverting this.


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] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

[Sam McCall via Phabricator via cfe-commits]

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

Nice! Only substantive suggestion: I think requiring the base type to be 
exactly a standard map type is too conservative.




Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:396
 void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
+  auto StandardMapMatcher =
+  hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(

I'd suggest dropping this condition.

Custom containers often emulate the standard library (e.g. llvm::DenseMap 
implements try_emplace, as do the absl maps and I found several other examples).

It seems very unlikely to encounter a try_emplace function without these 
semantics.
(The only case I can imagine is if for some reason they didn't bother to 
implement the bool return value, so there was no way to know whether 
emplacement happened. False negative there doesn't seem bad).



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:406
+   unless(inDecltypeOrTemplateArg()),
+   // Ignore std::move()s inside a try_emplace() call to avoid 
false
+   // positives as we can't reason about the bool returned by

Maybe explain why try_emplace is special?

try_emplace is a common maybe-moving function that returns a bool that tells 
callers whether it moved.
Ignore std::move inside try_emplace to avoid false positives, as we don't track 
uses of the bool.



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:409
+   // try_emplace.
+   unless(hasParent(cxxMemberCallExpr(
+   on(expr(declRefExpr(), StandardMapMatcher)),

i'm trying to think if there's an important case where the call isn't the 
immediate parent of the move and yet we're still maybe-moving... but I can't 
think of one. (Just mentioning in case one occurs to you)



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst:173
+There is one special case: A call to ``std::move`` inside a ``try_emplace`` 
call
+on a standard map type is ignored. This is to avoid spurious warnings, as the
+check has no way to reason about the ``bool`` returned by ``try_emplace``.

s/ignored/conservatively assumed *not* to move/?

(It wasn't obvious to me when reading how "ignored" in the implementation 
translates to the domain)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:510
+// Ignore moves that happen in a try_emplace.
+void try_emplace() {
+  {

calling this function exactly try_emplace seems gratuitiously confusing - since 
the condition we're changing here is "calls inside a function named 
try_emplace", just with a different definition of "inside"...



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:831
   {
-std::map container;
+std::map container;
 std::move(container);

hahaha :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98034

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

[PATCH] D98030: [IR] Add vscale_range IR function attribute

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.



Comment at: llvm/docs/LangRef.rst:1998
+function. A value of 0 means unbounded. If the optional max value is 
omitted
+then max is set to the value of min.
 

sdesmalen wrote:
> Do you need to say anything about a default if the attribute is not set, e.g. 
> if `vscale_range` is not set, no assumptions are made about the range of 
> vscale and the compiler falls back on the architectural maximum (if 
> available).
When the attribute is omitted there is no implicit knowledge and thus I'd stop 
at "... no assumptions are made about the range of vscale."

I would have though such a condition is implicit across most attributes (i.e. 
if omitted then no extra information is available) but I guess it cannot hurt 
to be explicit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98030

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

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan requested review of this revision.
abhina.sreeskantharajan added a comment.

This issue has been fixed in https://reviews.llvm.org/D97472.
@ASDenysPetrov I'm unable to close this revision and the other one 
https://reviews.llvm.org/D95808.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

___
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.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: 
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-gcc:1
+#!/usr/bin/env python
+

Python 3



Comment at: 
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-gcc:3-5
+print ("rv32iac/ilp32;@march=rv32iac@mabi=ilp32")
+print ("rv32imafc/ilp32f;@march=rv32iac@mabi=ilp32f")
+print ("rv64imafdc/lp64d;@march=rv64imafdc@mabi=lp64d")




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] D98037: [clangd] Add config block for Completion and option for AllScopes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Depends on D98029 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98037

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -10,10 +10,12 @@
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
 #include "Protocol.h"
+#include "llvm/ADT/None.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -164,6 +166,35 @@
   EXPECT_THAT(*Results[0].Index.External.getValue()->Server, Val("bar"));
 }
 
+TEST(ParseYAML, AllScopes) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Completion:
+  AllScopes: True
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_EQ(Results.size(), 1u);
+  EXPECT_THAT(Results[0].Completion.AllScopes, llvm::ValueIs(Val(true)));
+}
+
+TEST(ParseYAML, AllScopesWarn) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Completion:
+  AllScopes: $diagrange[[Truex]]
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(AllOf(DiagMessage("AllScopes should be a boolean"),
+DiagKind(llvm::SourceMgr::DK_Warning),
+DiagPos(YAML.range("diagrange").start),
+DiagRange(YAML.range("diagrange");
+  ASSERT_EQ(Results.size(), 1u);
+  EXPECT_THAT(Results[0].Completion.AllScopes, testing::Eq(llvm::None));
+}
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -443,6 +443,22 @@
   ASSERT_FALSE(Conf.Index.External);
 #endif
 }
+
+TEST_F(ConfigCompileTests, AllScopes) {
+  // Defaults to true.
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_TRUE(Conf.Completion.AllScopes);
+
+  Frag = {};
+  Frag.Completion.AllScopes = false;
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_FALSE(Conf.Completion.AllScopes);
+
+  Frag = {};
+  Frag.Completion.AllScopes = true;
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_TRUE(Conf.Completion.AllScopes);
+}
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -591,6 +591,8 @@
   if (!EnableBackgroundIndex) {
 C.Index.Background = Config::BackgroundPolicy::Skip;
   }
+  if (AllScopesCompletion.getNumOccurrences())
+C.Completion.AllScopes = AllScopesCompletion;
   return true;
 };
 return {Frag};
@@ -817,7 +819,6 @@
 Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
   }
   Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
-  Opts.CodeComplete.AllScopes = AllScopesCompletion;
   Opts.CodeComplete.RunParser = CodeCompletionParse;
   Opts.CodeComplete.RankingModel = RankingModel;
 
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -63,6 +63,7 @@
 Dict.handle("Index", [&](Node &N) { parse(F.Index, N); });
 Dict.handle("Style", [&](Node &N) { parse(F.Style, N); });
 Dict.handle("Diagnostics", [&](Node &N) { parse(F.Diagnostics, N); });
+Dict.handle("Completion", [&](Node &N) { parse(F.Completion, N); });
 Dict.parse(N);
 return !(N.failed() || HadError);
   }
@@ -165,6 +166,19 @@
 Dict.parse(N);
   }
 
+  void parse(Fragment::CompletionBlock &F, Node &N) {
+DictParser Dict("Completion", this);
+Dict.handle("AllScopes", [&](Node &N) {
+  if (auto V

[PATCH] D98032: [clang-format] Improve clang-format-diff.py error message

[Krasimir Georgiev via Phabricator via cfe-commits]

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

LGTM.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98032

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

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:364
   for (auto &I : Atomic64BitTypes)
 setOpenCLExtensionForType(I,
 "cl_khr_int64_base_atomics cl_khr_int64_extended_atomics");

azabaznov wrote:
> Anastasia wrote:
> > I think this should be lifted into the above if statement? Although since 
> > we are adding the types conditionally we could even drop this code 
> > completely and let the developers just use the types as they are already 
> > added and made available by the frontend.
> >  I think this should be lifted into the above if statement?
> 
> No, this is needed to be here as `atomic_double` added above.
> 
> >  Although since we are adding the types conditionally we could even drop 
> > this code completely and let the developers just use the types as they are 
> > already added and made available by the frontend.
> 
> I'm now worried about diagnostics  with that change:
> 
> ```
> tmp.cl:2:17: error: expected ';' after expression
> atomic_ulong a;
> ^
> ;
> tmp.cl:2:5: error: use of undeclared identifier 'atomic_ulong'
> atomic_ulong a;
> ^
> tmp.cl:2:18: error: use of undeclared identifier 'a'
> atomic_ulong a;
>  ^
> tmp.cl:3:5: error: unknown type name 'atomic_uintptr_t'; did you mean 
> 'atomic_uint'?
> atomic_uintptr_t u;
> ^~~~
> atomic_uint
> note: 'atomic_uint' declared here
> 4 errors generated.
> ```
> Is this expected behaviour when `cl_khr_int64_base_atomics` and 
> 'cl_khr_int64_extended_atomics' not supported? I think we need o be more 
> careful with reserved identifiers. However,  I didn't find explicit wording 
> about it, but I'm pretty sure that all of these atomic types identifiers 
> should be reserved. 
Well, the reserved types are those listed in `6.3.4. Reserved Data Types`.

These are typical examples of library types that are added in the standard 
headers outside of the compiler even though it was done in the clang source 
code in this case. Theoretically, they should have been added via an include 
file but in OpenCL it was supposed to be replaced by the pragma which has never 
happened for various reasons.

Anyway I think we should just either add the types and let them be used w/o any 
pragma magic or not add those and allow the identifiers to be reused in the 
application kernels. This is how atomic builtin functions behave now (e.g. 
`atomic_init`). The types should not be different.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058

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

[PATCH] D46443: Add missing cstdalign header

[Louis Dionne via Phabricator via cfe-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

In D46443#2566394 , 
@hubert.reinterpretcast wrote:

> The resolution of LWG 2828 
>  means that 
> `` exists in C++11 and C++14. The rationale from 
> https://reviews.llvm.org/D96786#2566110 can be taken as saying that adding 
> this header for C++11 conformance is reasonable.

Yes, I agree, let's add it. But the header needs to be added in 
`include/CMakeLists.txt`. There also needs to be a test added to check for the 
contents of the headers, and the auto-generated header tests should be 
modified. Take a look at what I did in https://reviews.llvm.org/D97870 for 
``.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46443

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

[PATCH] D97930: [OpenCL] Fix builtins that require multiple extensions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:21
+#if __OPENCL_C_VERSION__ <= CL_VERSION_1_2
+#pragma OPENCL EXTENSION cl_khr_3d_image_writes : enable
+#endif

You don't seem to be using `write_imagef` for OpenCL versions <= 1.2. But 
should we be checking that the diagnostic is given?



Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:184
+
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200
+  int4 i4;

Btw OpenCL 3.0 will be like OpenCL 1.2, although we could probably change this 
check later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97930

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10264a1b21ae: Introduce noundef attribute at call sites for 
stricter poison analysis (authored by guiand).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  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
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -enable-noundef-analysis -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = [[GLOBAL:(dso_local )?global]] i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = [[GLOBAL]] i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK: [[DEFINE:define( dso_local)?]] noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK: [[DEFINE]] i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE:define( dso_local)?]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE:define( dso_local)?]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added inline comments.



Comment at: clang/lib/CodeGen/CGValue.h:18
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/Sema.h"
 #include "clang/AST/Type.h"

aaron.ballman wrote:
> rnk wrote:
> > This includes Sema.h into every codegen file that uses CGValue.h (most of 
> > them). That seems bad for build time. :(
> > 
> > This also seems like a layering violation. CodeGen has no dependency on 
> > Sema:
> > https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CMakeLists.txt#L104
> I agree that this is a layering violation (Sema relies on CodeGen which now 
> relies on Sema due to this change). We just ran into it in a downstream fork 
> when we had to add `clangSema` to the codegen linker input to avoid linking 
> errors. I'm a bit surprised given that the only use appears to be a 
> `static_assert` that shouldn't require anything to be linked in, but here we 
> are just the same.
> 
> I think this should be rolled back so that we don't get additional 
> dependencies on Sema within CodeGen by accident. It helps that @rnk moved 
> this change into CGDecl.cpp (limits the scope of where we may introduce 
> accidental dependencies), but I don't think we should be including anything 
> from Sema.h here.
It was fixed over a year ago: 01943a59f51d8b5ede062305941c1f864b8a6a13


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73363

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

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGValue.h:18
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/Sema.h"
 #include "clang/AST/Type.h"

davezarzycki wrote:
> aaron.ballman wrote:
> > rnk wrote:
> > > This includes Sema.h into every codegen file that uses CGValue.h (most of 
> > > them). That seems bad for build time. :(
> > > 
> > > This also seems like a layering violation. CodeGen has no dependency on 
> > > Sema:
> > > https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CMakeLists.txt#L104
> > I agree that this is a layering violation (Sema relies on CodeGen which now 
> > relies on Sema due to this change). We just ran into it in a downstream 
> > fork when we had to add `clangSema` to the codegen linker input to avoid 
> > linking errors. I'm a bit surprised given that the only use appears to be a 
> > `static_assert` that shouldn't require anything to be linked in, but here 
> > we are just the same.
> > 
> > I think this should be rolled back so that we don't get additional 
> > dependencies on Sema within CodeGen by accident. It helps that @rnk moved 
> > this change into CGDecl.cpp (limits the scope of where we may introduce 
> > accidental dependencies), but I don't think we should be including anything 
> > from Sema.h here.
> It was fixed over a year ago: 01943a59f51d8b5ede062305941c1f864b8a6a13
That fixes the transitive include issues @rnk raised, but is not a fix for the 
layering violation. It's still a layering violation because it's including Sema 
from within CodeGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73363

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

[PATCH] D97869: [OpenCL][Draft] Add OpenCL builtin test generator

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D97869#2603249 , @azabaznov wrote:

> That's awesome!
>
> I'm thinking of how can you track correctness of generated built-ins 
> according to spec.  Perhaps you can compare the number of distinct 
> declarations for each  built-in in the spec and in tablegen and diagnose if 
> they are not equal. This is a very straightforward, but I can't think of on 
> other solution yet (with the absence of CTS coverage)...

The advantage of adding standard clang testing through the whole frontend 
invocation is that we can test all of the frontend features together to make 
sure that they indeed work as expected. For example there are types or macros 
added by clang and declarations from the `opencl-base-c.h` that interact with 
the headers declaring builtin functions so I think it would be desirable to use 
the standard clang testing strategy. There is also another advantage - it is 
more intuitive for the community to understand and maintain further.

But of course simple diffing should be fast I assume. We could look into this 
if we get push back on the traditional testing or we could consider it as a 
hybrid approach too.


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

https://reviews.llvm.org/D97869

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

[PATCH] D97869: [OpenCL][Draft] Add OpenCL builtin test generator

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D97869#2603068 , @svenvh wrote:

> In D97869#2602943 , @Anastasia wrote:
>
>> I was just thinking if we could combine the calls into one function to 
>> minimize the number of lines to parse? Perhaps this will make the Tablegen 
>> generator too complex?
>
> That will increase the emitter's complexity.  Especially when taking into 
> account the next point about handling optional functionality.  Hard to say in 
> advance if paying for the added emitter complexity is worth the parsing time 
> reduction, though the parsing time reduction is probably not going to be an 
> order of magnitude difference.

We could also consider generating the file with a simple emitter and then 
partitioning it manually before adding into the repo. But I think you are not 
suggesting checking it in?

Could you explain a bit more about the overall approach?


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

https://reviews.llvm.org/D97869

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

[clang] 9c0069d - [clang-format] Improve clang-format-diff.py error message

[David Spickett via cfe-commits]

Author: David Spickett
Date: 2021-03-05T13:28:51Z
New Revision: 9c0069d836b326f3ae0b92a8f095b0707a944ed0

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

LOG: [clang-format] Improve clang-format-diff.py error message

Previously if we couldn't run the clang-format command
for some reason, you'd get an unhelpful error message:
```
OSError: [Errno 2] No such file or directory
```

Which doesn't tell you what was happening to cause this.

Catch the error and add the command we were attempting to run:
```
RuntimeError: Failed to run "<...>/clang-food <...>" - No such file or 
directory"
RuntimeError: Failed to run "<...>/clang-format <...>" - Permission denied"
```

Reviewed By: krasimir

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

Added: 


Modified: 
clang/tools/clang-format/clang-format-diff.py

Removed: 




diff  --git a/clang/tools/clang-format/clang-format-
diff .py b/clang/tools/clang-format/clang-format-
diff .py
index 6e653a134289..efed8168cc50 100755
--- a/clang/tools/clang-format/clang-format-
diff .py
+++ b/clang/tools/clang-format/clang-format-
diff .py
@@ -103,11 +103,19 @@ def main():
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command,
- stdout=subprocess.PIPE,
- stderr=None,
- stdin=subprocess.PIPE,
- universal_newlines=True)
+
+try:
+  p = subprocess.Popen(command,
+   stdout=subprocess.PIPE,
+   stderr=None,
+   stdin=subprocess.PIPE,
+   universal_newlines=True)
+except OSError as e:
+  # Give the user more context when clang-format isn't
+  # found/isn't executable, etc.
+  raise RuntimeError(
+'Failed to run "%s" - %s"' % (" ".join(command), e.strerror))
+
 stdout, stderr = p.communicate()
 if p.returncode != 0:
   sys.exit(p.returncode)



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

[PATCH] D98032: [clang-format] Improve clang-format-diff.py error message

[David Spickett 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 rG9c0069d836b3: [clang-format] Improve clang-format-diff.py 
error message (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98032

Files:
  clang/tools/clang-format/clang-format-diff.py


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -103,11 +103,19 @@
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command,
- stdout=subprocess.PIPE,
- stderr=None,
- stdin=subprocess.PIPE,
- universal_newlines=True)
+
+try:
+  p = subprocess.Popen(command,
+   stdout=subprocess.PIPE,
+   stderr=None,
+   stdin=subprocess.PIPE,
+   universal_newlines=True)
+except OSError as e:
+  # Give the user more context when clang-format isn't
+  # found/isn't executable, etc.
+  raise RuntimeError(
+'Failed to run "%s" - %s"' % (" ".join(command), e.strerror))
+
 stdout, stderr = p.communicate()
 if p.returncode != 0:
   sys.exit(p.returncode)


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -103,11 +103,19 @@
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command,
- stdout=subprocess.PIPE,
- stderr=None,
- stdin=subprocess.PIPE,
- universal_newlines=True)
+
+try:
+  p = subprocess.Popen(command,
+   stdout=subprocess.PIPE,
+   stderr=None,
+   stdin=subprocess.PIPE,
+   universal_newlines=True)
+except OSError as e:
+  # Give the user more context when clang-format isn't
+  # found/isn't executable, etc.
+  raise RuntimeError(
+'Failed to run "%s" - %s"' % (" ".join(command), e.strerror))
+
 stdout, stderr = p.communicate()
 if p.returncode != 0:
   sys.exit(p.returncode)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added inline comments.



Comment at: clang/lib/CodeGen/CGValue.h:18
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/Sema.h"
 #include "clang/AST/Type.h"

aaron.ballman wrote:
> davezarzycki wrote:
> > aaron.ballman wrote:
> > > rnk wrote:
> > > > This includes Sema.h into every codegen file that uses CGValue.h (most 
> > > > of them). That seems bad for build time. :(
> > > > 
> > > > This also seems like a layering violation. CodeGen has no dependency on 
> > > > Sema:
> > > > https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CMakeLists.txt#L104
> > > I agree that this is a layering violation (Sema relies on CodeGen which 
> > > now relies on Sema due to this change). We just ran into it in a 
> > > downstream fork when we had to add `clangSema` to the codegen linker 
> > > input to avoid linking errors. I'm a bit surprised given that the only 
> > > use appears to be a `static_assert` that shouldn't require anything to be 
> > > linked in, but here we are just the same.
> > > 
> > > I think this should be rolled back so that we don't get additional 
> > > dependencies on Sema within CodeGen by accident. It helps that @rnk moved 
> > > this change into CGDecl.cpp (limits the scope of where we may introduce 
> > > accidental dependencies), but I don't think we should be including 
> > > anything from Sema.h here.
> > It was fixed over a year ago: 01943a59f51d8b5ede062305941c1f864b8a6a13
> That fixes the transitive include issues @rnk raised, but is not a fix for 
> the layering violation. It's still a layering violation because it's 
> including Sema from within CodeGen.
Ah, sorry. It does seem odd and probably a bug that a static_assert would 
trigger this. Did you verify that the imported symbol dependency is triggered 
by this static_assert? Or is the problem the mere inclusion, not the 
static_assert itself? And is the actual dependency only happen in 
unoptimized/debug builds or release too?

In any case, I believe it was discussed at the time that the LLVM variable 
ought to be moved to some kind of policy/config header that both LLVM's CodeGen 
and clang's Sema can include. Would you be open to that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73363

___
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

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added inline comments.



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

amccarth wrote:
> 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.
Right, my intention was not to change the current behaviour, but only limit it 
to Windows which is the only affected platform.


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] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan updated this revision to Diff 328483.
abhina.sreeskantharajan added reviewers: amccarth, MaskRay, jhenderson.
abhina.sreeskantharajan added a comment.

Fix formatting from lint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -803,7 +803,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -825,7 +827,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr =
+MemoryBuffer::getFile(OutputFilename, -1, true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(Filename, EC,
+   HostTriple.isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -816,13 +816,7 @@
 
 std::error_code createUniqueFile(const Twine &Model, int &ResultFd,
  SmallVectorImpl &ResultPath,
- 

[PATCH] D98029: [clangd] Introduce a CommandLineConfigProvider

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 328485.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98029

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/log.test
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -8,6 +8,8 @@
 
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
+#include "Config.h"
+#include "ConfigProvider.h"
 #include "Features.inc"
 #include "PathMapping.h"
 #include "Protocol.h"
@@ -43,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #ifndef _WIN32
@@ -544,15 +547,91 @@
 log("Associating {0} with monolithic index at {1}.", External.MountPoint,
 External.Location);
 auto NewIndex = std::make_unique(std::make_unique());
-Tasks.runAsync("Load-index:" + External.Location,
-   [File = External.Location, PlaceHolder = NewIndex.get()] {
- if (auto Idx = loadIndex(File, /*UseDex=*/true))
-   PlaceHolder->reset(std::move(Idx));
-   });
+auto IndexLoadTask = [File = External.Location,
+  PlaceHolder = NewIndex.get()] {
+  if (auto Idx = loadIndex(File, /*UseDex=*/true))
+PlaceHolder->reset(std::move(Idx));
+};
+// FIXME: The signature should contain a null-able TaskRunner istead, and
+// the task should be scheduled accordingly.
+if (Sync) {
+  IndexLoadTask();
+} else {
+  Tasks.runAsync("Load-index:" + External.Location,
+ std::move(IndexLoadTask));
+}
 return std::move(NewIndex);
   }
   llvm_unreachable("Invalid ExternalIndexKind.");
 }
+
+class FlagsConfigProvider : public config::Provider {
+private:
+  config::CompiledFragment Frag;
+
+  std::vector
+  getFragments(const config::Params &,
+   config::DiagnosticCallback) const override {
+return {Frag};
+  }
+
+public:
+  FlagsConfigProvider() {
+llvm::Optional CDBSearch;
+llvm::Optional IndexSpec;
+llvm::Optional BGPolicy;
+
+// If --compile-commands-dir arg was invoked, check value and override
+// default path.
+if (!CompileCommandsDir.empty()) {
+  if (llvm::sys::fs::exists(CompileCommandsDir)) {
+// We support passing both relative and absolute paths to the
+// --compile-commands-dir argument, but we assume the path is absolute
+// in the rest of clangd so we make sure the path is absolute before
+// continuing.
+llvm::SmallString<128> Path(CompileCommandsDir);
+if (std::error_code EC = llvm::sys::fs::make_absolute(Path)) {
+  elog("Error while converting the relative path specified by "
+   "--compile-commands-dir to an absolute path: {0}. The argument "
+   "will be ignored.",
+   EC.message());
+} else {
+  CDBSearch = {Config::CDBSearchSpec::FixedDir, Path.str().str()};
+}
+  } else {
+elog("Path specified by --compile-commands-dir does not exist. The "
+ "argument will be ignored.");
+  }
+}
+if (!IndexFile.empty()) {
+  Config::ExternalIndexSpec Spec;
+  Spec.Kind = Spec.File;
+  Spec.Location = IndexFile;
+  IndexSpec = std::move(Spec);
+}
+if (!RemoteIndexAddress.empty()) {
+  assert(!ProjectRoot.empty() && IndexFile.empty());
+  Config::ExternalIndexSpec Spec;
+  Spec.Kind = Spec.Server;
+  Spec.Location = RemoteIndexAddress;
+  Spec.MountPoint = ProjectRoot;
+  IndexSpec = std::move(Spec);
+  BGPolicy = Config::BackgroundPolicy::Skip;
+}
+if (!EnableBackgroundIndex) {
+  BGPolicy = Config::BackgroundPolicy::Skip;
+}
+Frag = [=](const config::Params &, Config &C) {
+  if (CDBSearch)
+C.CompileFlags.CDBSearch = *CDBSearch;
+  if (IndexSpec)
+C.Index.External = *IndexSpec;
+  if (BGPolicy)
+C.Index.Background = *BGPolicy;
+  return true;
+};
+  }
+};
 } // namespace
 } // namespace clangd
 } // namespace clang
@@ -693,29 +772,6 @@
   ClangdLSPServer::Options Opts;
   Opts.UseDirBasedCDB = (CompileArgsFrom == FilesystemCompileArgs);
 
-  // If --compile-commands-dir arg was invoked, check value and override default
-  // path.
-  if (!CompileCommandsDir.empty()) {
-if (llvm::sys::fs::exists(CompileCommandsDir)) {
-  // We support passing both relative and absolute paths to the
-  // --compile-commands-dir argument, but we assume the path is absolute in
-  // the rest of clangd so we make sure 

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

[Jinsong Ji via Phabricator via cfe-commits]

jsji accepted this revision as: jsji.
jsji 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/D98010/new/

https://reviews.llvm.org/D98010

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

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGValue.h:18
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/Sema.h"
 #include "clang/AST/Type.h"

davezarzycki wrote:
> aaron.ballman wrote:
> > davezarzycki wrote:
> > > aaron.ballman wrote:
> > > > rnk wrote:
> > > > > This includes Sema.h into every codegen file that uses CGValue.h 
> > > > > (most of them). That seems bad for build time. :(
> > > > > 
> > > > > This also seems like a layering violation. CodeGen has no dependency 
> > > > > on Sema:
> > > > > https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CMakeLists.txt#L104
> > > > I agree that this is a layering violation (Sema relies on CodeGen which 
> > > > now relies on Sema due to this change). We just ran into it in a 
> > > > downstream fork when we had to add `clangSema` to the codegen linker 
> > > > input to avoid linking errors. I'm a bit surprised given that the only 
> > > > use appears to be a `static_assert` that shouldn't require anything to 
> > > > be linked in, but here we are just the same.
> > > > 
> > > > I think this should be rolled back so that we don't get additional 
> > > > dependencies on Sema within CodeGen by accident. It helps that @rnk 
> > > > moved this change into CGDecl.cpp (limits the scope of where we may 
> > > > introduce accidental dependencies), but I don't think we should be 
> > > > including anything from Sema.h here.
> > > It was fixed over a year ago: 01943a59f51d8b5ede062305941c1f864b8a6a13
> > That fixes the transitive include issues @rnk raised, but is not a fix for 
> > the layering violation. It's still a layering violation because it's 
> > including Sema from within CodeGen.
> Ah, sorry. It does seem odd and probably a bug that a static_assert would 
> trigger this. Did you verify that the imported symbol dependency is triggered 
> by this static_assert? Or is the problem the mere inclusion, not the 
> static_assert itself? And is the actual dependency only happen in 
> unoptimized/debug builds or release too?
> 
> In any case, I believe it was discussed at the time that the LLVM variable 
> ought to be moved to some kind of policy/config header that both LLVM's 
> CodeGen and clang's Sema can include. Would you be open to that?
> Ah, sorry. It does seem odd and probably a bug that a static_assert would 
> trigger this. Did you verify that the imported symbol dependency is triggered 
> by this static_assert? Or is the problem the mere inclusion, not the 
> static_assert itself? And is the actual dependency only happen in 
> unoptimized/debug builds or release too?

I'd have to go back to the developer who found it to get the exact details, but 
my belief is that it's the inclusion more so than the static assertion itself. 
FWIW, the dependency was happening when doing a shared libraries build (I 
believe in release mode). I can dig up those details if you'd like, but my 
concern is that even if the issue is the static assertion, including the header 
file makes it easy for someone to accidentally use other facilities from Sema.h 
under the mistaken belief they're fine to do so.

> In any case, I believe it was discussed at the time that the LLVM variable 
> ought to be moved to some kind of policy/config header that both LLVM's 
> CodeGen and clang's Sema can include. Would you be open to that?

That sounds like a very good idea to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73363

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

[clang] f068656 - [OpenCL] Fix `mix` builtin overloads

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2021-03-05T13:43:30Z
New Revision: f0686569cc5789f93d76461143c232882b7ff2e9

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

LOG: [OpenCL] Fix `mix` builtin overloads

`mix` is subtly different from `clamp`: in the overloads where the
last argument is a scalar, the second argument should be a gentype for
`mix`.

As scalars can be implicitly converted to vectors, this cannot be
caught in the Sema test.  Hence adding a CodeGen test, where we can
verify the types using the mangled name.

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td
clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index f2a9b98196e8..05dd6a1bd8f8 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -651,12 +651,18 @@ foreach name = ["step"] in {
 }
 
 // --- 3 arguments ---
-foreach name = ["clamp", "mix"] in {
+foreach name = ["clamp"] in {
   def : Builtin;
   def : Builtin;
   def : Builtin;
   def : Builtin;
 }
+foreach name = ["mix"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
 foreach name = ["smoothstep"] in {
   def : Builtin;
   def : Builtin;

diff  --git a/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl 
b/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
index e4f4db0cd174..a59357e331e7 100644
--- a/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
+++ b/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -1,5 +1,13 @@
 // RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown 
-cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header %s | FileCheck 
%s
 
+// Test that mix is correctly defined.
+// CHECK-LABEL: @test_float
+// CHECK: call <4 x float> @_Z3mixDv4_fS_f
+// CHECK: ret
+void test_float(float4 x, float a) {
+  float4 ret = mix(x, x, a);
+}
+
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone 
attribute.
 // CHECK-LABEL: @test_const_attr
 // CHECK: call i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]



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

[clang] 258ecf5 - [HIP] do not use -mconstructor-aliases for device

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-03-05T08:46:58-05:00
New Revision: 258ecf5f33e7b0b43bc1c7252cd84eea596c39d2

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

LOG: [HIP] do not use -mconstructor-aliases for device

Like nvptx and some other targets, -mconstructor-aliases does not work well 
with amdgpu,
therefore we disable it in the same approach.

Reviewed by: Artem Belevich

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index eb09d722d304..8e50e8f7d139 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4925,9 +4925,11 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   }
 
   // Enable -mconstructor-aliases except on darwin, where we have to work 
around
-  // a linker bug (see ), and CUDA device code, where
-  // aliases aren't supported. Similarly, aliases aren't yet supported for AIX.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isOSAIX())
+  // a linker bug (see ), and CUDA/AMDGPU device code,
+  // where aliases aren't supported. Similarly, aliases aren't yet supported
+  // for AIX.
+  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() &&
+  !RawTriple.isAMDGPU() && !RawTriple.isOSAIX())
 CmdArgs.push_back("-mconstructor-aliases");
 
   // Darwin's kernel doesn't support guard variables; just die if we

diff  --git a/clang/test/Driver/hip-options.hip 
b/clang/test/Driver/hip-options.hip
index d66eb14ca9bf..e1050dfab1be 100644
--- a/clang/test/Driver/hip-options.hip
+++ b/clang/test/Driver/hip-options.hip
@@ -40,3 +40,10 @@
 // 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"
 // FIX-OVERLOAD: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-fgpu-exclude-wrong-side-overloads" "-fgpu-defer-diag"
+
+// Check -mconstructor-aliases is not passed to device compilation.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=CTA %s
+// CTA: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-mconstructor-aliases"
+// CTA-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mconstructor-aliases"



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

[clang] 5b3fc71 - [HIP] do not use -munsafe-fp-atomics by default

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-03-05T08:46:58-05:00
New Revision: 5b3fc7180c8e4a2c57946d5e3cc325744a770717

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

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

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

Reviewed by: Artem Belevich

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 8e50e8f7d139..28ac00e970c4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6437,7 +6437,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 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");
   }
 

diff  --git a/clang/test/Driver/hip-options.hip 
b/clang/test/Driver/hip-options.hip
index e1050dfab1be..99e9f0c24719 100644
--- a/clang/test/Driver/hip-options.hip
+++ b/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"



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

[PATCH] D97959: [HIP] do not use -mconstructor-aliases for device

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG258ecf5f33e7: [HIP] do not use -mconstructor-aliases for 
device (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97959

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
@@ -40,3 +40,10 @@
 // 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"
 // FIX-OVERLOAD: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-fgpu-exclude-wrong-side-overloads" "-fgpu-defer-diag"
+
+// Check -mconstructor-aliases is not passed to device compilation.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=CTA %s
+// CTA: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-mconstructor-aliases"
+// CTA-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mconstructor-aliases"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4925,9 +4925,11 @@
   }
 
   // Enable -mconstructor-aliases except on darwin, where we have to work 
around
-  // a linker bug (see ), and CUDA device code, where
-  // aliases aren't supported. Similarly, aliases aren't yet supported for AIX.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isOSAIX())
+  // a linker bug (see ), and CUDA/AMDGPU device code,
+  // where aliases aren't supported. Similarly, aliases aren't yet supported
+  // for AIX.
+  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() &&
+  !RawTriple.isAMDGPU() && !RawTriple.isOSAIX())
 CmdArgs.push_back("-mconstructor-aliases");
 
   // Darwin's kernel doesn't support guard variables; just die if we


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -40,3 +40,10 @@
 // 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"
 // FIX-OVERLOAD: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fgpu-exclude-wrong-side-overloads" "-fgpu-defer-diag"
+
+// Check -mconstructor-aliases is not passed to device compilation.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=CTA %s
+// CTA: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-mconstructor-aliases"
+// CTA-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mconstructor-aliases"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4925,9 +4925,11 @@
   }
 
   // Enable -mconstructor-aliases except on darwin, where we have to work around
-  // a linker bug (see ), and CUDA device code, where
-  // aliases aren't supported. Similarly, aliases aren't yet supported for AIX.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isOSAIX())
+  // a linker bug (see ), and CUDA/AMDGPU device code,
+  // where aliases aren't supported. Similarly, aliases aren't yet supported
+  // for AIX.
+  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() &&
+  !RawTriple.isAMDGPU() && !RawTriple.isOSAIX())
 CmdArgs.push_back("-mconstructor-aliases");
 
   // Darwin's kernel doesn't support guard variables; just die if we
___
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

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b3fc7180c8e: [HIP] do not use -munsafe-fp-atomics by 
default (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

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
@@ -6437,7 +6437,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
@@ -6437,7 +6437,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] D98037: [clangd] Add config block for Completion and option for AllScopes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 328492.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98037

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -10,10 +10,12 @@
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
 #include "Protocol.h"
+#include "llvm/ADT/None.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -164,6 +166,35 @@
   EXPECT_THAT(*Results[0].Index.External.getValue()->Server, Val("bar"));
 }
 
+TEST(ParseYAML, AllScopes) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Completion:
+  AllScopes: True
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_EQ(Results.size(), 1u);
+  EXPECT_THAT(Results[0].Completion.AllScopes, llvm::ValueIs(Val(true)));
+}
+
+TEST(ParseYAML, AllScopesWarn) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Completion:
+  AllScopes: $diagrange[[Truex]]
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(AllOf(DiagMessage("AllScopes should be a boolean"),
+DiagKind(llvm::SourceMgr::DK_Warning),
+DiagPos(YAML.range("diagrange").start),
+DiagRange(YAML.range("diagrange");
+  ASSERT_EQ(Results.size(), 1u);
+  EXPECT_THAT(Results[0].Completion.AllScopes, testing::Eq(llvm::None));
+}
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -443,6 +443,22 @@
   ASSERT_FALSE(Conf.Index.External);
 #endif
 }
+
+TEST_F(ConfigCompileTests, AllScopes) {
+  // Defaults to true.
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_TRUE(Conf.Completion.AllScopes);
+
+  Frag = {};
+  Frag.Completion.AllScopes = false;
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_FALSE(Conf.Completion.AllScopes);
+
+  Frag = {};
+  Frag.Completion.AllScopes = true;
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_TRUE(Conf.Completion.AllScopes);
+}
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -621,6 +621,7 @@
 if (!EnableBackgroundIndex) {
   BGPolicy = Config::BackgroundPolicy::Skip;
 }
+
 Frag = [=](const config::Params &, Config &C) {
   if (CDBSearch)
 C.CompileFlags.CDBSearch = *CDBSearch;
@@ -628,6 +629,8 @@
 C.Index.External = *IndexSpec;
   if (BGPolicy)
 C.Index.Background = *BGPolicy;
+  if (AllScopesCompletion.getNumOccurrences())
+C.Completion.AllScopes = AllScopesCompletion;
   return true;
 };
   }
@@ -825,7 +828,6 @@
 Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
   }
   Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
-  Opts.CodeComplete.AllScopes = AllScopesCompletion;
   Opts.CodeComplete.RunParser = CodeCompletionParse;
   Opts.CodeComplete.RankingModel = RankingModel;
 
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -63,6 +63,7 @@
 Dict.handle("Index", [&](Node &N) { parse(F.Index, N); });
 Dict.handle("Style", [&](Node &N) { parse(F.Style, N); });
 Dict.handle("Diagnostics", [&](Node &N) { parse(F.Diagnostics, N); });
+Dict.handle("Completion", [&](Node &N) { parse(F.Completion, N); });
 Dict.parse(N);
 return !(N.failed() || HadError);
   }
@@ -165,6 +166,19 @@
 Dict.parse(N);
   }
 
+  void parse(Fragment::CompletionBlock &F, Node &N) {
+DictParser Dict("Completion", this);
+

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

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

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

Responses to review comments.


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

https://reviews.llvm.org/D98034

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -32,6 +32,31 @@
   bool expired() const;
 };
 
+template 
+struct pair {};
+
+template 
+struct map {
+  struct iterator {};
+
+  map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
+template 
+struct unordered_map {
+  struct iterator {};
+
+  unordered_map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
 #define DECLARE_STANDARD_CONTAINER(name) \
   template   \
   struct name {  \
@@ -55,11 +80,9 @@
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
 DECLARE_STANDARD_CONTAINER(set);
-DECLARE_STANDARD_CONTAINER(map);
 DECLARE_STANDARD_CONTAINER(multiset);
 DECLARE_STANDARD_CONTAINER(multimap);
 DECLARE_STANDARD_CONTAINER(unordered_set);
-DECLARE_STANDARD_CONTAINER(unordered_map);
 DECLARE_STANDARD_CONTAINER(unordered_multiset);
 DECLARE_STANDARD_CONTAINER(unordered_multimap);
 
@@ -483,6 +506,22 @@
   }
 };
 
+// Ignore moves that happen in a try_emplace.
+void ignoreMoveInTryEmplace() {
+  {
+std::map amap;
+A a;
+amap.try_emplace(1, std::move(a));
+a.foo();
+  }
+  {
+std::unordered_map amap;
+A a;
+amap.try_emplace(1, std::move(a));
+a.foo();
+  }
+}
+
 
 // Tests involving control flow.
 
@@ -776,7 +815,7 @@
 container.empty();
   }
   {
-std::map container;
+std::map container;
 std::move(container);
 container.clear();
 container.empty();
@@ -800,7 +839,7 @@
 container.empty();
   }
   {
-std::unordered_map container;
+std::unordered_map container;
 std::move(container);
 container.clear();
 container.empty();
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -169,6 +169,10 @@
 The check will assume that the last line causes a move, even though, in this
 particular case, it does not. Again, this is intentional.
 
+There is one special case: A call to ``std::move`` inside a ``try_emplace`` call
+is conservatively assumed not to move. This is to avoid spurious warnings, as
+the check has no way to reason about the ``bool`` returned by ``try_emplace``.
+
 When analyzing the order in which moves, uses and reinitializations happen (see
 section `Unsequenced moves, uses, and reinitializations`_), the move is assumed
 to occur in whichever function the result of the ``std::move`` is passed to.
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -398,7 +398,13 @@
hasArgument(0, declRefExpr().bind("arg")),
anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
  hasAncestor(functionDecl().bind("containing-func"))),
-   unless(inDecltypeOrTemplateArg()))
+   unless(inDecltypeOrTemplateArg()),
+   // try_emplace is a common maybe-moving function that returns a
+   // bool to tell callers whether it moved. Ignore std::move inside
+   // try_emplace to avoid false positives as we don't track uses of
+   // the bool.
+   unless(hasParent(cxxMemberCallExpr(
+   callee(cxxMethodDecl(hasName("try_emplace")))
   .bind("call-move");
 
   Finder->addMatcher(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

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

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



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:396
 void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
+  auto StandardMapMatcher =
+  hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(

sammccall wrote:
> I'd suggest dropping this condition.
> 
> Custom containers often emulate the standard library (e.g. llvm::DenseMap 
> implements try_emplace, as do the absl maps and I found several other 
> examples).
> 
> It seems very unlikely to encounter a try_emplace function without these 
> semantics.
> (The only case I can imagine is if for some reason they didn't bother to 
> implement the bool return value, so there was no way to know whether 
> emplacement happened. False negative there doesn't seem bad).
Thanks -- I wasn't sure myself whether I should be so restrictive, and you make 
some very convincing points. Done!



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:406
+   unless(inDecltypeOrTemplateArg()),
+   // Ignore std::move()s inside a try_emplace() call to avoid 
false
+   // positives as we can't reason about the bool returned by

sammccall wrote:
> Maybe explain why try_emplace is special?
> 
> try_emplace is a common maybe-moving function that returns a bool that tells 
> callers whether it moved.
> Ignore std::move inside try_emplace to avoid false positives, as we don't 
> track uses of the bool.
I've stolen your wording -- thanks!



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:409
+   // try_emplace.
+   unless(hasParent(cxxMemberCallExpr(
+   on(expr(declRefExpr(), StandardMapMatcher)),

sammccall wrote:
> i'm trying to think if there's an important case where the call isn't the 
> immediate parent of the move and yet we're still maybe-moving... but I can't 
> think of one. (Just mentioning in case one occurs to you)
Yeah, I thought about that myself but couldn't come up with anything.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:831
   {
-std::map container;
+std::map container;
 std::move(container);

sammccall wrote:
> hahaha :-)
Yeah, I was being a bit sloppy with my definition of what a map is...


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

https://reviews.llvm.org/D98034

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

[PATCH] D98039: [OpenCL] Set calling convention for -fdeclare-opencl-builtins

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
svenvh added a project: clang.
Herald added a subscriber: yaxunl.
svenvh requested review of this revision.
Herald added a subscriber: cfe-commits.

IR produced using TableGen builtin function declarations
(`fdeclare-opencl-builtins.cl`) did not have the target's calling
convention applied to builtin calls.

Fix this, and update the codegen test to check that IR produced using
opencl-c.h and `-fdeclare-opencl-builtins` is identical with respect
to the builtin calls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98039

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl


Index: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -1,8 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown 
-cl-std=CL1.2 -finclude-default-header %s | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown 
-cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header %s | FileCheck 
%s
 
 // Test that mix is correctly defined.
 // CHECK-LABEL: @test_float
-// CHECK: call <4 x float> @_Z3mixDv4_fS_f
+// CHECK: call spir_func <4 x float> @_Z3mixDv4_fS_f
 // CHECK: ret
 void test_float(float4 x, float a) {
   float4 ret = mix(x, x, a);
@@ -10,7 +11,7 @@
 
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone 
attribute.
 // CHECK-LABEL: @test_const_attr
-// CHECK: call i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
+// CHECK: call spir_func i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
 // CHECK: ret
 int test_const_attr(int a) {
   return max(a, 2);
@@ -18,7 +19,7 @@
 
 // Test that Attr.Pure from OpenCLBuiltins.td is lowered to a readonly 
attribute.
 // CHECK-LABEL: @test_pure_attr
-// CHECK: call <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
+// CHECK: call spir_func <4 x float> @_Z11read_imagef{{.*}} 
[[ATTR_PURE:#[0-9]]]
 // CHECK: ret
 kernel void test_pure_attr(read_only image1d_t img) {
   float4 resf = read_imagef(img, 42);
@@ -26,7 +27,7 @@
 
 // Test that builtins with only one prototype are mangled.
 // CHECK-LABEL: @test_mangling
-// CHECK: call i32 @_Z12get_local_idj
+// CHECK: call spir_func i32 @_Z12get_local_idj
 kernel void test_mangling() {
   size_t lid = get_local_id(0);
 }
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -755,7 +755,8 @@
 ASTContext &Context, unsigned GenTypeMaxCnt,
 std::vector &FunctionList, SmallVector &RetTypes,
 SmallVector, 5> &ArgTypes) {
-  FunctionProtoType::ExtProtoInfo PI;
+  FunctionProtoType::ExtProtoInfo PI(
+  Context.getDefaultCallingConvention(false, false, true));
   PI.Variadic = false;
 
   // Create FunctionTypes for each (gen)type.


Index: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -1,8 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown -cl-std=CL1.2 -finclude-default-header %s | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown -cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header %s | FileCheck %s
 
 // Test that mix is correctly defined.
 // CHECK-LABEL: @test_float
-// CHECK: call <4 x float> @_Z3mixDv4_fS_f
+// CHECK: call spir_func <4 x float> @_Z3mixDv4_fS_f
 // CHECK: ret
 void test_float(float4 x, float a) {
   float4 ret = mix(x, x, a);
@@ -10,7 +11,7 @@
 
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone attribute.
 // CHECK-LABEL: @test_const_attr
-// CHECK: call i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
+// CHECK: call spir_func i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
 // CHECK: ret
 int test_const_attr(int a) {
   return max(a, 2);
@@ -18,7 +19,7 @@
 
 // Test that Attr.Pure from OpenCLBuiltins.td is lowered to a readonly attribute.
 // CHECK-LABEL: @test_pure_attr
-// CHECK: call <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
+// CHECK: call spir_func <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
 // CHECK: ret
 kernel void test_pure_attr(read_only image1d_t img) {
   float4 resf = read_imagef(img, 42);
@@ -26,7 +27,7 @@
 
 // Test that builtins with only one prototype are mangled.
 // CHECK-LABEL: @test_mangling
-// CHECK: call i32 @_Z12get_local_idj
+// CHECK: call spir_func i32 @_Z12get_local_idj
 kernel void test_mangling() {
   size_t lid = get_local_id(0);
 }
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/Se

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

[Nathan James via Phabricator via cfe-commits]

njames93 requested changes to this revision.
njames93 added a comment.
This revision now requires changes to proceed.

While `try_emplace` is a special case, it's may not be the only special case in 
someone's codebase, this should be behind extended with options to let users 
handle their special containers.




Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:406
+   // the bool.
+   unless(hasParent(cxxMemberCallExpr(
+   callee(cxxMethodDecl(hasName("try_emplace")))

Going with a custom list of bad names I'd suggest matching on function decls 
instead of method decls. 


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

https://reviews.llvm.org/D98034

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

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D98034#2606488 , @njames93 wrote:

> While `try_emplace` is a special case, it's may not be the only special case 
> in someone's codebase, this should be extended with options to let users 
> handle their special containers.




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

https://reviews.llvm.org/D98034

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

[clang-tools-extra] e67d91f - [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

[Martin Boehme via cfe-commits]

Author: Martin Boehme
Date: 2021-03-05T15:05:09+01:00
New Revision: e67d91faec2183789b220c15444929fb2efa6ecd

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

LOG: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

We have no way to reason about the bool returned by try_emplace, so we
simply ignore any std::move()s that happen in a try_emplace argument.
A lot of the time in this situation, the code will be checking the
bool and doing something else if it turns out the value wasn't moved
into the map, and this has been causing false positives so far.

I don't currently have any intentions of handling "maybe move" functions
more generally.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 028fefa9b99e..136d8f862b95 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -398,7 +398,13 @@ void UseAfterMoveCheck::registerMatchers(MatchFinder 
*Finder) {
hasArgument(0, declRefExpr().bind("arg")),
anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
  hasAncestor(functionDecl().bind("containing-func"))),
-   unless(inDecltypeOrTemplateArg()))
+   unless(inDecltypeOrTemplateArg()),
+   // try_emplace is a common maybe-moving function that returns a
+   // bool to tell callers whether it moved. Ignore std::move 
inside
+   // try_emplace to avoid false positives as we don't track uses 
of
+   // the bool.
+   unless(hasParent(cxxMemberCallExpr(
+   callee(cxxMethodDecl(hasName("try_emplace")))
   .bind("call-move");
 
   Finder->addMatcher(

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst 
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
index aab7cfd0ccd4..8509292eff99 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -169,6 +169,10 @@ that a move always takes place:
 The check will assume that the last line causes a move, even though, in this
 particular case, it does not. Again, this is intentional.
 
+There is one special case: A call to ``std::move`` inside a ``try_emplace`` 
call
+is conservatively assumed not to move. This is to avoid spurious warnings, as
+the check has no way to reason about the ``bool`` returned by ``try_emplace``.
+
 When analyzing the order in which moves, uses and reinitializations happen (see
 section `Unsequenced moves, uses, and reinitializations`_), the move is assumed
 to occur in whichever function the result of the ``std::move`` is passed to.

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
index 527c79840696..73ca59ccc91b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -32,6 +32,31 @@ struct weak_ptr {
   bool expired() const;
 };
 
+template 
+struct pair {};
+
+template 
+struct map {
+  struct iterator {};
+
+  map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
+template 
+struct unordered_map {
+  struct iterator {};
+
+  unordered_map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
 #define DECLARE_STANDARD_CONTAINER(name) \
   template   \
   struct name {  \
@@ -55,11 +80,9 @@ DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(deque);
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
 DECLARE_STANDARD_CONTAINER(set);
-DECLARE_STANDARD_CONTAINER(map);
 DECLARE_STANDARD_CONTAINER(multiset);
 DECLARE_STANDARD_CONTAINER(multimap);
 DECLARE_STANDARD_CONTAINER(unordered_set);
-DECLARE_STANDARD_CONTAINER(unordered_map);
 DECLARE_STANDARD_CONTAINER(unordered_multiset);
 DECLARE_STANDARD_CONTAINER(unordered_multimap);
 
@@ -483,6 +506,22 @@ class IgnoreMemberVariables {
   }
 };
 
+// Ignore moves that happen in a try_emplace.
+void ignoreMoveInTryEmplace() {
+  {
+std::map amap;
+   

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

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

This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mboehme marked 3 inline comments as done.
Closed by commit rGe67d91faec21: [clang-tidy] Use-after-move: Ignore moves 
inside a try_emplace. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98034

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -32,6 +32,31 @@
   bool expired() const;
 };
 
+template 
+struct pair {};
+
+template 
+struct map {
+  struct iterator {};
+
+  map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
+template 
+struct unordered_map {
+  struct iterator {};
+
+  unordered_map();
+  void clear();
+  bool empty();
+  template 
+  pair try_emplace(const Key &key, Args &&...args);
+};
+
 #define DECLARE_STANDARD_CONTAINER(name) \
   template   \
   struct name {  \
@@ -55,11 +80,9 @@
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
 DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
 DECLARE_STANDARD_CONTAINER(set);
-DECLARE_STANDARD_CONTAINER(map);
 DECLARE_STANDARD_CONTAINER(multiset);
 DECLARE_STANDARD_CONTAINER(multimap);
 DECLARE_STANDARD_CONTAINER(unordered_set);
-DECLARE_STANDARD_CONTAINER(unordered_map);
 DECLARE_STANDARD_CONTAINER(unordered_multiset);
 DECLARE_STANDARD_CONTAINER(unordered_multimap);
 
@@ -483,6 +506,22 @@
   }
 };
 
+// Ignore moves that happen in a try_emplace.
+void ignoreMoveInTryEmplace() {
+  {
+std::map amap;
+A a;
+amap.try_emplace(1, std::move(a));
+a.foo();
+  }
+  {
+std::unordered_map amap;
+A a;
+amap.try_emplace(1, std::move(a));
+a.foo();
+  }
+}
+
 
 // Tests involving control flow.
 
@@ -776,7 +815,7 @@
 container.empty();
   }
   {
-std::map container;
+std::map container;
 std::move(container);
 container.clear();
 container.empty();
@@ -800,7 +839,7 @@
 container.empty();
   }
   {
-std::unordered_map container;
+std::unordered_map container;
 std::move(container);
 container.clear();
 container.empty();
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -169,6 +169,10 @@
 The check will assume that the last line causes a move, even though, in this
 particular case, it does not. Again, this is intentional.
 
+There is one special case: A call to ``std::move`` inside a ``try_emplace`` call
+is conservatively assumed not to move. This is to avoid spurious warnings, as
+the check has no way to reason about the ``bool`` returned by ``try_emplace``.
+
 When analyzing the order in which moves, uses and reinitializations happen (see
 section `Unsequenced moves, uses, and reinitializations`_), the move is assumed
 to occur in whichever function the result of the ``std::move`` is passed to.
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -398,7 +398,13 @@
hasArgument(0, declRefExpr().bind("arg")),
anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
  hasAncestor(functionDecl().bind("containing-func"))),
-   unless(inDecltypeOrTemplateArg()))
+   unless(inDecltypeOrTemplateArg()),
+   // try_emplace is a common maybe-moving function that returns a
+   // bool to tell callers whether it moved. Ignore std::move inside
+   // try_emplace to avoid false positives as we don't track uses of
+   // the bool.
+   unless(hasParent(cxxMemberCallExpr(
+   callee(cxxMethodDecl(hasName("try_emplace")))
   .bind("call-move");
 
   Finder->addMatcher(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added inline comments.



Comment at: clang/lib/CodeGen/CGValue.h:18
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/Sema.h"
 #include "clang/AST/Type.h"

aaron.ballman wrote:
> davezarzycki wrote:
> > aaron.ballman wrote:
> > > davezarzycki wrote:
> > > > aaron.ballman wrote:
> > > > > rnk wrote:
> > > > > > This includes Sema.h into every codegen file that uses CGValue.h 
> > > > > > (most of them). That seems bad for build time. :(
> > > > > > 
> > > > > > This also seems like a layering violation. CodeGen has no 
> > > > > > dependency on Sema:
> > > > > > https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CMakeLists.txt#L104
> > > > > I agree that this is a layering violation (Sema relies on CodeGen 
> > > > > which now relies on Sema due to this change). We just ran into it in 
> > > > > a downstream fork when we had to add `clangSema` to the codegen 
> > > > > linker input to avoid linking errors. I'm a bit surprised given that 
> > > > > the only use appears to be a `static_assert` that shouldn't require 
> > > > > anything to be linked in, but here we are just the same.
> > > > > 
> > > > > I think this should be rolled back so that we don't get additional 
> > > > > dependencies on Sema within CodeGen by accident. It helps that @rnk 
> > > > > moved this change into CGDecl.cpp (limits the scope of where we may 
> > > > > introduce accidental dependencies), but I don't think we should be 
> > > > > including anything from Sema.h here.
> > > > It was fixed over a year ago: 01943a59f51d8b5ede062305941c1f864b8a6a13
> > > That fixes the transitive include issues @rnk raised, but is not a fix 
> > > for the layering violation. It's still a layering violation because it's 
> > > including Sema from within CodeGen.
> > Ah, sorry. It does seem odd and probably a bug that a static_assert would 
> > trigger this. Did you verify that the imported symbol dependency is 
> > triggered by this static_assert? Or is the problem the mere inclusion, not 
> > the static_assert itself? And is the actual dependency only happen in 
> > unoptimized/debug builds or release too?
> > 
> > In any case, I believe it was discussed at the time that the LLVM variable 
> > ought to be moved to some kind of policy/config header that both LLVM's 
> > CodeGen and clang's Sema can include. Would you be open to that?
> > Ah, sorry. It does seem odd and probably a bug that a static_assert would 
> > trigger this. Did you verify that the imported symbol dependency is 
> > triggered by this static_assert? Or is the problem the mere inclusion, not 
> > the static_assert itself? And is the actual dependency only happen in 
> > unoptimized/debug builds or release too?
> 
> I'd have to go back to the developer who found it to get the exact details, 
> but my belief is that it's the inclusion more so than the static assertion 
> itself. FWIW, the dependency was happening when doing a shared libraries 
> build (I believe in release mode). I can dig up those details if you'd like, 
> but my concern is that even if the issue is the static assertion, including 
> the header file makes it easy for someone to accidentally use other 
> facilities from Sema.h under the mistaken belief they're fine to do so.
> 
> > In any case, I believe it was discussed at the time that the LLVM variable 
> > ought to be moved to some kind of policy/config header that both LLVM's 
> > CodeGen and clang's Sema can include. Would you be open to that?
> 
> That sounds like a very good idea to me!
Okay. I sent out the RFC to llvm-dev. It was long overdue. Somebody just needed 
to volunteer, start the discussion, and get it over with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73363

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

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

[Ilya Kuteev via Phabricator via cfe-commits]

ilyakuteev added a comment.

@teemperor 's test shows the problem correctly.

In my case I am working on a dist-compilation system (similar to distcc) for 
objective-c with `-fmodules`. Our previous generation used tmpfs for module 
cache and was ephemeral (Unique temp module cache per compilation). Now we want 
to move module cache to ext4 filesystem to make it actually cache modules 
across compilations. This bug is blocking us from doing this change, so I came 
up with this draft fix.

I think that removing inode cache for PCMs may be some kind of final solution. 
(For example we can make another `FileManager.getNoncacedFileRef` method).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97850

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

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

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

mboehme added a comment.

In D98034#2606488 , @njames93 wrote:

> While `try_emplace` is a special case, it's may not be the only special case 
> in someone's codebase, this should be extended with options to let users 
> handle their special containers.

Sorry, I didn't see your comment before submitting.

I agree that there may be other cases of "maybe move" functions, but 
try_emplace seems to be the most common case. Handling "maybe move" functions 
more general would require some way of annotating them; I felt it was useful to 
get the try_emplace case handled first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98034

___
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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

This needs tests demonstrating the notes. The defacto way is to replace all 
`CHECK-MESSAGES`Directives with `CHECK-NOTES`. This'll for file check to look 
for lines containing `note`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97960

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

[clang] afa76fe - [XCOFF][DWARF] set default DWARF version to 3.

[Chen Zheng via cfe-commits]

Author: Chen Zheng
Date: 2021-03-05T09:21:57-05:00
New Revision: afa76fe67a42f6d40d83a4f7d0cb03373045f3f4

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

LOG: [XCOFF][DWARF] set default DWARF version to 3.

Reviewed By: jsji

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AIX.h 
b/clang/lib/Driver/ToolChains/AIX.h
index d4e593255736..c6aac09ddfac 100644
--- a/clang/lib/Driver/ToolChains/AIX.h
+++ b/clang/lib/Driver/ToolChains/AIX.h
@@ -74,6 +74,9 @@ class LLVM_LIBRARY_VISIBILITY AIX : public ToolChain {
 
   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;

diff  --git a/clang/test/CodeGen/dwarf-version.c 
b/clang/test/CodeGen/dwarf-version.c
index 87143ee13118..6d131c470d5b 100644
--- a/clang/test/CodeGen/dwarf-version.c
+++ b/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;
 }



___
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

[ChenZheng 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 rGafa76fe67a42: [XCOFF][DWARF] set default DWARF version to 3. 
(authored by shchenz).

Repository:
  rG LLVM Github Monorepo

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

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] D73363: Verify that clang's max alignment is <= LLVM's max alignment

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGValue.h:18
 #include "clang/AST/ASTContext.h"
+#include "clang/Sema/Sema.h"
 #include "clang/AST/Type.h"

davezarzycki wrote:
> aaron.ballman wrote:
> > davezarzycki wrote:
> > > aaron.ballman wrote:
> > > > davezarzycki wrote:
> > > > > aaron.ballman wrote:
> > > > > > rnk wrote:
> > > > > > > This includes Sema.h into every codegen file that uses CGValue.h 
> > > > > > > (most of them). That seems bad for build time. :(
> > > > > > > 
> > > > > > > This also seems like a layering violation. CodeGen has no 
> > > > > > > dependency on Sema:
> > > > > > > https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CMakeLists.txt#L104
> > > > > > I agree that this is a layering violation (Sema relies on CodeGen 
> > > > > > which now relies on Sema due to this change). We just ran into it 
> > > > > > in a downstream fork when we had to add `clangSema` to the codegen 
> > > > > > linker input to avoid linking errors. I'm a bit surprised given 
> > > > > > that the only use appears to be a `static_assert` that shouldn't 
> > > > > > require anything to be linked in, but here we are just the same.
> > > > > > 
> > > > > > I think this should be rolled back so that we don't get additional 
> > > > > > dependencies on Sema within CodeGen by accident. It helps that @rnk 
> > > > > > moved this change into CGDecl.cpp (limits the scope of where we may 
> > > > > > introduce accidental dependencies), but I don't think we should be 
> > > > > > including anything from Sema.h here.
> > > > > It was fixed over a year ago: 01943a59f51d8b5ede062305941c1f864b8a6a13
> > > > That fixes the transitive include issues @rnk raised, but is not a fix 
> > > > for the layering violation. It's still a layering violation because 
> > > > it's including Sema from within CodeGen.
> > > Ah, sorry. It does seem odd and probably a bug that a static_assert would 
> > > trigger this. Did you verify that the imported symbol dependency is 
> > > triggered by this static_assert? Or is the problem the mere inclusion, 
> > > not the static_assert itself? And is the actual dependency only happen in 
> > > unoptimized/debug builds or release too?
> > > 
> > > In any case, I believe it was discussed at the time that the LLVM 
> > > variable ought to be moved to some kind of policy/config header that both 
> > > LLVM's CodeGen and clang's Sema can include. Would you be open to that?
> > > Ah, sorry. It does seem odd and probably a bug that a static_assert would 
> > > trigger this. Did you verify that the imported symbol dependency is 
> > > triggered by this static_assert? Or is the problem the mere inclusion, 
> > > not the static_assert itself? And is the actual dependency only happen in 
> > > unoptimized/debug builds or release too?
> > 
> > I'd have to go back to the developer who found it to get the exact details, 
> > but my belief is that it's the inclusion more so than the static assertion 
> > itself. FWIW, the dependency was happening when doing a shared libraries 
> > build (I believe in release mode). I can dig up those details if you'd 
> > like, but my concern is that even if the issue is the static assertion, 
> > including the header file makes it easy for someone to accidentally use 
> > other facilities from Sema.h under the mistaken belief they're fine to do 
> > so.
> > 
> > > In any case, I believe it was discussed at the time that the LLVM 
> > > variable ought to be moved to some kind of policy/config header that both 
> > > LLVM's CodeGen and clang's Sema can include. Would you be open to that?
> > 
> > That sounds like a very good idea to me!
> Okay. I sent out the RFC to llvm-dev. It was long overdue. Somebody just 
> needed to volunteer, start the discussion, and get it over with.
Thank you for your help on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73363

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

[PATCH] D98034: [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D98034#2606535 , @mboehme wrote:

> In D98034#2606488 , @njames93 wrote:
>
>> While `try_emplace` is a special case, it's may not be the only special case 
>> in someone's codebase, this should be extended with options to let users 
>> handle their special containers.
>
> Sorry, I didn't see your comment before submitting.
>
> I agree that there may be other cases of "maybe move" functions, but 
> try_emplace seems to be the most common case. Handling "maybe move" functions 
> more general would require some way of annotating them; I felt it was useful 
> to get the try_emplace case handled first.

The canonical way of doing this in clang-tidy is to add an option to the check 
which takes a semi-colon delimited list.
`clang::tidy::utils::options::parseStringList` can then turn that into a vector 
of string.
Annoyingly the hasAnyName matcher needs a vector of StringRef's so most checks 
do something like this

  hasAnyName(SmallVector(
StringLikeClasses.begin(), StringLikeClasses.end())

As you are reading options, you will need to override the storeOptions method 
in the check to then store the value read from config, `serializeStringList` is 
your friend here.
The documentation will also need updating to describe the option and what it 
does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98034

___
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

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

balazske added a comment.

I tried to test the notes but could not get it to work. I think the problem is 
related to the fact that the notes are displayed at different locations that 
are past or before the current warning. The solution could be to manually 
specify line numbers in the CHECK-NOTE parts but I do not like this (and did 
not tried it). Or is there other solution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97960

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

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan updated this revision to Diff 328513.
abhina.sreeskantharajan marked an inline comment as done.
abhina.sreeskantharajan added a comment.

Add assertion, add testcase for multi-line raw string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/LiteralConverter.h
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Lex/CMakeLists.txt
  clang/lib/Lex/LiteralConverter.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/test/CodeGen/systemz-charset.c
  clang/test/CodeGen/systemz-charset.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Support/Triple.cpp

Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -1038,6 +1038,13 @@
   return Tmp.split('-').second;  // Strip second component
 }
 
+// System charset on z/OS is IBM-1047 and UTF-8 otherwise
+StringRef Triple::getSystemCharset() const {
+  if (getOS() == llvm::Triple::ZOS)
+return "IBM-1047";
+  return "UTF-8";
+}
+
 static unsigned EatNumber(StringRef &Str) {
   assert(!Str.empty() && isDigit(Str[0]) && "Not a number");
   unsigned Result = 0;
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -395,6 +395,9 @@
   /// if the environment component is present).
   StringRef getOSAndEnvironmentName() const;
 
+  /// getSystemCharset - Get the system charset of the triple.
+  StringRef getSystemCharset() const;
+
   /// @}
   /// @name Convenience Predicates
   /// @{
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -209,8 +209,14 @@
 // RUN: %clang -### -S -finput-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-CHARSET %s
 // CHECK-INVALID-CHARSET: error: invalid value 'iso-8859-1' in '-finput-charset=iso-8859-1'
 
-// RUN: %clang -### -S -fexec-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
-// CHECK-INVALID-INPUT-CHARSET: error: invalid value 'iso-8859-1' in '-fexec-charset=iso-8859-1'
+// RUN: %clang -### -S -fexec-charset=invalid-charset -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
+// CHECK-INVALID-INPUT-CHARSET: error: invalid value 'invalid-charset' in '-fexec-charset=invalid-charset'
+
+// Test that we support the following exec charsets.
+// RUN: %clang -### -S -fexec-charset=UTF-8 -o /dev/null %s 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: %clang -### -S -fexec-charset=ISO8859-1 -o /dev/null %s 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: %clang -### -S -fexec-charset=IBM-1047 -o /dev/null %s 2>&1 | FileCheck --check-prefix=INVALID %s
+// INVALID-NOT: error: invalid value
 
 // Test that we don't error on these.
 // RUN: %clang -### -S -Werror\
@@ -224,7 +230,7 @@
 // RUN: -fident -fno-ident\
 // RUN: -fimplicit-templates -fno-implicit-templates  \
 // RUN: -finput-charset=UTF-8 \
-// RUN: -fexec-charset=UTF-8 \
+// RUN: -fexec-charset=UTF-8  \
 // RUN: -fivopts -fno-ivopts  \
 // RUN: -fnon-call-exceptions -fno-non-call-exceptions\
 // RUN: -fpermissive -fno-permissive  \
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -209,10 +209,11 @@
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
 // source-charset-utf-16: invalid value 'utf-16' in '/source-charset:utf-16'
 
-// /execution-charset: should warn on everything except UTF-8.
-// RUN: %clang_cl /execution-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=execution-charset-utf-16 %s
-// execution-charset-utf-16: invalid value 'utf-16' in '/execution-charset:utf-16'
+// /execution-charset: should warn on invalid charsets.
+// RUN: %clang_cl /execution-charset:invalid-charset -### -- %s 2>&1 | FileCheck -check-prefix=execution-charset-invalid %s
+// execution-c

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1367
+Converter->convert(StringRef((char *)tmp_out_start), 
ConvertedChar);
+memmove((void *)tmp_out_start, ConvertedChar.data(), 1);
+  }

rsmith wrote:
> What assurance do we have that 1 output character is correct? I would expect 
> we need to reject with a diagnostic if the character doesn't fit in one 
> converted character.
Right, I'll add a similar assertion to the one we have above.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1700-1702
 // Point into the \n inside the \r\n sequence and operate on the
 // remaining portion of the literal.
 RemainingTokenSpan = AfterCRLF.substr(1);

rsmith wrote:
> Do we need to convert the newline character too?
> 
> Perhaps for raw string literals it'd be better to do the normal processing 
> here and then convert the entire string at once?
Yes, we need to convert newlines as well. I think the current behaviour is 
already converting multi line raw strings correctly. I'll add a testcase for 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

___
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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

The issue is FileCheck expects its directives to be in order they appear in the 
file being checked. Notes are always emitted just after the warning they are 
attached to.
So there are a few ways to go:

- use CHECK-NOTES-DAG to disregard the ordering and place them next to the 
location of the note in the code, to give correct line information.
- place the CHECK-NOTES directly after the warning they are attached.

Going with the first option makes it much harder to reason about.
Going with the second option you'll either have to just put a generic number 
regex for the line number, use (potentially large)  relative `@LINE` offsets or 
absolute line numbers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97960

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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 328522.
sbc100 added a comment.
Herald added a project: libc++.
Herald added a reviewer: libc++.

Rebase against mono-repo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

Files:
  libcxx/include/cstdalign
  libcxx/test/libcxx/min_max_macros.compile.pass.cpp
  libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp


Index: libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -13,6 +13,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: libcxx/test/libcxx/min_max_macros.compile.pass.cpp
===
--- libcxx/test/libcxx/min_max_macros.compile.pass.cpp
+++ libcxx/test/libcxx/min_max_macros.compile.pass.cpp
@@ -96,6 +96,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: libcxx/include/cstdalign
===
--- /dev/null
+++ libcxx/include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN


Index: libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -13,6 +13,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: libcxx/test/libcxx/min_max_macros.compile.pass.cpp
===
--- libcxx/test/libcxx/min_max_macros.compile.pass.cpp
+++ libcxx/test/libcxx/min_max_macros.compile.pass.cpp
@@ -96,6 +96,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: libcxx/include/cstdalign
===
--- /dev/null
+++ libcxx/include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 328524.
sbc100 added a comment.
Herald added a subscriber: mgorny.

feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

Files:
  libcxx/include/CMakeLists.txt
  libcxx/include/cstdalign
  libcxx/include/module.modulemap
  libcxx/test/libcxx/double_include.sh.cpp
  libcxx/test/libcxx/min_max_macros.compile.pass.cpp
  libcxx/test/libcxx/no_assert_include.compile.pass.cpp
  libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp

Index: libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -13,6 +13,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: libcxx/test/libcxx/no_assert_include.compile.pass.cpp
===
--- libcxx/test/libcxx/no_assert_include.compile.pass.cpp
+++ libcxx/test/libcxx/no_assert_include.compile.pass.cpp
@@ -63,6 +63,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/test/libcxx/min_max_macros.compile.pass.cpp
===
--- libcxx/test/libcxx/min_max_macros.compile.pass.cpp
+++ libcxx/test/libcxx/min_max_macros.compile.pass.cpp
@@ -96,6 +96,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: libcxx/test/libcxx/double_include.sh.cpp
===
--- libcxx/test/libcxx/double_include.sh.cpp
+++ libcxx/test/libcxx/double_include.sh.cpp
@@ -70,6 +70,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/include/module.modulemap
===
--- libcxx/include/module.modulemap
+++ libcxx/include/module.modulemap
@@ -161,7 +161,10 @@
   header "csignal"
   export *
 }
-// FIXME:  is missing.
+module cstdalign {
+  header "cstdlign"
+  export *
+}
 module cstdarg {
   header "cstdarg"
   export *
Index: libcxx/include/cstdalign
===
--- /dev/null
+++ libcxx/include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: libcxx/include/CMakeLists.txt
===
--- libcxx/include/CMakeLists.txt
+++ libcxx/include/CMakeLists.txt
@@ -55,6 +55,7 @@
   condition_variable
   csetjmp
   csignal
+  cstdalign
   cstdarg
   cstdbool
   cstddef
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97869: [OpenCL][Draft] Add OpenCL builtin test generator

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

To be honest, I don't have a concrete picture yet of how to deploy this emitter 
for testing.  So here are a few thoughts about the various options that come to 
mind:

1. Regenerate the test file every time we run check-clang and use it to 
validate the `-fdeclare-opencl-builtins` option.


Advantage:

- This gives confidence that each builtin described in `OpenCLBuiltins.td` is 
usable in OpenCL code.

Disadvantages:

- This doesn't protect against unintended removals of builtins/overloads, as 
the same source of truth is used for implementation and testing.
- It is quick to generate (<1 second), but it takes some time to parse the 
entire test with a debug build of Clang (35 seconds).



2. As 1, but also keep maintaining `opencl-c.h` and test with both `opencl-c.h` 
and `-fdeclare-opencl-builtins`.


This addresses the disadvantage of option 1 at the expense of maintaining 
`opencl-c.h` and increased testing time.

3. Generate the test file once, manually verify that it is correct, check it in 
to git, and maintain it from there.
---

Advantage:

- Different sources of truth for implementation (`OpenCLBuiltins.td`) and 
testing (the generated and maintained test).

Disadvantages:

- The output is big, currently already 56k lines, and I expect it will still 
get a bit bigger when we add conditionals.  I don't feel comfortable about 
putting such big autogenerated files under git (regardless of whether we add it 
as a single file, or as a collection of smaller files).  Ideally the repo 
should only contain the source from which we generate things.
- Updates to `OpenCLBuiltins.td` will give large diffs when regenerating the 
tests (regardless of whether the file is split or not).
- Manually verifying that the generated test is actually correct is not trivial.



4. Generate a condensed summary of all builtins, check that in to git.
--

Then regenerate the summary during testing and diff it against the checked in 
reference. This builds upon @azabaznov's suggestion.  We could for example use 
a count for each builtin and maintain that.  We could maintain multiple counts 
to cover different configurations (e.g. CL versions).
Advantages:

- There are only about 1000 different builtin function names, so this reduces 
the size of the reference data considerably, making it easier to verify the 
initial checkin and making maintainance easier.
- Generating the summary and diffing it against a reference is quick (<1 
second).
- It would catch the case when builtins are accidentally removed from a 
configuration: if the count in the summary is not updated, the test will fail.  
I believe this is an important scenario to test in light of the OpenCL 3 work?

Disadvantage:

- Not giving perfect coverage, so subtle errors might slip through, such as for 
example changing the argument type of a builtin.

Other ideas / feedback welcome!


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

https://reviews.llvm.org/D97869

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

[clang-tools-extra] 3bca861 - [clang-tidy][NFC] Remove unsupported language version checks from vector

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-03-05T15:35:34Z
New Revision: 3bca86170da0a7cadb4cc13b178ff1edf162ea8e

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

LOG: [clang-tidy][NFC] Remove unsupported language version checks from vector

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidy.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp 
b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index f65e8ed216f2..88ba4bf63e09 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -393,6 +393,10 @@ ClangTidyASTConsumerFactory::CreateASTConsumer(
   std::vector> Checks =
   CheckFactories->createChecks(&Context);
 
+  llvm::erase_if(Checks, [&](std::unique_ptr &Check) {
+return !Check->isLanguageVersionSupported(Context.getLangOpts());
+  });
+
   ast_matchers::MatchFinder::MatchFinderOptions FinderOptions;
 
   std::unique_ptr Profiling;
@@ -416,8 +420,6 @@ ClangTidyASTConsumerFactory::CreateASTConsumer(
   }
 
   for (auto &Check : Checks) {
-if (!Check->isLanguageVersionSupported(Context.getLangOpts()))
-  continue;
 Check->registerMatchers(&*Finder);
 Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }



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

[PATCH] D46443: Add missing cstdalign header

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: libcxx/include/cstdalign:19-22
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined

You should test that each of these macros is really defined when including 
`cstdalign`.



Comment at: libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp:16
 #include 
+#include 
 

This doesn't seem to be necessary. Or?
In C++, `alignof` is a keyword.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[clang] ed95129 - Fix Wdocumentation unknown parameter warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2021-03-05T15:58:20Z
New Revision: ed951293affca9abd92c886432115f35065cad3c

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

LOG: Fix Wdocumentation unknown parameter warning. NFCI.

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 6fcb472cd60cc..8f7cf57fb9112 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -5187,7 +5187,7 @@ static VarDecl *precomputeExpr(Sema &Actions,
 /// \param Rel   Comparison operator of the loop condition.
 /// \param StartExpr Value of the loop counter at the first iteration.
 /// \param StopExpr  Expression the loop counter is compared against in the 
loop
-/// condition. \param Step  Amount of increment after each iteration.
+/// condition. \param StepExpr  Amount of increment after each iteration.
 ///
 /// \return Closure (CapturedStmt) of the distance calculation.
 static CapturedStmt *buildDistanceFunc(Sema &Actions, QualType LogicalTy,



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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: libcxx/test/libcxx/utilities/any/size_and_alignment.pass.cpp:16
 #include 
+#include 
 

curdeius wrote:
> This doesn't seem to be necessary. Or?
> In C++, `alignof` is a keyword.
You are right its not necessary in C++ since its a keyword.   My memory of 
exactly why I added to this file in the original version of the CL back in 2018 
is ... missing.Maybe its to ensure that the inclusion of the C header and 
the macro don't interfere with C++ usage?

I'm happy to revert this line and find a better way to test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: libcxx/include/cstdalign:19-22
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined

curdeius wrote:
> You should test that each of these macros is really defined when including 
> `cstdalign`.
Done.  As you can see there is not much to tests since in C++ this is 
essentially an empty header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 328538.
sbc100 added a comment.

add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

Files:
  libcxx/include/CMakeLists.txt
  libcxx/include/cstdalign
  libcxx/include/module.modulemap
  libcxx/test/libcxx/double_include.sh.cpp
  libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
  libcxx/test/libcxx/min_max_macros.compile.pass.cpp
  libcxx/test/libcxx/no_assert_include.compile.pass.cpp
  libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp

Index: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp
@@ -0,0 +1,31 @@
+//===--===//
+//
+// 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
+//
+//===--===//
+
+// test 
+
+#include 
+
+int main(int, char**) {
+#ifdef alignas
+#error alignas should not be a macro in C++
+#endif
+
+#ifdef alignof
+#error alignof should not be a macro in C++
+#endif
+
+#ifndef __alignas_is_defined
+#error __alignas_is_defined not defined
+#endif
+
+#ifndef __alignof_is_defined
+#error __alignof_is_defined not defined
+#endif
+
+  return 0;
+}
Index: libcxx/test/libcxx/no_assert_include.compile.pass.cpp
===
--- libcxx/test/libcxx/no_assert_include.compile.pass.cpp
+++ libcxx/test/libcxx/no_assert_include.compile.pass.cpp
@@ -63,6 +63,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/test/libcxx/min_max_macros.compile.pass.cpp
===
--- libcxx/test/libcxx/min_max_macros.compile.pass.cpp
+++ libcxx/test/libcxx/min_max_macros.compile.pass.cpp
@@ -96,6 +96,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// 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 
+
+#include "test_macros.h"
+
+#ifndef _LIBCPP_VERSION
+#error _LIBCPP_VERSION not defined
+#endif
+
+int main(int, char**)
+{
+
+  return 0;
+}
Index: libcxx/test/libcxx/double_include.sh.cpp
===
--- libcxx/test/libcxx/double_include.sh.cpp
+++ libcxx/test/libcxx/double_include.sh.cpp
@@ -70,6 +70,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/include/module.modulemap
===
--- libcxx/include/module.modulemap
+++ libcxx/include/module.modulemap
@@ -161,7 +161,10 @@
   header "csignal"
   export *
 }
-// FIXME:  is missing.
+module cstdalign {
+  header "cstdalign"
+  export *
+}
 module cstdarg {
   header "cstdarg"
   export *
Index: libcxx/include/cstdalign
===
--- /dev/null
+++ libcxx/include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: libcxx/include/CMakeLists.txt
===
--- libcxx/include/CMakeLists.txt
+++ libcxx/include/CMakeLists.txt
@@ -55,6 +55,7 @@
   condition_variable
   csetjmp
   csignal
+  cstdalign
   cstdarg
   cstdbool
   cstddef
___
cfe-commits mailing 

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen updated this revision to Diff 328533.
khchen added a comment.

rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vfadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
  clang/test/CodeGen/RISCV/vadd.c
  clang/test/Headers/riscv-vector-header.c
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

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

[PATCH] D98023: [clang] Don't make the g++ driver imply an explicitly shared libunwind

[Mateusz Mikuła via Phabricator via cfe-commits]

mati865 added a comment.

It's clear overlook from D79995 , that part is 
perfectly reasonable to me.

I'm a bit worried about this code though:

  bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
  !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();

https://github.com/llvm/llvm-project/blob/0b274ed499603d30694c0b995252ab014609acf9/clang/lib/Driver/ToolChains/CommonArgs.cpp#L1413
IIUC this change would flip it from `false` to `true` for Linux targets, which 
also sounds fine on it's own but goes beyond this diff description and seems to 
lack any test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98023

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

[PATCH] D97930: [OpenCL] Fix builtins that require multiple extensions

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh updated this revision to Diff 328549.
svenvh added a comment.

> You don't seem to be using write_imagef for OpenCL versions <= 1.2. But 
> should we be checking that the diagnostic is given?

Good point, I have updated the test to check for the diagnostic.


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

https://reviews.llvm.org/D97930

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -17,6 +17,10 @@
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 #endif
 
+#if __OPENCL_C_VERSION__ <= CL_VERSION_1_2
+#pragma OPENCL EXTENSION cl_khr_3d_image_writes : enable
+#endif
+
 // First, test that Clang gracefully handles missing types.
 #ifdef NO_HEADER
 void test_without_header() {
@@ -169,13 +173,20 @@
 }
 #endif // __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 
-kernel void basic_image_writeonly(write_only image1d_buffer_t 
image_write_only_image1d_buffer) {
+kernel void basic_image_writeonly(write_only image1d_buffer_t 
image_write_only_image1d_buffer, write_only image3d_t image3dwo) {
   half4 h4;
   float4 f4;
   int i;
 
   write_imagef(image_write_only_image1d_buffer, i, f4);
   write_imageh(image_write_only_image1d_buffer, i, h4);
+
+  int4 i4;
+  write_imagef(image3dwo, i4, i, f4);
+#if __OPENCL_C_VERSION__ <= CL_VERSION_1_2 && !defined(__OPENCL_CPP_VERSION__)
+  // expected-error@-2{{no matching function for call to 'write_imagef'}}
+  // expected-note@-3 + {{candidate function not viable}}
+#endif
 }
 
 kernel void basic_subgroup(global uint *out) {
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -815,9 +815,20 @@
 // Ignore this builtin function if it carries an extension macro that is
 // not defined. This indicates that the extension is not supported by the
 // target, so the builtin function should not be available.
-StringRef Ext = FunctionExtensionTable[OpenCLBuiltin.Extension];
-if (!Ext.empty() && !S.getPreprocessor().isMacroDefined(Ext))
-  continue;
+StringRef Extensions = FunctionExtensionTable[OpenCLBuiltin.Extension];
+if (!Extensions.empty()) {
+  SmallVector ExtVec;
+  Extensions.split(ExtVec, " ");
+  bool AllExtensionsDefined = true;
+  for (StringRef Ext : ExtVec) {
+if (!S.getPreprocessor().isMacroDefined(Ext)) {
+  AllExtensionsDefined = false;
+  break;
+}
+  }
+  if (!AllExtensionsDefined)
+continue;
+}
 
 SmallVector RetTypes;
 SmallVector, 5> ArgTypes;


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -17,6 +17,10 @@
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 #endif
 
+#if __OPENCL_C_VERSION__ <= CL_VERSION_1_2
+#pragma OPENCL EXTENSION cl_khr_3d_image_writes : enable
+#endif
+
 // First, test that Clang gracefully handles missing types.
 #ifdef NO_HEADER
 void test_without_header() {
@@ -169,13 +173,20 @@
 }
 #endif // __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 
-kernel void basic_image_writeonly(write_only image1d_buffer_t image_write_only_image1d_buffer) {
+kernel void basic_image_writeonly(write_only image1d_buffer_t image_write_only_image1d_buffer, write_only image3d_t image3dwo) {
   half4 h4;
   float4 f4;
   int i;
 
   write_imagef(image_write_only_image1d_buffer, i, f4);
   write_imageh(image_write_only_image1d_buffer, i, h4);
+
+  int4 i4;
+  write_imagef(image3dwo, i4, i, f4);
+#if __OPENCL_C_VERSION__ <= CL_VERSION_1_2 && !defined(__OPENCL_CPP_VERSION__)
+  // expected-error@-2{{no matching function for call to 'write_imagef'}}
+  // expected-note@-3 + {{candidate function not viable}}
+#endif
 }
 
 kernel void basic_subgroup(global uint *out) {
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -815,9 +815,20 @@
 // Ignore this builtin function if it carries an extension macro that is
 // not defined. This indicates that the extension is not supported by the
 // target, so the builtin function should not be available.
-StringRef Ext = FunctionExtensionTable[OpenCLBuiltin.Extension];
-if (!Ext.empty() && !S.getPreprocessor().isMacroDefined(Ext))
-  continue;
+StringRef Extensions = FunctionExtensionTable[OpenCLBuiltin.Extension];
+if (!Extensions.empty()) {
+  SmallVector ExtVec;
+  Extensions.split(ExtVec, " ");
+  bool AllExtensionsDefined = true;
+  for (StringRef Ext : ExtVec) {
+ 

[PATCH] D46443: Add missing cstdalign header

[Louis Dionne via Phabricator via cfe-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:8
+//===--===//
+
+// test 

I think we should add `// REQUIRES: c++03 || c++11 || c++14 || c++17`, because 
a conforming implementation could be not shipping the header in C++20 and 
beyond.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:14-28
+#ifdef alignas
+#error alignas should not be a macro in C++
+#endif
+
+#ifdef alignof
+#error alignof should not be a macro in C++
+#endif

This can be moved outside of `main()`.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:27
+#ifndef __alignof_is_defined
+#error __alignof_is_defined not defined
+#endif

I'm not seeing `__alignof_is_defined` anywhere in the spec?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D92363: [HIP] Warn no --offload-arch option

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

@tra I got some issue with this patch. There are cases that an expression using 
a host variable is compile-time constant, e.g.

  int x;
  __device__ void fun() {
sizeof(x);
  }

Do we want to allow that? Thanks.


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

https://reviews.llvm.org/D92363

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

[PATCH] D92363: [HIP] Warn no --offload-arch option

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D92363#2607072 , @yaxunl wrote:

> @tra I got some issue with this patch. There are cases that an expression 
> using a host variable is compile-time constant, e.g.
>
>   int x;
>   __device__ void fun() {
> sizeof(x);
>   }
>
> Do we want to allow that? Thanks.

sorry, wrong patch.


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

https://reviews.llvm.org/D92363

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

[PATCH] D91281: [CUDA][HIP] Diagnose reference of host variable

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

@tra I got some issue with this patch. There are cases that an expression using 
a host variable is compile-time constant, e.g.

  int x;
  __device__ void fun() {
sizeof(x);
  }

Do we want to allow that? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91281

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

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Amy Kwan via Phabricator via cfe-commits]

amyk added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1885
 
+let isReMaterializable = 1, isMoveImm = 1, Predicates = [PrefixInstrs] in {
+  def XXSPLTI32DX :

I think it might be good to add a comment of why the `XXSPLTI32DX` instruction 
needs to be split out like this from the other instructions.



Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll:41
+; which is defined between the pld and the stb.
+; In this test the stb r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lbz r3, 0(r3)

nit: Remove duplicate comment.



Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll:63
+; which is defined between the pld and the sth.
+; In this test the sth r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lhz r3, 0(r3)

nit: Remove duplicate comment.


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

https://reviews.llvm.org/D95458

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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 marked 2 inline comments as done.
sbc100 added inline comments.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:14-28
+#ifdef alignas
+#error alignas should not be a macro in C++
+#endif
+
+#ifdef alignof
+#error alignof should not be a macro in C++
+#endif

ldionne wrote:
> This can be moved outside of `main()`.
I was copying from 
`libcxx/test/std/language.support/cstdint/cstdint.syn/cstdint.pass.cpp` which 
does these checks inside of main.   

Will move them.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:27
+#ifndef __alignof_is_defined
+#error __alignof_is_defined not defined
+#endif

ldionne wrote:
> I'm not seeing `__alignof_is_defined` anywhere in the spec?
Removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 328557.
sbc100 marked 2 inline comments as done.
sbc100 added a comment.

feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

Files:
  libcxx/include/CMakeLists.txt
  libcxx/include/cstdalign
  libcxx/include/module.modulemap
  libcxx/test/libcxx/double_include.sh.cpp
  libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
  libcxx/test/libcxx/min_max_macros.compile.pass.cpp
  libcxx/test/libcxx/no_assert_include.compile.pass.cpp
  libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp

Index: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp
@@ -0,0 +1,24 @@
+//===--===//
+//
+// 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
+//
+//===--===//
+
+// test 
+
+#include 
+
+#ifdef alignas
+#error alignas should not be a macro in C++
+#endif
+
+#ifndef __alignas_is_defined
+#error __alignas_is_defined not defined
+#endif
+
+int main(int, char**) {
+
+  return 0;
+}
Index: libcxx/test/libcxx/no_assert_include.compile.pass.cpp
===
--- libcxx/test/libcxx/no_assert_include.compile.pass.cpp
+++ libcxx/test/libcxx/no_assert_include.compile.pass.cpp
@@ -63,6 +63,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/test/libcxx/min_max_macros.compile.pass.cpp
===
--- libcxx/test/libcxx/min_max_macros.compile.pass.cpp
+++ libcxx/test/libcxx/min_max_macros.compile.pass.cpp
@@ -96,6 +96,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// 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 
+
+#include "test_macros.h"
+
+#ifndef _LIBCPP_VERSION
+#error _LIBCPP_VERSION not defined
+#endif
+
+int main(int, char**)
+{
+
+  return 0;
+}
Index: libcxx/test/libcxx/double_include.sh.cpp
===
--- libcxx/test/libcxx/double_include.sh.cpp
+++ libcxx/test/libcxx/double_include.sh.cpp
@@ -70,6 +70,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/include/module.modulemap
===
--- libcxx/include/module.modulemap
+++ libcxx/include/module.modulemap
@@ -161,7 +161,10 @@
   header "csignal"
   export *
 }
-// FIXME:  is missing.
+module cstdalign {
+  header "cstdalign"
+  export *
+}
 module cstdarg {
   header "cstdarg"
   export *
Index: libcxx/include/cstdalign
===
--- /dev/null
+++ libcxx/include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: libcxx/include/CMakeLists.txt
===
--- libcxx/include/CMakeLists.txt
+++ libcxx/include/CMakeLists.txt
@@ -55,6 +55,7 @@
   condition_variable
   csetjmp
   csignal
+  cstdalign
   cstdarg
   cstdbool
   cstddef
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: rjmccall, erichkeane, aaron.ballman.
fhahn requested review of this revision.
Herald added a project: clang.

This patch implements the conditional select operator for
ext_vector_types in C++. It does so by using the same semantics as for
C.

D71463  added support for the conditional 
select operator for VectorType
in C++. Unfortunately the semantics between ext_vector_type in C are
different to VectorType in C++. Select for ext_vector_type is based on
the MSB of the condition vector, whereas for VectorType it is `!= 0`.

This unfortunately means that the behavior is inconsistent between
ExtVectorType and VectorType, but I think using the C semantics for
ExtVectorType in C++ as well should be less surprising for users.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98055

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/ext-vector-type-conditional.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/CodeGenCXX/vector-size-conditional.cpp
  clang/test/SemaCXX/ext-vector-type-conditional.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector-size-conditional.cpp

Index: 
===
--- /dev/null
+++ /dev/null
@@ -1,172 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
-// Note that this test depends on the size of long-long to be different from
-// int, so it specifies a triple.
-
-using FourShorts = short __attribute__((__vector_size__(8)));
-using TwoInts = int __attribute__((__vector_size__(8)));
-using TwoUInts = unsigned __attribute__((__vector_size__(8)));
-using FourInts = int __attribute__((__vector_size__(16)));
-using FourUInts = unsigned __attribute__((__vector_size__(16)));
-using TwoLongLong = long long __attribute__((__vector_size__(16)));
-using FourLongLong = long long __attribute__((__vector_size__(32)));
-using TwoFloats = float __attribute__((__vector_size__(8)));
-using FourFloats = float __attribute__((__vector_size__(16)));
-using TwoDoubles = double __attribute__((__vector_size__(16)));
-using FourDoubles = double __attribute__((__vector_size__(32)));
-
-FourShorts four_shorts;
-TwoInts two_ints;
-TwoUInts two_uints;
-FourInts four_ints;
-FourUInts four_uints;
-TwoLongLong two_ll;
-FourLongLong four_ll;
-TwoFloats two_floats;
-FourFloats four_floats;
-TwoDoubles two_doubles;
-FourDoubles four_doubles;
-
-enum E {};
-enum class SE {};
-E e;
-SE se;
-
-// Check the rules of the condition of the conditional operator.
-void Condition() {
-  // Only int types are allowed here, the rest should fail to convert to bool.
-  (void)(four_floats ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
-  (void)(two_doubles ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
-}
-
-// Check the rules of the LHS/RHS of the conditional operator.
-void Operands() {
-  (void)(four_ints ? four_ints : throw 1); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
-  (void)(four_ints ? throw 1 : four_ints); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
-  (void)(four_ints ?: throw 1);// expected-error {{GNU vector conditional operand cannot be a throw expression}}
-  (void)(four_ints ? (void)1 : four_ints); // expected-error {{GNU vector conditional operand cannot be void}}
-  (void)(four_ints ?: (void)1);// expected-error {{GNU vector conditional operand cannot be void}}
-
-  // Vector types must be the same element size as the condition.
-  (void)(four_ints ? two_ll : two_ll); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoLongLong' (vector of 2 'long long' values) do not have the same number of elements}}
-  (void)(four_ints ? four_ll : four_ll);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourLongLong' (vector of 4 'long long' values) do not have elements of the same size}}
-  (void)(four_ints ? two_doubles : two_doubles);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoDoubles' (vector of 2 'double' values) do not have the same number of elements}}
-  (void)(four_ints ? four_doubles : four_doubles); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourDoubles' (vector of 4 'double' values) do not have elements of the same size}}
-  (void)(four_ints ?: two_ints);   // expected-error {{vector operands to the vector conditional must be the same type ('FourInts' (vector of 4 'int' values) and 'TwoInts' (vector of 2 'int' values)}}
-  (void)(four_ints ?: four_doubles);   // expected-error {{vector operands to the vector 

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 328561.
sbc100 added a comment.

feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

Files:
  libcxx/include/CMakeLists.txt
  libcxx/include/cstdalign
  libcxx/include/module.modulemap
  libcxx/test/libcxx/double_include.sh.cpp
  libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
  libcxx/test/libcxx/min_max_macros.compile.pass.cpp
  libcxx/test/libcxx/no_assert_include.compile.pass.cpp
  libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp

Index: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp
@@ -0,0 +1,27 @@
+//===--===//
+//
+// 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
+//
+//===--===//
+
+// test 
+
+// This header was removed in C++20
+// REQUIRES: c++03 || c++11 || c++14 || c++17
+
+#include 
+
+#ifdef alignas
+#error alignas should not be a macro in C++
+#endif
+
+#ifndef __alignas_is_defined
+#error __alignas_is_defined not defined
+#endif
+
+int main(int, char**) {
+
+  return 0;
+}
Index: libcxx/test/libcxx/no_assert_include.compile.pass.cpp
===
--- libcxx/test/libcxx/no_assert_include.compile.pass.cpp
+++ libcxx/test/libcxx/no_assert_include.compile.pass.cpp
@@ -63,6 +63,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/test/libcxx/min_max_macros.compile.pass.cpp
===
--- libcxx/test/libcxx/min_max_macros.compile.pass.cpp
+++ libcxx/test/libcxx/min_max_macros.compile.pass.cpp
@@ -96,6 +96,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/language.support/cstdalign/version.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// 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 
+
+#include "test_macros.h"
+
+#ifndef _LIBCPP_VERSION
+#error _LIBCPP_VERSION not defined
+#endif
+
+int main(int, char**)
+{
+
+  return 0;
+}
Index: libcxx/test/libcxx/double_include.sh.cpp
===
--- libcxx/test/libcxx/double_include.sh.cpp
+++ libcxx/test/libcxx/double_include.sh.cpp
@@ -70,6 +70,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: libcxx/include/module.modulemap
===
--- libcxx/include/module.modulemap
+++ libcxx/include/module.modulemap
@@ -161,7 +161,10 @@
   header "csignal"
   export *
 }
-// FIXME:  is missing.
+module cstdalign {
+  header "cstdalign"
+  export *
+}
 module cstdarg {
   header "cstdarg"
   export *
Index: libcxx/include/cstdalign
===
--- /dev/null
+++ libcxx/include/cstdalign
@@ -0,0 +1,30 @@
+// -*- C++ -*-
+//===--- cstdalign ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: libcxx/include/CMakeLists.txt
===
--- libcxx/include/CMakeLists.txt
+++ libcxx/include/CMakeLists.txt
@@ -55,6 +55,7 @@
   condition_variable
   csetjmp
   csignal
+  cstdalign
   cstdarg
   cstdbool
   cstddef
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits