[PATCH] D50771: [clang-tblgen] Add -print-records and -dump-json modes.

nhaehnle accepted this revision. nhaehnle added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D50771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D38770: AMDGPU: Use stricter bounds for workitem builtins

nhaehnle accepted this revision. nhaehnle added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D38770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D44426: Fix llvm + clang build with Intel compiler

nhaehnle added a comment. I don't think we should add workarounds for broken compilers. https://reviews.llvm.org/D44426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44426: Fix llvm + clang build with Intel compiler

nhaehnle added a comment. Is that compiler really supported? Look at this: void operator delete(void *) = delete; It's been there in the code since early 2015. The bitwise OR on ELF::xxx has been there even longer. Are you telling me that we officially support a compiler, but nobody has ac

[PATCH] D51223: Update tests for new YAMLIO polymorphic traits

nhaehnle added a subscriber: tpr. nhaehnle added a comment. Adding @tpr as a subscriber due to the (admittedly maybe a bit indirect) MsgPack connection. Repository: rC Clang https://reviews.llvm.org/D51223 ___ cfe-commits mailing list cfe-commit

[PATCH] D69498: IR: Invert convergent attribute handling

nhaehnle added a comment. As you know, I am very much in favor of this change, and really anything that re-establishes the rule that code is treated maximally conservatively when there are no attributes or metadata. There is one slight concern here because what we arguably need in SIMT targets

[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 https://reviews.llvm.org/D81728/ne

[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 fee

[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] 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 ver

[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, sha

[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: D8308

[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, sha

[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/ https://reviews.ll

[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 CHANG

[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 i

[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/ https://reviews.ll

[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 SINCE

[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/ htt

[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. + assert(!m_instr-

[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 he

[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/ https://revie

[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, hira

[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 o

[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 stran

[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] 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] 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: > llvm_unreac

[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. The

[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 able

[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 way

[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. Her

[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, Joons

[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 https://reviews.llvm.org/D

[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 ManagedSt

[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 t

[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 File

[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 bolt/tools/driver/llvm-bolt.cp

[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] 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 File

[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 bolt/tools/merge-fdata/merge-fdat

[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 (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[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 ACT

[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 ACT

[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: https://reviews.llvm.org

[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 > (https://repo.or.cz/isl.git/blob/29

[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 bolt/tools/merge-

[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 w

[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 refers

[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] 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 #if's (authored by nhaehnle). Changed prior to

[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

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" > "build\llvm\tools\clan

[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, I

[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] 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] 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] 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] 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 co

[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] 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 https://revi

[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 gene

[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 inh

[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: clang/i

[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 rea

[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: llvm/include/llvm/Support/CfgT

[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 A

[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 opaqu

[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 cfe-commits@lis

[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 cfe-commits@l

[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 https://rev

[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] 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. 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 you

[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 https://l

[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] 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 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 h

[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 https://reviews.llvm.org/D134637/ne

[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 subscribe

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

[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 https://reviews.llvm.

[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 cla

[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 unconditional

[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] 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/ https://reviews.llvm.or

[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, a

[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] 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 c

[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 `GlobalV

[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 cfe-commits@lists

[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, rriddle,

[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] 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 L

[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] 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 acces

[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

nhaehnle created this revision. nhaehnle added a reviewer: rjmccall. Herald added a project: clang. Attribute annotations are recorded in a special global composite variable that points to annotation strings and the annotated objects. As a restriction of the LLVM IR type system, those pointers ar

[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

nhaehnle added a comment. In D71208#1776202 , @rjmccall wrote: > The questions I'd like to have answered before I can approve this are: > > - whether there are clients of `@llvm.global.annotations` that will have > problems with non-0 address spaces and

[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

nhaehnle added a comment. > My concern is that there's something that's going to blow up or miscompile if > we start passing in constants that aren't in a regular address space. Aren't > there kinds of annotations which get persisted into the emitted code? Annotations don't seem to be used for

[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

This revision was automatically updated to reflect the committed changes. Closed by commit rGf21c081b78ec: CodeGen: Allow annotations on globals in non-zero address space (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71208/new

  1   2   >