[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

tschuett added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

tschuett added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227 + // Specifies, using a regex, which successful optimization passes done, + // to include in the final optimization record file generated. If not provided victorkingi

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

tschuett added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227 + // Specifies, using a regex, which successful optimization passes done, + // to include in the final optimization record file generated. If not provided awarzynski

[PATCH] D156320: [FLang] Add support for Rpass flag

tschuett added inline comments. Comment at: flang/include/flang/Frontend/CodeGenOptions.h:72 + enum RemarkKind { +RK_Missing,// Remark argument not present on the command line. enum class? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D152351: [clang] Add __builtin_isfpclass

tschuett added a comment. Could Clang offer a builtin.h file? It is always included. You place named constants and enums inside. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152351/new/ https://reviews.llvm.org/D152351

[PATCH] D149444: [ARM] Allow codegen for Armv6m eXecute-Only (XO) sections

tschuett added inline comments. Comment at: llvm/lib/Target/ARM/ARMSubtarget.cpp:194 + NoMovt = false; +assert(hasV6MOps() && "Cannot generate execute-only code for this target"); } What happens in release mode? At the top you now claim that ARMV6M

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names and introduce Bfloat16 arithmetic type.

tschuett added a comment. Agreed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149573/new/ https://reviews.llvm.org/D149573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names and introduce Bfloat16 arithmetic type.

tschuett added a comment. I don't believe that there is NativeBFloat16Type. AArch64 learned bf16 with ARMv8.6-A, but very limited operations and only in SIMD. X86 supports bf16 since Cooperlake, but very limited operations and only in SIMD. Maybe GPUs? CHANGES SINCE LAST ACTION

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

tschuett added inline comments. Comment at: clang/lib/Interpreter/Interpreter.cpp:198 +// These better to put in a runtime header but we can't. This is because we +// can't find the percise resource directory in unittests so we have to hard +// code them.

[PATCH] D145579: [Clang][Flang][AMDGPU] Add support for AMDGPU to Flang driver

tschuett added a comment. I wanted to ask whether you want to put an AMDGPU.cpp and AMD.cpp file in the flang/lib/Frontend directory. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145579/new/ https://reviews.llvm.org/D145579 ___ cfe-commits

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

tschuett added a comment. Could you hide the amdgpu and nvptx somewhere here `clang -print-resource-dir` in two different directories? One for AMD, one for NVPTX. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146973/new/

[PATCH] D145579: [Clang][Flang][AMDGPU] Add support for AMDGPU to Flang driver

tschuett added a comment. Do you want to move the AMDGPU changes into AMDGPU.cpp next to AMD.cpp? From the conversation, there seems to be more target specific behaviours. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145579/new/ https://reviews.llvm.org/D145579

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

tschuett added a comment. You can either pass parameters to passes or use different pass pipelines for different optimisation levels (in MLIR territory). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146278/new/ https://reviews.llvm.org/D146278

[PATCH] D146163: Experimental new python bindings for clang/llvm based on Cython

tschuett added a comment. In D146163#4202969 , @rickmark wrote: > Began discussion in Discord to see interest for other collaborators on this > type of binding: > https://discord.com/channels/636084430946959380/700158649129238591/1086362007684513844

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

tschuett added a comment. Flang will also support OpenACC for offload. It is very similar to OpenMP. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145815/new/ https://reviews.llvm.org/D145815 ___

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

tschuett added a comment. Frontend is in the file name. Comment at: flang/lib/Frontend/FrontendActions.cpp:79 + llvm::StringRef inputFile, llvm::StringRef outputTag) { + if (!ci.getCodeGenOpts().SaveTempsDir.has_value()) { +return true;

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

tschuett added a comment. What is the policy on trivial braces in the frontend? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146075/new/ https://reviews.llvm.org/D146075 ___ cfe-commits mailing list

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

tschuett added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:77 +/// specified. +bool saveMLIRTempFile(const CompilerInvocation , mlir::ModuleOp mlirModule, + llvm::StringRef inputFile, llvm::StringRef outputTag) {

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

tschuett added a comment. Any binary that uses this feature is not forward portable to hardware with a larger vector size. That's true for SVE as well. I did not understood this sentence. AFAIK, SVE uses the ptrue instruction to generate a mask to only activate the necessary lanes. If I do

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

tschuett added inline comments. Comment at: clang/include/clang/Lex/Lexer.h:89 - // End of the buffer. - const char *BufferEnd; + // Size of the buffer. + unsigned BufferSize; cor3ntin wrote: > Should that use `SourceLocation::UIntTy`? > Looking at

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

tschuett added a comment. For AArch64 the default alignment is 0? I would have expected 128. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141910/new/ https://reviews.llvm.org/D141910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

tschuett added a comment. In D133289#4096588 , @aaron.ballman wrote: > In D133289#4037180 , @tschuett > wrote: > >> It surprised me that there are no type inference messages? The type of this >> auto is

[PATCH] D142578: [Clang][Doc] Edit the Clang release notes

tschuett added a comment. IDK. Clang 16 is fully is a fully conformant C++20 except for some DRs. Or beginning with Clang 16, we start a long-term project to overhaul the diagnostics. I agree that breaking changes are important. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D142578: [Clang][Doc] Edit the Clang release notes

tschuett added a comment. I am not the only who has a different background. If the introduction says this is the introduction of the release notes, I am not super motivated to read on. I would prefer to see some highlights to motivate the readers to continue reading. Repository: rG LLVM

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

tschuett added a comment. Could you please put an X86.rst file into llvm/docs? There is a Sphinx template with instructions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141899/new/ https://reviews.llvm.org/D141899

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

tschuett added a comment. EXPANSIVE_CHECKS will reshuffle the llvm::sort input: https://lists.llvm.org/pipermail/llvm-dev/2018-April/122576.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D141625

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

tschuett added a comment. The summary only says what you are doing. I am missing something about why. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141899/new/ https://reviews.llvm.org/D141899 ___

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

tschuett added a comment. - Can "x86.AMX" be a named constant somewhere? I found a lot of copies. - Release note? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141899/new/ https://reviews.llvm.org/D141899

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

tschuett added a comment. I though Clang Basic is a leaf library and must not depend on anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141581/new/ https://reviews.llvm.org/D141581 ___

[PATCH] D141723: [Clang] Remove `CLANG_OPENMP_NVPTX_DEFAULT_ARCH` CMake option.

tschuett added a comment. Up right you will find `Edit Related Revisions...`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141723/new/ https://reviews.llvm.org/D141723 ___ cfe-commits mailing list

[PATCH] D141310: [clang] add -Wcompare-function-pointers

tschuett added subscribers: MaskRay, tschuett. tschuett added a comment. @MaskRay Dependent of the build flags, we decide which warnings to enable or not? Do we have other warnings for ThinLTO? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

tschuett added a comment. It surprised me that there are no type inference messages? The type of this auto is double. I only found warnings of misuse of auto and codegen tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133289/new/

[PATCH] D140959: RFC: Multilib prototype

tschuett added a comment. Did you look at Yaml I/O ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140959/new/ https://reviews.llvm.org/D140959 ___ cfe-commits mailing

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

tschuett added a comment. I believe you are arguing for a JSON file. The complexity of flags that want to pass to clang-scan-deps is too high. You even get some benefit of caching if you create a JSON file with several targets. CHANGES SINCE LAST ACTION

[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration

tschuett added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/AddDoxygenComment.cpp:24 +namespace clang { +namespace clangd { +namespace { You can merge this into `namespace clang::clangd`. Comment at:

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

tschuett added a comment. In D140226#4003794 , @jhuber6 wrote: > In D140226#4003788 , @tschuett > wrote: > >> There are already SYCL specific attributes: https://reviews.llvm.org/D60455 > > We could potentially

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

tschuett added a comment. There are already SYCL specific attributes: https://reviews.llvm.org/D60455 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140226/new/ https://reviews.llvm.org/D140226 ___

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

tschuett added inline comments. Comment at: llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll:2 +; RUN: llc -mtriple=aarch64 -mattr=+v8.9a < %s | FileCheck %s +; RUN: llc -mtriple=aarch64 -mattr=+v8.9a -O0 --global-isel-abort=1 < %s | FileCheck %s + lenary wrote:

[PATCH] D139443: [AArch64] Support SLC in ACLE prefetch intrinsics

tschuett added inline comments. Comment at: llvm/test/CodeGen/AArch64/arm64-prefetch-new.ll:2 +; RUN: llc -mtriple=aarch64 -mattr=+v8.9a < %s | FileCheck %s +; RUN: llc -mtriple=aarch64 -mattr=+v8.9a -O0 --global-isel-abort=1 < %s | FileCheck %s + I believe you

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tschuett added a comment. I am bit afraid about release builds: if (!Initializer) return; Is this semantically incorrect? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139148/new/ https://reviews.llvm.org/D139148 ___ cfe-commits

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. I do not ask you to do anything! I just noticed that you add a lot of `FormatXXXDiagnostic` functions. An alternativ design is to have one `FormatDiagnostic` function with a mode parameter. Then you can decide whether to print legacy or user-oriented reasons. If next

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Maybe the kind/amount of information printed ( `DiagnosticMode` ) and the output device (console/sarif) are orthogonal issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/ https://reviews.llvm.org/D138939

[PATCH] D139182: AArch64: add CodeGen support for FEAT_XS DSB instructions

tschuett added a comment. The def file is going away: https://reviews.llvm.org/D139102 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139182/new/ https://reviews.llvm.org/D139182 ___ cfe-commits mailing

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing: enum class DiagnosticMode { Legacy, UserOriented, Default = Legacy } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

tschuett added a comment. The LLVM coding style says to prefer static over anonymous namespaces. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst:19 +* Functions or variables in header files, since anonymous namespaces in headers + is

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Maybe `void FormatDiagnostic(SmallVectorImpl , DiagnosticMode mode)`instead of `void FormatDiagnostic(SmallVectorImpl )`? To make the transition easer and future proof. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138474: [clang] Speedup LineOffsetMapping::get

tschuett added a comment. error: comparison of integers of different signs: 'long' and 'unsigned long' [-Werror,-Wsign-compare] if (End - Start > sizeof(Word)) { Comment at: clang/lib/Basic/SourceManager.cpp:1292 // This is much faster than scanning each byte

[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

tschuett added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64.td:510 +def FeatureRCPC3 : SubtargetFeature<"rcpc3", "HasRCPC3", +"true", "Enable Armv8.9-A RCPC instructions added to A64 and Advanced SIMD and floating-point instruction set (FEAT_LRCPC3)",

[PATCH] D138488: [AArch64][clang] implement 2022 General Data-Processing instructions

tschuett added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:11698 +//--- +// 2022 Architecture Extensions: General Data Processing (FEAT_V94_DP) +//--- FEAT_CSSC or FEAT_V94_DP? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D138376: Use None consistently (NFC)

tschuett added a comment. https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138376/new/ https://reviews.llvm.org/D138376

[PATCH] D137885: [modules] Support zstd in .pcm file

tschuett added a comment. Release note? Comment at: clang/lib/Serialization/ASTReader.cpp:1457 + const llvm::compression::Format F = + Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0 + ? llvm::compression::Format::Zlib

[PATCH] D137825: [clang-include-cleaner] make SymbolLocation a real class, move FindHeaders

tschuett added a comment. You could: static_assert(std::variant_size_v<> == 2); Kind kind() const { if (std::holds_alternative(Storage)) return Kind::Physical; return Kind::Standard; } and Kind could become `enum class Kind`. => You would get rid of the order on the variant and

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

tschuett added a comment. In D136589#3915126 , @vhscampos wrote: > @tschuett Do you still want me to put the note about 'native' detection for > neoverse-v2 back? Up to you. No worries. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

tschuett added a comment. No worries. The style asks to move the definitions out of the anonymous namespace, but Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135859/new/ https://reviews.llvm.org/D135859

[PATCH] D137488: [clang][Interp] Array initialization via string literal

tschuett added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1138 +// FIXME: There is a certain code duplication between here +// and Porgram::createGlobalString(). +const size_t CharWidth = SL->getCharByteWidth(); tschuett

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

tschuett added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.h:17 +namespace tidy { +namespace modernize { + Why do you refuse to use nested namespaces in `modernize` ? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D137487: [clang][Interp] Start implementing builtin functions

tschuett added inline comments. Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:14 +namespace clang { +namespace interp { + You are allowed to use nested namespaces. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137488: [clang][Interp] Array initialization via string literal

tschuett added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1138 +// FIXME: There is a certain code duplication between here +// and Porgram::createGlobalString(). +const size_t CharWidth = SL->getCharByteWidth(); Program

[PATCH] D136919: [X86][RFC] Change mangle name of __bf16 from u6__bf16 to DF16b

tschuett added subscribers: aaron.ballman, tschuett. tschuett added a comment. @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136919/new/ https://reviews.llvm.org/D136919 ___ cfe-commits

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

tschuett added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:25 namespace clang::include_cleaner { namespace { +using DeclCallback = There is a cuter way to use anonymous namespaces:

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

tschuett added a comment. A release note? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137269/new/ https://reviews.llvm.org/D137269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

tschuett added inline comments. Comment at: clang/docs/ReleaseNotes.rst:696 -- Add driver and tuning support for Neoverse V2 via the flag ``-mcpu=neoverse-v2``. - Native detection is also supported via ``-mcpu=native``. dmgreen wrote: > tschuett wrote: > >

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

tschuett added inline comments. Comment at: clang/docs/ReleaseNotes.rst:696 -- Add driver and tuning support for Neoverse V2 via the flag ``-mcpu=neoverse-v2``. - Native detection is also supported via ``-mcpu=native``. What happened with native detection?

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

tschuett added a comment. Almost all flags are off by default. That's why they are called flags, i.e., `-Weverything`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134410/new/ https://reviews.llvm.org/D134410

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

tschuett added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:676 +namespace { +void applyNoundefToLoadInst(bool enable, const clang::QualType , +llvm::LoadInst *Load) { Nit: You meant static. Repository: rG LLVM Github

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

tschuett added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:148 +/// Tests whether the given identifier is reserved as a module name and +/// diagnoses if it is. Returs true if a diagnostic is emitted and false +/// otherwise. Returns CHANGES

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

tschuett added a comment. Are malformed imports an issue or are they already covered? There is limit test coverage for import. Am I missing something? import import; import module; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136953/new/ https://reviews.llvm.org/D136953

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

tschuett added a comment. Thx. Comment at: clang/test/Modules/reserved-names-1.cpp:42 + +// We can still use a reserved name on imoport. +import std; // expected-error {{module 'std' not found}} import? CHANGES SINCE LAST ACTION

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

tschuett added inline comments. Comment at: clang/tools/clang-stat-cache/clang-stat-cache.cpp:61 + +namespace { + Sorry, but you misuse anonymous namespaces. You want static instead. https://llvm.org/docs/CodingStandards.html#anonymous-namespaces Repository:

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

tschuett added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2120 +Optional SDKName = None; +if (getTriple().isWatchOS()) { + if (getTriple().isSimulatorEnvironment()) Will there be an enum over the Apple variants to make this

[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

tschuett added a comment. We support Apple Clang 9.3, but `std:variant` ships with Apple Clang 10.0. :-( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135953/new/ https://reviews.llvm.org/D135953 ___

[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

tschuett added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:64 +using SymbolLocation = std::variant; +/// A set of locations that provides the declaration, while indicating if sammccall wrote: > This is an important

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

tschuett added a comment. The one phase compilation should work with `-fno-implicit-modules`! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/ https://reviews.llvm.org/D134267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

tschuett added a comment. What really worries me that you are talking about one phase compilation and there is a module cache. Even if it is one phase compilation you must pass all dependent modules on the command line. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/

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

tschuett added a comment. In D134267#3848838 , @iains wrote: > In D134267#3848822 , @tschuett > wrote: > >> Clang header modules started with implicit builds and module caches. Apple >> is moving too explicit

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

tschuett added a comment. Clang header modules started with implicit builds and module caches. Apple is moving too explicit builds. There are all kinds of problems with implicit module builds. I believe that C++20 modules shouldn't make the same mistake. CHANGES SINCE LAST ACTION

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

tschuett added a comment. The OpenMP frontend is mainly an IRBuilder. It is a different layering than for HLSL. Are there plans for an HLSL(IR)Builder? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135110/new/ https://reviews.llvm.org/D135110

[PATCH] D134791: [clang] Unify Sema and CodeGen implementation of isFlexibleArrayMemberExpr

tschuett added inline comments. Comment at: clang/lib/AST/Expr.cpp:225 + + } else if (!Context.getAsIncompleteArrayType(getType())) +return false; Could this be an early exit? Comment at: clang/lib/AST/Expr.cpp:267 + +if

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

tschuett added a comment. Thanks for doing this! The traditional Clang Header modules call this implicit and explicit modules. Feel free to ignore, but I found this awesome article about implicit and explicit modules in Swift. They have same nomenclature and issues.

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

tschuett added inline comments. Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:238 AArch64::AEK_RAND | AArch64::AEK_FP16FML | AArch64::AEK_I8MM)) +AARCH64_CPU_NAME("neoverse-v2", ARMV9A, FK_NEON_FP_ARMV8, false, +

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

tschuett added a comment. `VScaleForTuning` is 1 for N2 and V2. It is 2 for V1. I though the V2 is more like the V1 than the N2? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134352/new/ https://reviews.llvm.org/D134352 ___ cfe-commits

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

tschuett added a comment. The LLD release notes always refer to the Diff where the change was introduced. It is kind of sad that we loose all the discussions when we move to GitHub PRs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133771/new/

[PATCH] D133392: [MTE] Add AArch64GlobalsTagging Pass

tschuett added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp:29 + +namespace { + I believe you are going too far with the anonymous namespace. There is no need for static functions in anonymous namespaces.

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

tschuett added inline comments. Comment at: clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp:25 + +// Register the GoogleTidyModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add Is Google a copy and paste error?

[PATCH] D133392: [MTE] Add AArch64GlobalsTagging Pass

tschuett added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp:93 + + uint64_t NewSize = alignTo(SizeInBytes, 16); + if (SizeInBytes != NewSize) { If the `16` is the size of the granule, then it deserves to be named constant.

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

tschuett added a comment. Will there be eventually a way to build a fully optimised clang/lld with ThinLTO, PGO, and Bolt? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132975/new/ https://reviews.llvm.org/D132975

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

tschuett added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:44 + + enum CommandKind { +CK_CC1, Why is this not an enum class? Comment at:

[PATCH] D128930: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.

tschuett added inline comments. Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Forest.h:202 +class ForestNode::RecursiveIterator +: public std::iterator { + llvm::DenseSet Seen; `std::iterator` is deprecated in C++17 and creates warnings.

[PATCH] D67025: Add .inl as valid C++ header type

tschuett added a comment. Note that in this directory is an ,inl file: https://github.com/openucx/ucx/tree/master/src/uct/ib/mlx5 It is a pure C library with C++ gtest. I believe that .inl is about inlining, but it is not tied to a language. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

tschuett added a comment. Could you cite a Diff where this was changed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129135/new/ https://reviews.llvm.org/D129135 ___ cfe-commits mailing list

[PATCH] D128415: [ARM] Add Support for Cortex-M85

tschuett accepted this revision. tschuett added a comment. LGTM. Please update the summary before you commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128415/new/ https://reviews.llvm.org/D128415

[PATCH] D128415: [ARM] Add Support for Cortex-M85

tschuett added a comment. The Clang release notes indicate that PACBTI is off by default. In several places, I can see PACBTI. Is the `ARM.td` missing something? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128415/new/

[PATCH] D128372: Clang-Tidy Empty Check

tschuett added a comment. Just for reference: https://reviews.llvm.org/D128267 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list

[PATCH] D128098: [clang][driver] Ensure we don't accumulate entries in -MJ files

tschuett added a comment. There are now several JSON fragments per file? Perviously there was one fragment per file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128098/new/ https://reviews.llvm.org/D128098

[PATCH] D128098: [clang][driver] Ensure we don't accumulate entries in -MJ files

tschuett added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128098/new/ https://reviews.llvm.org/D128098 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D127898: [clang][dataflow] Find unsafe locs after fixpoint

tschuett added a comment. C++ seems to be tied to templates. I wonder if super classes resp. inheritance would make your life easier. You can explicitly define the API and documentation. You could provide default implementations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D127976: [IR] Move vector.insert/vector.extract out of experimental namespace

tschuett added a comment. Maybe a line in the Release Notes of LLVM? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127976/new/ https://reviews.llvm.org/D127976 ___ cfe-commits mailing list

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

tschuett added a comment. Do you believe an entry in the ReleaseNotes would get this achievement more visibility? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127379/new/ https://reviews.llvm.org/D127379

[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

tschuett added a comment. In D126796#3551781 , @cjdb wrote: > In D126796#3551312 , @tschuett > wrote: > >> Would a JSON dump help you for your tools? > > Do you think a JSON dump would be more appropriate? I was

[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

tschuett added a comment. Would a JSON dump help you for your tools? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126796/new/ https://reviews.llvm.org/D126796 ___ cfe-commits mailing list

  1   2   >