[PATCH] D117593: [clang-tidy] Change google-explicit-constructor to ignore conversions and operators marked `explicit(false)`

2023-10-12 Thread Luka Govedič via Phabricator via cfe-commits
ProExpertProg added a comment. Is there an update on this? I agree adding a flag to control this behavior would be good but the Google style guide clearly does not take the `explicit(false)` option into consideration. Currently the only alternatives are a disabling the check completely or addin

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

2023-10-12 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment. In D152914#4653669 , @lei wrote: > HI @nemanjai, Did you get a chance to post this as a github PR? Long overdue, but I finally have it up at https://github.com/llvm/llvm-project/pull/68919 Repository: rG LLVM Github Monorep

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-12 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment. In D153131#4653664 , @aaronpuchert wrote: > In D153131#4653564 , @courbet wrote: > >> We have a large number of users of `-Werror -Wthread-safety-analysis` >> internally. When we make th

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-12 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments. Comment at: clang/unittests/AST/Interp/CMakeLists.txt:2 +add_clang_unittest(InterpTests + Descriptor.cpp + ) thakis wrote: > Why is this in a separate executable instead of in ASTTests? So it can have > fewer deps? Mostly because

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-12 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments. Comment at: clang/unittests/AST/Interp/CMakeLists.txt:2 +add_clang_unittest(InterpTests + Descriptor.cpp + ) Why is this in a separate executable instead of in ASTTests? So it can have fewer deps? Repository: rG LLVM Github Mo

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-12 Thread Timm Bäder 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 rG2f43ace0f48f: [clang][Interp] Fix expected values for Pointer API (authored by tbaeder). Changed prior to commit: https://reviews.llvm.org/D158069

[PATCH] D8484: Allow -gsplit-dwarf for all ELF targets, not just Linux

2023-10-11 Thread Ed Maste via Phabricator via cfe-commits
emaste abandoned this revision. emaste added a comment. Herald added a project: All. Allowed for all ELF OSes as of commit ee957e045f526ce45d24b0f081f277262c3da43d Author: Fangrui Song Date: Thu Mar 28 08:24:00 2019 + [Driver] Allow -gsplit-dwarf on ELF OSes other than Linux

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158069/new/ https://reviews.llvm.org/D158069 ___ cfe-commits mailing lis

[PATCH] D153001: [clang][ThreadSafety] Add __builtin_instance_member (WIP)

2023-10-11 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder abandoned this revision. tbaeder added a comment. As discussed via other channels, this is going nowhere and we will take another approach, if any. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153001/new/ https://reviews.llvm.org/D153001

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:42 const Descriptor *D) { + new (Ptr) InitMapPtr(std::nullopt); + tbaeder wrote: > aaron.ballman wrote: > > This worries me a little bit for a few reaso

[PATCH] D159351: [Sema] Change order of displayed overloads in diagnostics

2023-10-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think the changes should come with a release note so users know about the improved user experience (it would be great to show the code example from this patch summary in the re

[PATCH] D156784: [AArch64][PAC] Declare FPAC subtarget feature

2023-10-11 Thread Anatoly Trosinenko via Phabricator via cfe-commits
atrosinenko added a comment. As discussed in D156716 , it is not clear if I have to add FeatureFPAC to every relevant CPU. Maybe it is worth conservatively assuming that this feature should only be enabled manually by the user as a precaution against "I have CP

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

2023-10-11 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment. The buildbot found one more test that needed updating, that was disabled on my system. Created https://github.com/llvm/llvm-project/pull/68781 for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86310/new/ https://revie

[PATCH] D159351: [Sema] Change order of displayed overloads in diagnostics

2023-10-11 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D159351#4653687 , @shafik wrote: > You say "attempts to be a strict weak order" does that mean there are still > cases which will cause an assert or are we not sure? No, it's an actual strict weak order. It's a bad cho

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. To be able to reason about the impact of various memory usage optimizations, I took some baseline measurements. I used a workload consisting of the LLVM codebase, with the compile_commands.json entries filtered to those containing `clang/lib` or `clang-tools-extra/clang

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge commandeered this revision. nridge added a reviewer: qchateau. nridge added a comment. In D93829#4422090 , @qchateau wrote: > So if anyone wants to take it from there, feel free to reuse my commits (or > not). I'm going to pick up this work and tr

[PATCH] D159351: [Sema] Change order of displayed overloads in diagnostics

2023-10-10 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. You say "attempts to be a strict weak order" does that mean there are still cases which will cause an assert or are we not sure? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159351/new/ https://reviews.llvm.org/D159351 __

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

2023-10-10 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment. In D86310#4653550 , @tmgross wrote: > Probably would be good to add https://bugs.llvm.org/show_bug.cgi?id=50198 to > the test suite if it isn't there already. That test would not work as an LLVM test directly, but we do already ha

[PATCH] D140925: [CMake] Use Clang to infer the target triple

2023-10-10 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: runtimes/CMakeLists.txt:167 +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(CXX_TARGET_TRIPLE ${CMAKE_CXX_COMPILER} --target=${LLVM_RUNTIME_TRIPLE} -print-target-triple) ldionne wrote: > Is there any reas

[PATCH] D140925: [CMake] Use Clang to infer the target triple

2023-10-10 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision. ldionne added inline comments. This revision is now accepted and ready to land. Comment at: runtimes/CMakeLists.txt:167 +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(CXX_TARGET_TRIPLE ${CMAKE_CXX_COMPILER} --target=${LLVM_RUNTIME_T

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

2023-10-10 Thread Fei Peng via Phabricator via cfe-commits
fiigii added a comment. That would be fine. Thanks for explaining. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155688/new/ https://reviews.llvm.org/D155688 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

2023-10-10 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. @jaredgrubb please provide your authorship for `git commit --author` so that we can land the patch on your behalf. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 ___ cfe-co

[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

2023-10-10 Thread Lei Huang via Phabricator via cfe-commits
lei added a comment. HI @nemanjai, Did you get a chance to post this as a github PR? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152914/new/ https://reviews.llvm.org/D152914 ___ cfe-commits mailing lis

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

2023-10-10 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision. owenpan added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-10 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D153131#4653564 , @courbet wrote: > We have a large number of users of `-Werror -Wthread-safety-analysis` > internally. When we make the new warnings part of that flag we cannot > integrate because we're breaking all the

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

2023-10-10 Thread Eduard Zingerman via Phabricator via cfe-commits
eddyz87 added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3791 + if (hasBPFPreserveStaticOffset(Base)) +addr = wrapWithBPFPreserveStaticOffset(CGF, addr); + ast wrote: > eddyz87 wrote: > > ast wrote: > > > eddyz87 wrote: > > > > ast wrote: > >

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

2023-10-10 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast accepted this revision. ast added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3791 + if (hasBPFPreserveStaticOffset(Base)) +addr = wrapWithBPFPreserveStaticOffset(CGF, addr); + eddyz87 wrote: > ast wrote: > > eddyz87 wrote: > > > ast wrote:

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

2023-10-10 Thread Eduard Zingerman via Phabricator via cfe-commits
eddyz87 added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3791 + if (hasBPFPreserveStaticOffset(Base)) +addr = wrapWithBPFPreserveStaticOffset(CGF, addr); + ast wrote: > eddyz87 wrote: > > ast wrote: > > > If I'm reading this correctly wrapping

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

2023-10-10 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3791 + if (hasBPFPreserveStaticOffset(Base)) +addr = wrapWithBPFPreserveStaticOffset(CGF, addr); + eddyz87 wrote: > ast wrote: > > If I'm reading this correctly wrapping with preserve_static

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

2023-10-10 Thread Eduard Zingerman via Phabricator via cfe-commits
eddyz87 marked an inline comment as done. eddyz87 added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3791 + if (hasBPFPreserveStaticOffset(Base)) +addr = wrapWithBPFPreserveStaticOffset(CGF, addr); + ast wrote: > If I'm reading this correctly wr

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

2023-10-10 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3791 + if (hasBPFPreserveStaticOffset(Base)) +addr = wrapWithBPFPreserveStaticOffset(CGF, addr); + If I'm reading this correctly wrapping with preserve_static_offset doesn't prevent further

[PATCH] D145214: [TSAN] add support for riscv64

2023-10-10 Thread Aditya Kumar via Phabricator via cfe-commits
hiraditya added a comment. nvm i found the file in one of the previous commits. https://github.com/llvm/llvm-project/pull/68735 for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145214/new/ https://reviews.llvm.org/D145214 ___

[PATCH] D158266: [OpenMP] Patch for Support to loop bind clause : Checking Parent Region

2023-10-10 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:6181-6190 + checkNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion, +BindKind, StartLoc); Kind = OMPD_for; DSAStack->setCurrentDirective(OMPD_for);

[PATCH] D158266: [OpenMP] Patch for Support to loop bind clause : Checking Parent Region

2023-10-10 Thread Sunil K via Phabricator via cfe-commits
koops added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:6181-6190 + checkNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion, +BindKind, StartLoc); Kind = OMPD_for; DSAStack->setCurrentDirective(OMPD_for);

[PATCH] D145214: [TSAN] add support for riscv64

2023-10-10 Thread Aditya Kumar via Phabricator via cfe-commits
hiraditya added a comment. @alexfanqi can you please upload the file (tsan_rtl_riscv64.S) with the patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145214/new/ https://reviews.llvm.org/D145214 ___ cf

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

2023-10-10 Thread Jared Grubb via Phabricator via cfe-commits
jaredgrubb updated this revision to Diff 557674. jaredgrubb added a comment. Addressed review feedback from Oct2 (@owenpan). A new issue on Github was opened for this enhancement (https://github.com/llvm/llvm-project/issues/68722) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

2023-10-10 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D155688#4653520 , @fiigii wrote: >> The reverse transform is only done if A + B simplifies. > > Looks like`simplifyAddInst` may give add expressions, so I guess this patch > may make IC run into infinite loops. simplifyAddInst

[PATCH] D155850: [HIP][Clang][CodeGen][RFC] Add codegen support for C++ Parallel Algorithm Offload

2023-10-10 Thread Alex Voicu via Phabricator via cfe-commits
AlexVlx updated this revision to Diff 557673. AlexVlx added a comment. Use unmangled names in test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155850/new/ https://reviews.llvm.org/D155850 Files: clang/lib/CodeGen/BackendUtil.cpp clang/lib/CodeGen/CGBuiltin.cpp clang/lib/CodeGe

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

2023-10-10 Thread Dmitriy Smirnov via Phabricator via cfe-commits
d-smirnov added a comment. We have some improvements with the patch, most notable: 549.fotonik_3d improves about 6%. @nikic Should we revert the patch and try another location for it (in LICM pass, as you previously suggested)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155850: [HIP][Clang][CodeGen][RFC] Add codegen support for C++ Parallel Algorithm Offload

2023-10-10 Thread Alex Voicu via Phabricator via cfe-commits
AlexVlx updated this revision to Diff 557672. AlexVlx removed reviewers: tra, jlebar. AlexVlx added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155850/new/ https://reviews.llvm.org/D155850 Files: clang/lib/CodeGen/BackendUtil.cpp clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-10 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:42 const Descriptor *D) { + new (Ptr) InitMapPtr(std::nullopt); + aaron.ballman wrote: > This worries me a little bit for a few reasons, but it might be okay:

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-10 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 557670. tbaeder marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154581/new/ https://reviews.llvm.org/D154581 Files: clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/lib/AST/Interp/EvalE

[PATCH] D155548: [clang][ExprConst] Short-circuit ConstantExpr evaluation

2023-10-10 Thread Timm Bäder via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb4343aba9fa1: [clang][ExprConst] Short-circuit ConstantExpr evaluation (authored by tbaeder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155548/new/ http

[PATCH] D156212: [clang][Interp] Implement remaining strcmp builtins

2023-10-10 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. I have a local patch that implements these builtins by doing a bitcast to a buffer first. There is a comment in `ExprConstant.cpp` that talks about the same thing. That path probably makes more sense? I'd abandon this review then. Repository: rG LLVM Github Monorepo

[PATCH] D156212: [clang][Interp] Implement remaining strcmp builtins

2023-10-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. There are still unaddressed comment here. It would be nice to complete this before phab shuts down Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156212/new/ https://reviews.llvm.org/D156212 __

[PATCH] D159351: [Sema] Change order of displayed overloads in diagnostics

2023-10-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a subscriber: aaron.ballman. cor3ntin added a comment. + @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159351/new/ https://reviews.llvm.org/D159351 ___ cfe-commits mailing l

[PATCH] D158156: [analyzer] Add C++ array delete checker

2023-10-10 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG71ae858c079f: [clang][analyzer] Rename DeleteWithNonVirtualDtorChecker to CXXDeleteChecker (authored by Viktor Cseh ). Changed prior to commit: https://reviews.llvm.org/D15815

[PATCH] D154951: [clang][Interp] __builtin_bit_cast, Take 2

2023-10-10 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder abandoned this revision. tbaeder added a comment. Abandoning this since I've migrated it to Github: https://github.com/llvm/llvm-project/pull/68288 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154951/new/ https://reviews.llvm.org/D154951

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-09 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment. In D153131#4653456 , @aaronpuchert wrote: > In D153131#4653412 , @courbet wrote: > >> I also had some push back internally on adding this to the existing flag. >> I'm going to add `-Wthr

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

2023-10-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 557659. MaskRay added a comment. test rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152279/new/ https://reviews.llvm.org/D152279 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/RISCV

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-10-09 Thread Sergei Barannikov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG79f87be6888d: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc (authored by barannikov88). Changed prior to commit: https://reviews.llvm.org/D159393?vs=557644&id=557657#toc Reposito

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

2023-10-09 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148474/new/ https://reviews.llvm.org/D148474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-09 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 557656. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158069/new/ https://reviews.llvm.org/D158069 Files: clang/lib/AST/Interp/Context.h clang/lib/AST/Interp/Pointer.h clang/unittests/AST/CMakeLists.txt clang/unittests/AST/Interp/CMakeLists.t

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

2023-10-09 Thread Trevor Gross via Phabricator via cfe-commits
tmgross accepted this revision. tmgross added a comment. Tested that this patch applied on top of `main` fixes all i128 ABI issues among gcc, clang, and rustc. Probably would be good to add https://bugs.llvm.org/show_bug.cgi?id=50198 to the test suite if it isn't there already. Thanks for stic

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

2023-10-09 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. Explicitly still ok with this as well. Thanks for continuing here. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86310/new/ https://reviews.llvm.org/D86310 _

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

2023-10-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Given the complexity here, I agree this is probably the best we can reasonably do. Code and test changes LGTM. That said, this is missing a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86310/new/ https://

[PATCH] D158266: [OpenMP] Patch for Support to loop bind clause : Checking Parent Region

2023-10-09 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:6181-6190 + checkNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion, +BindKind, StartLoc); Kind = OMPD_for; DSAStack->setCurrentDirective(OMPD_for);

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

2023-10-09 Thread Fei Peng via Phabricator via cfe-commits
fiigii added a comment. > The reverse transform is only done if A + B simplifies. Looks like`simplifyAddInst` may give add expressions, so I guess this patch may make IC run into infinite loops. Additionally, this change could make longer GEP chains that could hurt other optimizations by excee

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. If people are passing `-Wthread-safety-reference`, there was clearly some value in the previous checks and it would be unfortunate to turn them off while fixing the codebase. I'm not super familiar with flag families and if what I'm proposing is easily doable, but I th

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

2023-10-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I re-read the code review, and I think most folks are in favor of this change, but I may have missed some. Many concerns were raised, so please wait for approval from @efriedma as well before landin

[PATCH] D159436: [clang-tidy] Add support for optional parameters in config.

2023-10-09 Thread Piotr Zegar via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7a73da4c85a1: [clang-tidy] Add support for optional parameters in config. (authored by felix642, committed by PiotrZSL). Changed prior to commit: https://reviews.llvm.org/D159436?vs=557546&id=557652#toc

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

2023-10-09 Thread Jacek Caban via Phabricator via cfe-commits
jacek added a comment. In D157547#4653477 , @efriedma wrote: > How important is that particular pattern? I think most patterns involving > assembly should be covered by some combination of naked functions, and > functions defined in separate assembly f

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

2023-10-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > For assembly, I'm not really comfortable with the fix you're proposing; it > relies on the order in which functions are defined in assembly. I think LLVM > usually ends up emitting module-level inline assembly before generated code, > but it seems fragile to depend o

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

2023-10-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma updated this revision to Diff 557651. efriedma added a comment. Fix the calling convention for functions returning C++ classes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157547/new/ https://reviews.llvm.org/D157547 Files: clang/lib/

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

2023-10-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. For assembly, I'm not really comfortable with the fix you're proposing; it relies on the order in which functions are defined in assembly. I think LLVM usually ends up emitting module-level inline assembly before generated code, but it seems fragile to depend on that

[PATCH] D159045: [clang-tidy] Improved documentation for readability-function-size

2023-10-09 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL closed this revision. PiotrZSL added a comment. Obsolete. No longer needed after D159436 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159045/new/ https://reviews.llvm.org/D159045 ___

[PATCH] D157331: [clang] Implement C23

2023-10-09 Thread Zijun Zhao via Phabricator via cfe-commits
ZijunZhao updated this revision to Diff 557650. ZijunZhao added a comment. Add the blank lines back and then rerun the utc script Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157331/new/ https://reviews.llvm.org/D157331 Files: clang/docs/Relea

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. @cjdb -- how would you like to proceed with this review? There's already quite a bit of discussion on the current Phab patch set, but it seems unlikely that we'd be able to wrap everything up by the Nov 15th deadline. But then again, GitHub has no ability to do st

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

2023-10-09 Thread Jacek Caban via Phabricator via cfe-commits
jacek added a comment. It works nice with my testing so far, thanks! I noticed one problem with code that defines symbols in assembly. It conflicts with anti-dependency symbols emitted by codegen and hits an asserts in assembly printer. This commit fixes it for me: https://github.com/cjacek/llv

[PATCH] D155548: [clang][ExprConst] Short-circuit ConstantExpr evaluation

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. In D155548#4653363 , @cor3ntin wrote: > @aaron.ballman Given the recent discussion we had on both github > (https://github.com/llvm/llvm-project/pull/66203) and privately, I'm going to

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

2023-10-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. In D137149#4653381 , @vchuravy wrote: > In D137149#4653308 , @aeubanks > wrote: > >> we don't currently support reusing a pipeline so I'm surprised that you're >> able to share/reuse pi

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D140828#4653389 , @nikic wrote: > FYI this causes some compile-time regression: > http://llvm-compile-time-tracker.com/compare.php?from=bc7d88faf1a595ab59952a2054418cdd0d98&to=af4751738db89a142a8880c782d12d4201b222a8

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-09 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D153131#4653412 , @courbet wrote: > I also had some push back internally on adding this to the existing flag. I'm > going to add `-Wthread-safety-reference-return`, can we start by not > temporarily including it in `-Wth

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Pointer.h:81-88 + /// Equality operators are just for tests. + bool operator==(const Pointer &P) const { +return Pointee == P.Pointee && Base == P.Base && Offset == P.Offset; + } + + bool operator!=(con

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added subscribers: hubert.reinterpretcast, rsmith. aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:42 const Descriptor *D) { + new (Ptr) InitMapPtr(std::nullopt); + This worries me a lit

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-09 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments. Comment at: clang/lib/AST/Interp/Pointer.h:81-88 + /// Equality operators are just for tests. + bool operator==(const Pointer &P) const { +return Pointee == P.Pointee && Base == P.Base && Offset == P.Offset; + } + + bool operator!=(const Poi

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM -- that release note is *awesome*, thank you for that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159393/new/ https://reviews.llvm.org/D159393 ___

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

2023-10-09 Thread Roman via Phabricator via cfe-commits
kekcheburec added a comment. In D70401#4653409 , @zixuan-wu wrote: > ping? Pong 😂 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70401/new/ https://reviews.llvm.org/D70401 _

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

2023-10-09 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D155688#4653347 , @fiigii wrote: > How does this patch work with `visitGEPOfGEP` that does a reverse > transformation? > > // Replace: gep (gep %P, long B), long A, ... > // With:T = long A+B; gep %P, T, ... The reverse

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-09 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment. In D153131#4653362 , @aaronpuchert wrote: > In D153131#4653345 , @aeubanks > wrote: > >> This is finding lots of real issues in code, which is awesome, but could I >> request that this

[PATCH] D157252: [clang][ExprConst] Handle 0 type size in builtin_memcpy etc.

2023-10-09 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157252/new/ https://reviews.llvm.org/D157252 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-09 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154581/new/ https://reviews.llvm.org/D154581 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

2023-10-08 Thread Zixuan Wu via Phabricator via cfe-commits
zixuan-wu added a comment. ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70401/new/ https://reviews.llvm.org/D70401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

2023-10-08 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added inline comments. Comment at: llvm/lib/IR/AutoUpgrade.cpp:5233 + SmallVector Groups; + Regex R("(.*-i64:64)(-.*)"); + if (R.match(Res, &Groups)) hvdijk wrote: > nikic wrote: > > I don't think this will work for the 32-bit targets that d

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-10-08 Thread Sergei Barannikov via Phabricator via cfe-commits
barannikov88 updated this revision to Diff 557644. barannikov88 added a comment. Add a hyphen Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159393/new/ https://reviews.llvm.org/D159393 Files: clang/docs/ReleaseNotes.rst clang/test/Preprocessor

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-10-08 Thread Sergei Barannikov via Phabricator via cfe-commits
barannikov88 added a comment. @aaron.ballman I added a release note as requested. Please see if it looks the way it should. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159393/new/ https://reviews.llvm.org/D159393 ___

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-10-08 Thread Sergei Barannikov via Phabricator via cfe-commits
barannikov88 updated this revision to Diff 557643. barannikov88 added a comment. Add a release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159393/new/ https://reviews.llvm.org/D159393 Files: clang/docs/ReleaseNotes.rst clang/test/Prepro

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-10-08 Thread Sergei Barannikov via Phabricator via cfe-commits
barannikov88 updated this revision to Diff 557642. barannikov88 added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159393/new/ https://reviews.llvm.org/D159393 Files: clang/test/Preprocessor/has_attribute.c clang/test/Prepro

[PATCH] D154893: [Clang] Fix some triviality computations

2023-10-08 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. I, uh, got preoccupied by some recent events. Don't think I'll have the time for this unfortunately. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154893/new/ https://reviews.llvm.org/D154893 _

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-10-08 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. FYI this causes some compile-time regression: http://llvm-compile-time-tracker.com/compare.php?from=bc7d88faf1a595ab59952a2054418cdd0d98&to=af4751738db89a142a8880c782d12d4201b222a8&stat=instructions:u About 0.7% for C++ code at `O0`. Sample for a file with >2% would be

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

2023-10-07 Thread CS via Phabricator via cfe-commits
CS added a comment. Herald added a subscriber: manas. Herald added a project: All. I'm sorry but is this some sort of joke? I don't think we should be calling ChickenFajitas straightaway like this, debounce it, recalculate, repent let's go Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

2023-10-07 Thread Valentin Churavy via Phabricator via cfe-commits
vchuravy added a subscriber: loladiro. vchuravy added a comment. In D137149#4653308 , @aeubanks wrote: > we don't currently support reusing a pipeline so I'm surprised that you're > able to share/reuse pipelines without running into any issues In additi

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

2023-10-07 Thread Prem Chintalapudi via Phabricator via cfe-commits
pchintalapudi added a comment. In D137149#4653308 , @aeubanks wrote: > we don't currently support reusing a pipeline so I'm surprised that you're > able to share/reuse pipelines without running into any issues The way I see it, any pipeline state that c

[PATCH] D155548: [clang][ExprConst] Short-circuit ConstantExpr evaluation

2023-10-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. @aaron.ballman Given the recent discussion we had on both github (https://github.com/llvm/llvm-project/pull/66203) and privately, I'm going to accept that. We only create ConstantExpr for

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-07 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D153131#4653345 , @aeubanks wrote: > This is finding lots of real issues in code, which is awesome, but could I > request that this be put under a separate warning flag so we can toggle off > just the new functionality a

[PATCH] D158266: [OpenMP] Patch for Support to loop bind clause : Checking Parent Region

2023-10-07 Thread Sunil K via Phabricator via cfe-commits
koops updated this revision to Diff 557639. koops added a comment. Avoiding executing "checkNestingOfRegions( )" multiple times. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158266/new/ https://reviews.llvm.org/D158266 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaOpen

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

2023-10-06 Thread Fei Peng via Phabricator via cfe-commits
fiigii added a comment. How does this patch work with `visitGEPOfGEP` that does a reverse transformation? // Replace: gep (gep %P, long B), long A, ... // With:T = long A+B; gep %P, T, ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1556

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-06 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. This is finding lots of real issues in code, which is awesome, but could I request that this be put under a separate warning flag so we can toggle off just the new functionality and turn it on as we clean our codebase? Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D157331: [clang] Implement C23

2023-10-06 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added inline comments. Comment at: clang/test/C/C2x/n2683_2.c:5-6 +#include +#include +// CHECK-LABEL: define dso_local void @test_add_overflow_to64( +// CHECK-SAME: ) #[[ATTR0:[0-9]+]] { jrtc27 wrote: > UTC won't insert such a blank line if it doesn't e

[PATCH] D145214: [TSAN] add support for riscv64

2023-10-06 Thread Haowei Wu via Phabricator via cfe-commits
haowei added a comment. Hi, I believe you forgot to add `tsan_rtl_riscv64.S` to your patch when you land it. We are seeing build errors on our bots with following messages: -- Configuring done (12.3s) CMake Error at /b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cm

<    1   2   3   4   5   6   7   8   9   10   >