[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli added inline comments. Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:52 + for (auto : Map) { +const auto = Def.second; +if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC")) barannikov88 wrote: > fpetrogalli wrote: > >

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli updated this revision to Diff 484564. fpetrogalli added a comment. Same NFC - restore an empty line change... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137517/new/ https://reviews.llvm.org/D137517 Files:

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli updated this revision to Diff 484563. fpetrogalli added a comment. NFC - I just restored an empty line to prevent modifying a file that is not being touched by the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137517/new/

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli updated this revision to Diff 484561. fpetrogalli added a comment. There is no need to use `target_include_directories(LLVMRISCVDesc PRIVATE ${LLVM_LIBRARY_DIR}/TargetParser/)` for components that are not related to the RISC-V target. Thank you @lenary for the suggestion!

[PATCH] D137838: [Support] Move TargetParsers to new component

fpetrogalli added a comment. In D137838#4010987 , @dblaikie wrote: > This has introduced a circular dependency due to the forwarding header > (forwarding header depends on new lib, new lib depends on support, where the > forwarding header is).

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

fpetrogalli added inline comments. Comment at: llvm/unittests/Support/Host.cpp:33 -class HostTest : public testing::Test { - Triple Host; - -protected: - bool isSupportedArchAndOS() { -// Initially this is only testing detection of the number of -// physical cores,

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

fpetrogalli accepted this revision. fpetrogalli added a comment. LGTM. Thank you @lenary Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137836/new/ https://reviews.llvm.org/D137836 ___ cfe-commits

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

fpetrogalli added inline comments. Comment at: llvm/unittests/Support/Host.cpp:33 -class HostTest : public testing::Test { - Triple Host; - -protected: - bool isSupportedArchAndOS() { -// Initially this is only testing detection of the number of -// physical cores,

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli added a comment. I intend to base this patch on D137838 once it goes through the major refactoring of bridging the header files. For the sake of simplicity, for now I'll keep it on top of D137516 . Repository:

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli updated this revision to Diff 474817. fpetrogalli added a comment. I have applied the chaged suggested by @jrtc27: 1. The classes `RISCVProcessorModelPROC` and `RISCVProcessorModelTUNE_PROC` now are derived from `ProcessorModel`; 2. The `Enum` field is removed from both classes.

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

fpetrogalli added a comment. @lenary - thank you for the update! I have added a bunch of miso comments, a bit repetitive... by the time I realised they were repetitive it was faster to get to the bottom of it than removing them! In D137838#3931295 ,

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

fpetrogalli accepted this revision. fpetrogalli added a comment. This revision is now accepted and ready to land. Thank you for working on this @lenary - LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/

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

fpetrogalli added a subscriber: mgorny. fpetrogalli added a comment. FWIW, the change in this patch solves the issue with standalone build of `clang` reported in https://reviews.llvm.org/rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf. (FYI, @mgorny ) Repository: rG LLVM Github Monorepo

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

fpetrogalli added a comment. @barannikov88 - Imam stuck with an incomplete explanation: The issue raised by @mgorny is about stand-alone builds. In this build configuration, clang is built separately from LLVM, which is first

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

fpetrogalli added a comment. In D141581#4056612 , @barannikov88 wrote: > In D141581#4056503 , @fpetrogalli > wrote: > >> This is because the sources of clangBasic and clangDriver might be compiled >> before

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

fpetrogalli added a comment. In D141581#4056492 , @tschuett wrote: > I though Clang Basic is a leaf library and must not depend on anything. I wasn't aware of this requirement. As of https://reviews.llvm.org/D137517 it depends on some auto-generated

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

fpetrogalli added a comment. In D141581#4056464 , @lebedev.ri wrote: > Why is it not sufficient to link to `RISCVTargetParserTableGen`, but is > sufficient to link to `LLVMTargetParser`? This is because the sources of clangBasic and clangDriver might

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

fpetrogalli added a comment. @jrtc27, @thakis, @craig.topper - gentle ping, it would be great if I could unlock @mgorny with this patch for the issue they are seeing at https://reviews.llvm.org/rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf Francesco Repository: rG LLVM Github Monorepo

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

fpetrogalli added a comment. In D141581#4056825 , @barannikov88 wrote: > clangBasic and clangDriver already have a dependency on TargetParser (see > LLVM_LINK_COMPONENTS at the beginning of corresponding files). Is that not > enough? > Will it build

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

fpetrogalli added a comment. Hi -has anybody any more concern on this change? I'd like to submit it as soon as possible to unlock @mgorny . Francesco Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141581/new/ https://reviews.llvm.org/D141581

[PATCH] D142403: [build] Fix stand-alone builds of clang.

fpetrogalli added a comment. In D142403#4075215 , @tstellar wrote: > LGTM. I tested and this fixes the build. @tstellar , I wonder how your build (and mine too, in fact) didn't complain about the typo... luckily the bots cover this functionality!

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

fpetrogalli updated this revision to Diff 491320. fpetrogalli added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, sstefan1. Herald added a project: LLVM. @mgorny, I have updated the patch according to the suggestions from @tstellar in

[PATCH] D141581: [build] Fix stand-alone builds of clang.

fpetrogalli added a comment. In D141581#4074312 , @tstellar wrote: > I tested this patch and it works, so I'll give it an LGTM. However, I think > the LLVMConfig.cmake.in changes are now unnecessary since the > RISCVTargetParserTableGen has been

[PATCH] D141581: [build] Fix stand-alone builds of clang.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG52bcdac3b842: [build] Fix stand-alone builds of clang. (authored by fpetrogalli). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141581: [build] Fix stand-alone builds of clang.

fpetrogalli added a comment. Ah - there you go. Without a proper chained dependency of LLVMTargetParser -> clangBasic we end up with failures if clangBasic is compiled before LLVMTargetParser is (even on non stand-alone builds). https://lab.llvm.org/buildbot/#/builders/193/builds/25362 :

[PATCH] D141581: [build] Fix stand-alone builds of clang.

fpetrogalli added a comment. Reverted in https://reviews.llvm.org/rG20ffb9aaf982 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141581/new/ https://reviews.llvm.org/D141581 ___ cfe-commits mailing list

[PATCH] D141581: [build] Fix stand-alone builds of clang.

fpetrogalli added inline comments. Comment at: clang/lib/Basic/CMakeLists.txt:113 omp_gen - RISCVTargetParserTableGen ) tstellar wrote: > Doesn't this break the monorepo (non-standalone) build? If the monorepo > build still works with this, then you

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

fpetrogalli added a comment. In D141581#4069078 , @tstellar wrote: > So, what's the actual dependency here? Do libBasic and libDriver just need > the header to be generated or does it actually need to link to the library? Just the header.

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

fpetrogalli added a comment. In D141581#4069123 , @tstellar wrote: > I don't think this is the correct way to specify dependencies if it's just an > issue of the header being included before a generated file it needs has been > generated. Are there

[PATCH] D142403: [build] Force building LLVMTargetParser before other non-LLVM components.

fpetrogalli created this revision. fpetrogalli added reviewers: mgorny, tstellar. Herald added subscribers: luismarques, s.egerton, kadircet, PkmX, simoncook, arichardson. Herald added a project: All. fpetrogalli requested review of this revision. Herald added subscribers: cfe-commits,

[PATCH] D142403: [build] Force building LLVMTargetParser before other non-LLVM components.

fpetrogalli added a comment. In D142403#4074885 , @tstellar wrote: > This is still the wrong change IMO. I don't know, maybe I'm not being clear, > but I don't 'think you ever actually tried my suggestion in D141581 >

[PATCH] D142403: [build] Force building LLVMTargetParser before other non-LLVM components.

fpetrogalli updated this revision to Diff 491506. fpetrogalli added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, sstefan1. Herald added a project: LLVM. Address code review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142403: [build] Fix stand-alone builds of clang.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5d02e38f56df: [build] Fix stand-alone builds of clang. (authored by fpetrogalli). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142403: [build] Fix stand-alone builds of clang.

fpetrogalli added a comment. In D142403#4075215 , @tstellar wrote: > LGTM. I tested and this fixes the build. Thank you @tstellar, I landed it. I am going offline for until tomorrow morning - feel free to revert it if the world of bots collapses :)

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

fpetrogalli added a comment. In D141581#4069053 , @tstellar wrote: > [...] > It's still not clear to me why LLVM_LINK_COMPONENTS does not work. Is > LLVMTargetParser a library? Yes, `LLVMTargetParser` is a library built located in

[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

fpetrogalli created this revision. Herald added subscribers: kosarev, foad, frasercrmck, kerbowa, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar,

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli created this revision. Herald added subscribers: kosarev, sunshaoce, VincentWu, foad, vkmr, frasercrmck, evandro, kerbowa, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27,

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli added a comment. RFC at RFC at https://discourse.llvm.org/t/targetparser-auto-generation-of-riscv-cpu-definitions/66419 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137517/new/ https://reviews.llvm.org/D137517

[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

fpetrogalli added a comment. RFC at https://discourse.llvm.org/t/targetparser-auto-generation-of-riscv-cpu-definitions/66419 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137516/new/ https://reviews.llvm.org/D137516

[PATCH] D137515: [TargetSupport] Move TargetParser API in a separate LLVM component.

fpetrogalli created this revision. Herald added subscribers: kosarev, foad, frasercrmck, kerbowa, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar,

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli updated this revision to Diff 473532. fpetrogalli added a comment. I updated the text of the header of the file `RISCVTargetDefEmitter.cpp`. NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137517/new/

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli updated this revision to Diff 473545. fpetrogalli added a comment. I have made the dependency on `RISCVTargetSupportTableGen` explicit in the `RISCVCodeGen` target. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137517/new/

[PATCH] D137515: [TargetSupport] Move TargetParser API in a separate LLVM component.

fpetrogalli abandoned this revision. fpetrogalli added a comment. Abandoning because I accidentally squashed the two commits together when using Arcanist. I'll follow up with two new revisions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli added inline comments. Comment at: llvm/lib/TargetSupport/CMakeLists.txt:2 + +set(LLVM_TARGET_DEFINITIONS ${CMAKE_SOURCE_DIR}/lib/Target/RISCV/RISCV.td) + tmatheson wrote: > Here `RISCVTargetSupportTableGen` depends on files in `lib/Target/RISCV`, >

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli added inline comments. Comment at: llvm/lib/Target/RISCV/RISCV.td:509 -def : ProcessorModel<"generic-rv32", NoSchedModel, [Feature32Bit]>; -def : ProcessorModel<"generic-rv64", NoSchedModel, [Feature64Bit]>; +class RISCVProcessorModelPROC { + string Enum = enum;

[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

fpetrogalli added a comment. In D137516#3912842 , @lenary wrote: > In D137516#3912751 , @fpetrogalli > wrote: > >> @arsenm @frasercrmck @lenary - thank you for the feedback >> >> 1. The library could host

[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

fpetrogalli added a comment. @arsenm @frasercrmck @lenary - thank you for the feedback 1. The library could host anything that needs to be auto-generated with tablegen. If we add `MVT`s, the name `TargetSupport` becomes obsolete. 2. Therefore we could use a name like `AutoGenSupport` (or

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

fpetrogalli updated this revision to Diff 473738. fpetrogalli added a comment. When looping through the records in tablegen, I have added an extra (necessary) condition to make sure that the records we are processing are also derived from the class `ProcessorModel`. Repository: rG LLVM

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

fpetrogalli added a comment. Hi @lenary - thank you for working on this! The patch is reasonable to me. I agree on the suggestion of using forwarding headers or the first iteration of the change, it will make it easier to review. I'll adapt the auto-get patch at D137517

[PATCH] D142976: [clang][CGCall] Remove header file not used. [NFCI]

fpetrogalli created this revision. Herald added a project: All. fpetrogalli 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/D142976 Files: clang/lib/CodeGen/CGCall.cpp

[PATCH] D142976: [clang][CGCall] Remove header file not used. [NFCI]

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20f3ebd25885: [clang][CGCall] Remove header file not used. [NFCI] (authored by fpetrogalli). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D142976: [clang][CGCall] Remove header file not used. [NFCI]

fpetrogalli accepted this revision. fpetrogalli added a comment. This revision is now accepted and ready to land. Self approving a non-controversial trivial change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142976/new/

[PATCH] D147209: [clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR.

fpetrogalli created this revision. fpetrogalli added a reviewer: mizvekov. Herald added a project: All. fpetrogalli requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is useful for retriving crash reports of LIT runs when the temporary

[PATCH] D147209: [clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR.

This revision was automatically updated to reflect the committed changes. Closed by commit rG029212617ca8: [clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR. (authored by fpetrogalli). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D147209: [clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR.

fpetrogalli added a comment. In D147209#4244615 , @leonardchan wrote: > Hi. I think this led to some test failures on our builder at > https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8784708268307261009/overview: Hi

[PATCH] D147209: [clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR.

fpetrogalli added a comment. In D147209#4246622 , @leonardchan wrote: > In D147209#4245229 , @fpetrogalli > wrote: > >> In D147209#4244615 , @leonardchan >> wrote: >>

<    1   2   3