[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

nhaehnle abandoned this revision. nhaehnle added a comment. Indeed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86154/new/ https://reviews.llvm.org/D86154 ___ cfe-commits mailing list

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

nhaehnle added a comment. Herald added a subscriber: wangpc. I haven't looked at this in a while, sorry. I do plan to cycle back to it eventually, but right now my work scheduling is more stack-based than FIFO and this is fairly low down... Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

nhaehnle added a comment. In D145441#4175428 , @krzysz00 wrote: > @foad I was trying to avoid sending in one mega-patch that has the entire > prototype. The best way to do this is to work on a branch that you rebase regularly. You can push the branch

[PATCH] D143522: [AMDGPU] Set a data layout entry for buffer descriptors (addrspace 7)

nhaehnle added a comment. This really needs to be a 160-bit type, not a 128-bit type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143522/new/ https://reviews.llvm.org/D143522 ___ cfe-commits mailing

[PATCH] D143318: [Support] Move ItaniumManglingCanonicalizer and SymbolRemappingReader from Support to ProfileData

nhaehnle added a comment. This (moving to ProfileData) seems like a simple and pragmatic solution for a (perhaps not gigantic but) real problem. It feels fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143318/new/

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

nhaehnle created this revision. Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini,

[PATCH] D138276: TableGen: require tablegen cl::opts to be registered explicitly

nhaehnle created this revision. Herald added subscribers: libc-commits, Moerafaat, zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen,

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

This revision was automatically updated to reflect the committed changes. Closed by commit rGdce78646f07f: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle added a comment. Ping^2 after ~a week. The previous discussion seemed quite favorable, so is this just a case of everybody being too afraid to say "Yes" because this is a dark corner that few people ever look at? ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle added a comment. Gentle ping :) If there are no objections, I'd like to commit this in a few days. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134637/new/ https://reviews.llvm.org/D134637

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle updated this revision to Diff 467550. nhaehnle added a comment. - use the object library version of clangSupport if available Thank you for the pointers. Using this object library makes a lot of sense to me and it does appear to work. Judging by AddClang.cmake though, the object library

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle added a comment. Thank you all for the reviews. I've integrated the suggestions except for: > A possible alternative solution would be to build clangSupport_sources as an > object library, and then link that library into clangSupport and clang-tblgen > which could be done

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle updated this revision to Diff 465284. nhaehnle added a comment. Remove the confusing "accidentally" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134637/new/ https://reviews.llvm.org/D134637 Files: clang/lib/Support/CMakeLists.txt

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle updated this revision to Diff 465283. nhaehnle added a comment. - define an alias so that clang-tblgen can always link against "clangSupport_tablegen" - use add_llvm_library(... BUILDTREE_ONLY ...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle added inline comments. Comment at: clang/lib/Support/CMakeLists.txt:21 + # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't + # accidentally link against libLLVMSupport twice (once statically and once via + # libLLVM-*.so).

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

nhaehnle created this revision. nhaehnle added reviewers: kito-cheng, khchen, MaskRay, aaron.ballman, DavidSpickett. Herald added subscribers: StephenFan, kristof.beyls. Herald added a project: All. nhaehnle requested review of this revision. Herald added a project: clang. Herald added a

[PATCH] D129131: Remove uses of llvm_shutdown

nhaehnle updated this revision to Diff 449609. nhaehnle added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129131/new/ https://reviews.llvm.org/D129131 Files: bolt/tools/driver/llvm-bolt.cpp

[PATCH] D129131: Remove uses of llvm_shutdown

nhaehnle added inline comments. Comment at: polly/lib/External/isl/interface/extract_interface.cc:590 delete Clang; - llvm::llvm_shutdown(); Meinersbur wrote: > This file is imported from the upstream project >

[PATCH] D129118: CommandLine: add and use cl::SubCommand::get{All,TopLevel}

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf7872cdce110: CommandLine: add and use cl::SubCommand::get{All,TopLevel} (authored by nhaehnle). Changed prior to commit:

[PATCH] D130576: ManagedStatic: remove from ASTMatchersInternal.h

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7132bcdc428d: ManagedStatic: remove from ASTMatchersInternal.h (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D130575: clang: include ManagedStatic.h for llvm_shutdown

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5a78ac21569a: clang: include ManagedStatic.h for llvm_shutdown (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D130574: ClangLinkerWrapper: explicitly #include

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3e874bcf0642: ClangLinkerWrapper: explicitly #include atomic (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D129131: Remove uses of llvm_shutdown

nhaehnle updated this revision to Diff 447708. nhaehnle added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129131/new/ https://reviews.llvm.org/D129131 Files: bolt/tools/driver/llvm-bolt.cpp

[PATCH] D130578: ManagedStatic: remove a use from Clang

nhaehnle created this revision. nhaehnle added reviewers: efriedma, lattner. Herald added a project: All. nhaehnle requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D130578

[PATCH] D130577: clang-driver: use llvm_fast_shutdown

nhaehnle created this revision. nhaehnle added reviewers: efriedma, lattner, mehdi_amini. Herald added a project: All. nhaehnle requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Avoid running almost all global destructors in the common case.

[PATCH] D129118: CommandLine: add and use cl::SubCommand::get{All,TopLevel}

nhaehnle updated this revision to Diff 447694. nhaehnle added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129118/new/ https://reviews.llvm.org/D129118 Files: bolt/lib/Utils/CommandLineOpts.cpp

[PATCH] D130576: ManagedStatic: remove from ASTMatchersInternal.h

nhaehnle created this revision. nhaehnle added reviewers: efriedma, lattner. Herald added a project: All. nhaehnle requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D130576

[PATCH] D130575: clang: include ManagedStatic.h for llvm_shutdown

nhaehnle created this revision. nhaehnle added reviewers: efriedma, lattner. Herald added a project: All. nhaehnle requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The code relied on ManagedStatic.h being included indirectly. This is about

[PATCH] D130574: ClangLinkerWrapper: explicitly #include

nhaehnle created this revision. nhaehnle added reviewers: efriedma, lattner. Herald added a project: All. nhaehnle requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This code relied on implicitly having std::atomic available via the

[PATCH] D130406: Use llvm::sort instead of std::sort where possible

nhaehnle accepted this revision. nhaehnle 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/D130406/new/ https://reviews.llvm.org/D130406

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

nhaehnle added a comment. In D130224#3668225 , @aaron.ballman wrote: > I'm in C standards meetings this week and don't have a lot of ability to > thoroughly review this yet, but the most immediate concern which springs to > mind for me is that this is

[PATCH] D130089: update-test-checks: safely handle tests with #if's

nhaehnle added a comment. My version of `diff` has a `--strip-trailing-cr` flag. But I don't really see that being used in many places at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130089/new/ https://reviews.llvm.org/D130089

[PATCH] D130089: update-test-checks: safely handle tests with #if's

nhaehnle added a comment. I've been staring at ifdef.test vs. basic-cplusplus.test for a while now and don't see any relevant difference, including when I look at the line endings in a hex editor (on Linux). So I guess there is a difference that appears only on Windows somehow? Unfortunately,

[PATCH] D130089: update-test-checks: safely handle tests with #if's

nhaehnle added a comment. In D130089#3666076 , @mizvekov wrote: > This seems to have introduced a test which always fails on windows: > > $ ":" "RUN: at line 4" > $ "cp" "clang\test\utils\update_cc_test_checks/Inputs/ifdef.c" >

[PATCH] D130089: update-test-checks: safely handle tests with #if's

nhaehnle added inline comments. Comment at: llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll:2 ; Check that we accept functions with '$' in the name. ; TODO: This is not handled correcly on 32bit ARM and needs to be fixed.

[PATCH] D130089: update-test-checks: safely handle tests with #if's

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. nhaehnle marked an inline comment as done. Closed by commit rG5a4033c36716: update-test-checks: safely handle tests with #ifs (authored by nhaehnle). Changed prior to

[PATCH] D130089: update-test-checks: safely handle tests with #if's

nhaehnle created this revision. nhaehnle added reviewers: MaskRay, arsenm, aemerson, arichardson. Herald added subscribers: StephenFan, kristof.beyls. Herald added a project: All. nhaehnle requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added projects: clang,

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable

nhaehnle added a comment. I can't judge the Clang changes. On the LLVM side, can you also add the implementation and a test for the GlobalISel path? Plus, I have some minor inline comments. Comment at: llvm/docs/LangRef.rst:24546 + +The first argument is pointer, which

[PATCH] D128166: ManagedStatic: Destroy from destructor

nhaehnle added a comment. Herald added a subscriber: anlunx. Abandoning this in favor of removing ManagedStatic entirely via the stack of changes that ends in D129134 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129131: Remove uses of llvm_shutdown

nhaehnle created this revision. nhaehnle added reviewers: efriedma, lattner. Herald added a reviewer: bollu. Herald added subscribers: anlunx, bzcheeseman, ayermolo, sdasgup3, wenzhicui, wrengr, dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, jvesely,

[PATCH] D129118: CommandLine: add and use cl::SubCommand::get{All,TopLevel}

nhaehnle created this revision. nhaehnle added reviewers: efriedma, lattner. Herald added subscribers: ayermolo, hiraditya. Herald added a reviewer: rafauler. Herald added a reviewer: Amir. Herald added a reviewer: maksfb. Herald added a project: All. nhaehnle requested review of this revision.

[PATCH] D128166: ManagedStatic: Destroy from destructor

nhaehnle created this revision. Herald added a reviewer: deadalnix. Herald added subscribers: bzcheeseman, ayermolo, sdasgup3, wenzhicui, wrengr, dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, jvesely, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob,

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

nhaehnle added inline comments. Comment at: llvm/include/llvm/IR/IRBuilder.h:746-747 + /// Create a call to llvm.threadlocal.address intrinsic. + CallInst *CreateThreadLocalAddress(Value *Ptr); + ChuanqiXu wrote: > nhaehnle wrote: > > This could be a

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

nhaehnle added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1393-1394 +def int_threadlocal_address : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty], +[IntrNoMem, IntrWillReturn]>; + Whether IntrNoMem is truly

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

nhaehnle added a comment. You can use the "Edit Related Revisions" option in the right-hand side menu of Phabricator to link this revision with the others of the series. I can't really speak for the Clang parts, but the LLVM parts looks reasonable to me modulo some detail comments.

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

nhaehnle requested changes to this revision. nhaehnle added a comment. I'm sorry to say that this patch seems confused about semantics. It lacks clear definitions, and in particular for the `shuffle` attribute in LLVM IR, you almost certainly just want the already existing `convergent` instead.

[PATCH] D124158: [Clang][Attr] Skip adding noundef attribute to arguments when function has convergent attribute

nhaehnle added a comment. In D124158#3486110 , @jdoerfert wrote: > I agree. As far as I can tell you have two options, both are specific to the > shuffle functions: > > 1. Do not set noundef for calls to them as they allow undef values for all > lanes

[PATCH] D69498: IR: Invert convergent attribute handling

nhaehnle added a comment. In D69498#2711396 , @foad wrote: > I don't have much to add to the conversation except to point out that this > definition defines `noconvergent` in terms of divergent control flow, but the > langref itself doesn't define what

[PATCH] D92661: [RFC] Fix TLS and Coroutine

nhaehnle added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1281-1282 // The pseudoprobe intrinsic works as a place holder to the block it probes. -// Like the sideeffect intrinsic defined above, this intrinsic is treated by the -// optimizer as having

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle abandoned this revision. nhaehnle added a comment. Herald added a subscriber: teijeong. Superseded by D92924 , D92925 , D92926 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. Herald added a subscriber: dexonsmith. I'm going to follow up with another RFC about this on llvm-dev. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83088/new/ https://reviews.llvm.org/D83088

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. I replied on llvm-dev. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83088/new/ https://reviews.llvm.org/D83088 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. Hi Mehdi, this is not an appropriate place for this discussion. Yes, we have a general rule that patches can be reverted if they're obviously broken (e.g. build bot problems) or clearly violate some other standard. This is a good rule, but it doesn't apply here. If

[PATCH] D89814: [TableGen] Change !getop and !setop to !getdagop and !setdagop

nhaehnle accepted this revision. nhaehnle 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/D89814/new/ https://reviews.llvm.org/D89814

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. David, I don't think this is appropriate here. Let's take the discussion to llvm-dev. Comment at: mlir/include/mlir/IR/Dominance.h:49 +template <> +struct llvm::CfgTraitsFor { + using CfgTraits = mlir::CfgTraits; antiagainst wrote:

[PATCH] D83088: Introduce CfgTraits abstraction

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc0cdd22c72fa: Introduce CfgTraits abstraction (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. Herald added a subscriber: rdzhabarov. ping^2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83088/new/ https://reviews.llvm.org/D83088 ___ cfe-commits mailing list

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. Herald added a subscriber: tatianashp. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83088/new/ https://reviews.llvm.org/D83088 ___ cfe-commits mailing list

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added inline comments. Comment at: llvm/include/llvm/Support/CfgTraits.h:51 + + operator bool() const { return ptr != nullptr; } + dblaikie wrote: > `operator bool` should be `explicit` Done. Comment at:

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

nhaehnle added a comment. >> ReplaceNodeResults expects the result type to be changed in semi-magical >> ways during vector type legalization, which is non-obvious since the method >> can be called from different places. I think it *could* be made to work with >> a lot of patience, but it's

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle updated this revision to Diff 287441. nhaehnle added a comment. - cleanup operators on CfgOpaqueType - address other review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83088/new/ https://reviews.llvm.org/D83088 Files:

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. The most immediate problem is divergence analysis, which is extremely complex and difficult to get right. If I had tried to fight the accidental complexity that comes with attempting to write such an algorithm as C++ templates in addition to the

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

nhaehnle added a comment. In D86154#2224272 , @arsenm wrote: > In D86154#2224270 , @nhaehnle wrote: > >> Note that part of my motivation here over D84639 >> is to support more

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

nhaehnle updated this revision to Diff 286898. nhaehnle added a comment. Don't duplicate the intrinsics. Rely on D86317 to reduce the pain of this change caused to downstream users. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. > Based on your description and the DomTree patches, if I understand correctly, > the primary motivation is to facilitate writing CFG-representation-agnostic > algorithms/analyses (e.g., dominators, divergence, convergence analyses), > such that you can later lift the

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. In D83088#2225415 , @dblaikie wrote: >>> But I guess coming back to the original/broader design: What problems is >>> this intended to solve? The inability to write non-template algorithms over >>> graphs? What cost does that

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. > Not sure that's the best place to be designing this fairly integral and > complicated piece of infrastructure from, but hoping we can find some good > places/solutions/etc. I sent an email to llvm-dev several weeks ago, but things seem to have moved here. Either

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

nhaehnle added a comment. Note that part of my motivation here over D84639 is to support more general types on the lane intrinsics, since they also express some semantic content which would be interesting to be able to express e.g. on descriptors. I wasn't

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

nhaehnle created this revision. nhaehnle added a reviewer: arsenm. Herald added subscribers: cfe-commits, kerbowa, jfb, hiraditya, t-tye, tpr, dstuttard, yaxunl, jvesely, kzhuravl. Herald added projects: clang, LLVM. nhaehnle requested review of this revision. Herald added a subscriber: wdng.

[PATCH] D85607: CfgTraits: add CfgInstructionRef

nhaehnle marked an inline comment as done. nhaehnle added inline comments. Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:100 +// provide one at all. We don't want to lay a subtle performance trap here. +abort(); + } arsenm wrote: >

[PATCH] D85607: CfgTraits: add CfgInstructionRef

nhaehnle updated this revision to Diff 285711. nhaehnle added a comment. - use llvm_unreachable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85607/new/ https://reviews.llvm.org/D85607 Files: clang/include/clang/Analysis/Analyses/Dominators.h

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. In D83088#2213886 , @dblaikie wrote: > In D83088#2213864 , @nhaehnle wrote: > >> In D83088#2213802 , @dblaikie wrote: >> >>> In D83088#2213797

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. In D83088#2213802 , @dblaikie wrote: > In D83088#2213797 , @nhaehnle wrote: > >> In D83088#2208611 , @dblaikie wrote: >> >>> This seems like a

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added a comment. In D83088#2208611 , @dblaikie wrote: > This seems like a strange hybrid between a static-polymorphism (with traits) > and dynamic polymorphism (with base classes/virtual functions). Could this > more readily be just one or the

[PATCH] D85607: CfgTraits: add CfgInstructionRef

nhaehnle created this revision. nhaehnle added reviewers: arsenm, foad, sameerds. Herald added subscribers: cfe-commits, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini, rogfer01, kuhar,

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle updated this revision to Diff 284195. nhaehnle marked 2 inline comments as done. nhaehnle added a comment. v7: - std::forward fix in wrapping_iterator - fix typos Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83088/new/

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle updated this revision to Diff 280481. nhaehnle marked an inline comment as done. nhaehnle added a comment. v6: - implement predecessors/successors for all CfgTraits implementations - fix error in unwrapRange - rename toGeneric/fromGeneric into wrapRef/unwrapRef to have naming

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle marked 3 inline comments as done. nhaehnle added inline comments. Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:44 +// use on a 32-bit architecture. +assert(wrapped != (uintptr_t)-1 && wrapped != (uintptr_t)-2); + arsenm wrote: > I

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nhaehnle accepted this revision. nhaehnle added a comment. This revision is now accepted and ready to land. This has had a month of good review that has been addressed, I'd say it's good to go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle marked an inline comment as done. nhaehnle added inline comments. Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:136-138 + // Prefer to avoid support for bundled instructions as long as we + // don't really need it. +

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

This revision was automatically updated to reflect the committed changes. Closed by commit rG3fa989d4fd6b: DomTree: remove explicit use of DomTreeNodeBase::iterator (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83087/new/

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

nhaehnle added a comment. In D83087#2134881 , @kuhar wrote: > modulo accidental formatting changes. I'm not aware of any. Some line breaks changed because "const_iterator" is longer than "iterator". Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D83084: DomTree: Remove the releaseMemory() method

This revision was automatically updated to reflect the committed changes. Closed by commit rG723a44c9b5d6: DomTree: Remove the releaseMemory() method (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83084/new/

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle updated this revision to Diff 275811. nhaehnle marked 4 inline comments as done. nhaehnle added a comment. - fix MachineCfgTraits::blockdef_iterator and allow it to iterate over the instructions in a bundle - use MachineBasicBlock::printName Repository: rG LLVM Github Monorepo

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle added inline comments. Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:133 + ++m_def; + if (m_def == m_instr->defs().end()) { +++m_instr; arsenm wrote: > != return early? The logic is actually subtly broken in the presence of

[PATCH] D83084: DomTree: Remove the releaseMemory() method

This revision was automatically updated to reflect the committed changes. Closed by commit rG723a44c9b5d6: DomTree: Remove the releaseMemory() method (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83084/new/

[PATCH] D83088: Introduce CfgTraits abstraction

nhaehnle created this revision. nhaehnle added reviewers: arsenm, RKSimon, mehdi_amini, courbet. Herald added subscribers: cfe-commits, msifontes, jurahul, Kayjukh, vkmr, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst,

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

nhaehnle created this revision. nhaehnle added reviewers: arsenm, RKSimon, mehdi_amini, courbet. Herald added subscribers: cfe-commits, msifontes, jurahul, Kayjukh, vkmr, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst,

[PATCH] D83084: DomTree: Remove the releaseMemory() method

nhaehnle created this revision. nhaehnle added reviewers: arsenm, RKSimon, mehdi_amini, courbet. Herald added subscribers: cfe-commits, wdng. Herald added projects: clang, LLVM. nhaehnle added a parent revision: D83083: DomTree: Remove getChildren() accessor. nhaehnle added a child revision:

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nhaehnle added inline comments. Comment at: llvm/include/llvm/Transforms/InstCombine/InstCombiner.h:46 +/// combine them. +class LLVM_LIBRARY_VISIBILITY InstCombiner { +public: lattner wrote: > I would really rather not make this be a public class - this is a

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nhaehnle added a comment. We've been handling target-specific intrinsics in InstCombine for a long time, and that's the place where they should naturally sit. This is a pretty clean refactoring in my opinion, I'm in favor. It's substantial enough as a change that it should probably receive a

[PATCH] D75661: Remove SequentialType from the type heirarchy.

nhaehnle added a comment. The AMDGPU changes LGTM, modulo a stylistic nitpick. As for the overall change, I'm perfectly fine with the general direction of SequentialType, though I don't know where we stand in terms of enough people having had a chance to look at it. I'm concerned though about

[PATCH] D75660: Remove CompositeType class

nhaehnle accepted this revision. nhaehnle added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75660/new/ https://reviews.llvm.org/D75660 ___ cfe-commits mailing list

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

nhaehnle added a comment. In D74935#1887245 , @efriedma wrote: > In D74935#1886020 , @nhaehnle wrote: > > > I find this phrasing pretty confusing. How about the following: > > > > > This indicates that objects

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

nhaehnle added a comment. I find this phrasing pretty confusing. How about the following: > This indicates that objects accessed via pointer values based on the argument > or return value are not **modified**, during the execution of the function, > via pointer values not based on the argument

[PATCH] D74787: [IRBuilder] Always respect inserter/folder

nhaehnle accepted this revision. nhaehnle added a comment. This revision is now accepted and ready to land. LGTM. I think it's a pretty reasonable idea to add this threshold check to track down things that should be minor, but can add up in terms of compile-time performance. Repository: rG

[PATCH] D74673: CGBuiltin: Remove uses of deprecated CreateCall overloads

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf197304a66a: CGBuiltin: Remove uses of deprecated CreateCall overloads (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74673/new/

[PATCH] D74673: CGBuiltin: Remove uses of deprecated CreateCall overloads

nhaehnle created this revision. nhaehnle added a reviewer: t.p.northover. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D74673 Files: clang/lib/CodeGen/CGBuiltin.cpp Index:

[PATCH] D74672: Fix various occurences of redundant std::move [gcc 9 -Wredundant-move]

nhaehnle created this revision. Herald added subscribers: cfe-commits, hiraditya. Herald added projects: clang, LLVM. There are hundreds (thousands?) of instances of this warning left. A blog post on the new warning in gcc 9 is in

[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

nhaehnle added a comment. In D71213#1781322 , @gchatelet wrote: > LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a > warning but it only works when building without `-Wall`. Did you mean to write _with_ -Wall? I fail to see

  1   2   >