[PATCH] D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types

2020-01-13 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary added a comment. @jrtc27 It would be good to get this in for LLVM 10.0. Having gone through the logic with @asb, we are both confident this is the right fix, and this patch does not require any additions at the moment. Repository: rG LLVM Github Monorepo

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

2020-01-13 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:151 + TargetMachine::initTargetOptions(M, Conf.Options); + tejohnson wrote: > This is going to be problematic. The Conf is a reference to the Config object > saved on the LTO class instance

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

2020-01-13 Thread Sam Elliott via Phabricator via cfe-commits
lenary created this revision. lenary added reviewers: tejohnson, dblaikie, khchen, dsanders, echristo. Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, liufengdb, herhut, lucyrfox, mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar, rriddle,

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

2020-01-13 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:151 + TargetMachine::initTargetOptions(M, Conf.Options); + tejohnson wrote: > lenary wrote: > > tejohnson wrote: > > > This is going to be problematic. The Conf is a reference to the Config

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

2020-01-14 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:151 + TargetMachine::initTargetOptions(M, Conf.Options); + tejohnson wrote: > lenary wrote: > > tejohnson wrote: > > > lenary wrote: > > > > tejohnson wrote: > > > > > This is going to be pr

[PATCH] D71553: [RISCV] Add Clang frontend support for Bitmanip extension

2020-01-14 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. @s.egerton Please can you revert rG57cf6ee9c84434161088c39a6f8dd2aae14eb12d - this patch has open dependencies, including on the LLVM support for RISC-V B extension. It would be confusing to users if c

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

2020-01-14 Thread Sam Elliott via Phabricator via cfe-commits
lenary marked 2 inline comments as done. lenary added a comment. In D72624#1817464 , @tejohnson wrote: > Thank you for your feedback! It has been very helpful. > I'm not sure if ThinLTOCodeGenerator.cpp and LTOBackend.cpp were > intentionally left out

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

2020-01-14 Thread Sam Elliott via Phabricator via cfe-commits
lenary updated this revision to Diff 238053. lenary added a comment. Herald added subscribers: mgorny, MatzeB. Address some review feedback. This patch remains incomplete with regards to module flags and ThinLTO. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

2020-01-14 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. @efriedma To keep you in the loop, D72624 is the patch to add a target hook for setting the TargetMachine options based on module metadata. That patch does not add any RISC-V specific functionality, it only lays the groundwork for it.

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

2020-01-14 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. One thing to note about my patch, above: I have not made the TargetMachine DataLayout non-const, but I wanted to ensure that this might be possible in future, which is why calling `initializeOptionsWithModuleMetadata` must be done before the first call to `createDataLayo

[PATCH] D71553: [RISCV] Add Clang frontend support for Bitmanip extension

2020-01-15 Thread Sam Elliott via Phabricator via cfe-commits
lenary reopened this revision. lenary added a comment. This revision is now accepted and ready to land. @s.egerton Thank you. Sorry again for the confusion. Reverted in rGcbe681bd8339d3a018d25441a5f4ef9da2bd017d Repository:

[PATCH] D72755: [RISCV] Pass target-abi via module flag metadata

2020-01-15 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Please can you also add code for ensuring that the `target-abi` module flag matches the `-target-abi` command line flag in llvm? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72755/new/ https://reviews.llvm.org/D72755 __

[PATCH] D84414: [RISCV] Support Shadow Call Stack

2020-08-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D84414#2186257 , @jrtc27 wrote: > There is a possibly-compelling argument against using x18: RV32E only gives > x0-x15, so would not be able to support the current implementation. We discussed this on the RISC-V LLVM Sync-up (b

[PATCH] D84414: [RISCV] Support Shadow Call Stack

2020-08-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Why do we have to pass `-ffixed-x18` when compiling? Is it enough to just reserve `x18` whenever the function has the shadow call stack attribute? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84414/new/ https://reviews.llv

[PATCH] D84414: [RISCV] Support Shadow Call Stack

2020-08-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D84414#2234255 , @zzheng wrote: > In D84414#2234112 , @lenary wrote: > >> Why do we have to pass `-ffixed-x18` when compiling? Is it enough to just >> reserve `x18` whenever the function

[PATCH] D84414: [RISCV] Support Shadow Call Stack

2020-08-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D84414#2234327 , @pcc wrote: > FWIW, on aarch64 we decided to make `-fsanitize=shadow-call-stack` require > the x18 reservation (instead of implying it) to try to avoid ABI mismatch > problems. That is, it should be safe to mix

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

2020-11-17 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I'm worried about this change - I *think* it doesn't cover the existing behaviour of a baremetal GCC toolchain being installed into the same prefix as clang, and clang automatically picking up that baremetal gcc toolchain. What should we expect to do here? This is especi

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

2020-11-17 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D91442#2399341 , @abidh wrote: > In D91442#2399200 , @lenary wrote: > >> I'm worried about this change - I *think* it doesn't cover the existing >> behaviour of a baremetal GCC toolchain

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

2020-11-17 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I recall that there is also https://reviews.llvm.org/D68407 which iirc hoped to address using `RISCVToolchain` with Compiler-rt and libunwind - presumably it is not a complete solution, but it might be we want to use some checking of the GCCInstallation (like `RISCVToolC

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

2020-11-26 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Nope, no more comments from me. Thanks for updating based on my comments :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91442/new/ https://reviews.llvm.org/D91442 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D91278: [Clang][CodeGen][RISCV] Fix hard float ABI for struct with empty struct and complex

2020-11-30 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. LGTM - the big hint is that the new test is handled the same as `f_empty_double2` as required by the ABI. Comment at: clang/test/CodeGen/riscv32-ilp32d-abi.cpp:36 + +struct empty_complex { struct {}; double _Complex fc; }; + Please can

[PATCH] D91270: [Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct

2020-11-30 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a reviewer: jrtc27. lenary added a comment. I *think* this is looking good, but I'd like a second pair of eyes from @jrtc27 as I know she's fixed bugs around this area recently too. We would like to have this ready for 11.0.1 if possible. Comment at: clang/lib/Co

[PATCH] D92650: [RISCV] Define preprocessor definitions for 'V' extension.

2020-12-04 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary 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/D92650/new/ https://reviews.llvm.org/D92650 ___

[PATCH] D91278: [Clang][CodeGen][RISCV] Fix hard float ABI for struct with empty struct and complex

2020-12-07 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91278/new/ https://reviews.llvm.org/D91278 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D91270: [Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct

2020-12-07 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary added a comment. LGTM! Please land these tonight so we can have them backported. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91270/new/ https://reviews.llvm.org/D91270 ___

[PATCH] D107878: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO) profile loader

2021-08-19 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/test/CodeGen/X86/fsafdo_test2.ll:3 +; RUN: llvm-profdata merge --sample -profile-isfs -o %t.afdo %S/Inputs/fsloader.afdo +; RUN: llc -enable-fs-discriminator -fs-profile-file=%t.afdo -show-fs-branchprob -disable-ra-fsprofile-loader

[PATCH] D113256: [AArch64][ARM] Enablement of Cortex-A710 Support

2021-11-05 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:159-160 AArch64::AEK_RCPC | AArch64::AEK_SSBS)) +AARCH64_CPU_NAME("cortex-a710", ARMV9A, FK_NEON_FP_ARMV8, false, + (AArch64::AEK_MTE | AArch64::AEK_PAUT

[PATCH] D113256: [AArch64][ARM] Enablement of Cortex-A710 Support

2021-11-05 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I'm happy with these changes, and with the comments from @dmgreen - as I'm away next week, don't wait for my approval to land this if David has given his approval. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113256/new/

[PATCH] D114703: [AArch64] Use Feature for A53 Erratum 835769 Fix

2021-11-29 Thread Sam Elliott via Phabricator via cfe-commits
lenary created this revision. lenary added a reviewer: ostannard. Herald added subscribers: hiraditya, kristof.beyls. lenary requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. When this pass was originally implemented, the fix pass was en

[PATCH] D136145: [IR][RFC] Restrict read only when cache type of llvm.prefetch is instruction

2022-10-18 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. The Arm changes (for tests) here are reasonable, and indeed both arm architectures do not allocate encoding space for instruction write. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136145/new/ https://reviews.llvm.org/D13

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-11-01 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Reverse ping. This has been accepted, what is the status of landing this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

2022-11-01 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Oh, can you close this revision and link it to the landed commit please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123630/new/ https://reviews.llvm.org/D123630 ___ cfe-commits

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. This caused an issue in the libc benchmarks, fix here: 2a261a7b5764 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D1

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary marked an inline comment as done. lenary added inline comments. Comment at: llvm/include/llvm/ADT/Triple.h:11 +/// This header is deprecated in favour of +/// `llvm/TargetParser/AArch64TargetParser.h`. /// barannikov88 wrote: > Invalid comment. Fixed in h

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary marked an inline comment as done. lenary added a comment. Another fixup for the llvm examples was required, landed in https://reviews.llvm.org/rG16c4c4e04c14 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. And a fixup for the compiler-rt symbolizer: https://reviews.llvm.org/rGecaab107e4d0 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D137838 ___

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137838#4007920 , @thakis wrote: > Thanks for the heads-up. I updated the GN build in > 4ac51dd53d93b8dd18c58093766483c657fe3a08 > and > 2aa998d22fe09191cd

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary edited the summary of this revision. lenary updated this revision to Diff 484281. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140222/new/ https://reviews.llvm.org/D140222 Files: clang/include/clang/Basic/BuiltinsAArch64.def clang/lib/He

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary marked 3 inline comments as done. lenary added a comment. Nits addressed. Will land in the new year, when I'm back at work. Right now it's time for a break! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140222/new/ https://reviews.llvm.org/

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137838#4008443 , @aprantl wrote: > Can you please update `llvm/include/llvm/module.modulemap` for this change or > revert the patch? This is breaking all bots that build with > `-DLLVM_ENABLE_MODULES=On`. > > For example: >

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I'm working on a follow-up, which should make the split a bit clearer, but I'm also not a modulemap expert and the `-DLLVM_ENABLE_MODULES=On` configuration is broken on my linux dev box. I'll post it for review if those two patches have at least made the build greener.

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. One comment on the build changes, I don't have opinions on the RISC-V changes. Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19 +target_include_directories(LLVMAArch64AsmParser PRIVATE ${LLVM_LIBRARY_DIR}/TargetParser/)

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19 +target_include_directories(LLVMAArch64AsmParser PRIVATE ${LLVM_LIBRARY_DIR}/TargetParser/) fpetrogalli wrote: > craig.topper wrote: > > fpetrogalli wrote: > > > lena

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137838#4008605 , @lenary wrote: > I'm working on a follow-up, which should make the split a bit clearer, but > I'm also not a modulemap expert and the `-DLLVM_ENABLE_MODULES=On` > configuration is broken on my linux dev box.

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

2022-12-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19 +target_include_directories(LLVMAArch64AsmParser PRIVATE ${LLVM_LIBRARY_DIR}/TargetParser/) fpetrogalli wrote: > lenary wrote: > > fpetrogalli wrote: > > > craig.topp

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

2022-12-21 Thread Sam Elliott via Phabricator via cfe-commits
lenary 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). Generally this

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

2022-12-30 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137838#4018930 , @vitalybuka wrote: > This bot is broken after the patch > https://lab.llvm.org/buildbot/#/builders/236/builds/1480 Thanks for letting me know. I'm back in the office on the 3rd, and it looks like this will

[PATCH] D123609: [Clang] Remove support for legacy pass manager

2023-01-03 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Herald added a subscriber: pcwang-thead. In D123609#3451334 , @nikic wrote: > In D123609#3451320 , @HaohaiWen > wrote: > >> Hi @nikic, >> We recently noticed legacy PM was removed from man

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

2023-01-04 Thread Sam Elliott via Phabricator via cfe-commits
lenary created this revision. Herald added subscribers: hiraditya, kristof.beyls. Herald added a project: All. lenary requested review of this revision. Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, MaskRay. Herald added projects: clang, LLDB, LLVM. This deprecates, but does n

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

2023-01-04 Thread Sam Elliott via Phabricator via cfe-commits
lenary added reviewers: tmatheson, pratlucas, MaskRay. lenary added a comment. Herald added subscribers: StephenFan, JDevlieghere. Adding some relevant reviewers. My plan is to land the other patches like this directly onto `main`, as they are NFC and a cleanup. I hope this is uncontroversial, b

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

2023-01-04 Thread Sam Elliott via Phabricator via cfe-commits
lenary edited the summary of this revision. lenary updated this revision to Diff 486324. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140999/new/ https://reviews.llvm.org/D140999 Files: clang/lib/Basic/Targets/AArch64.cpp clang/lib/Basic/Target

[PATCH] D141411: [AArch64] Make -march and target("arch=..") attributes imply dependent features

2023-01-11 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Can we check this logic, especially around adding both AEK_SVE and AEK_SVE2 in the `AARCH64_ARCH` descriptions, this means that `-march=armv9-a+nosve2` still can generate sve instructions. I'm not entirely sure of the intended behaviour there, especially as sve2 should m

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

2022-11-07 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I'm not convinced we should be leaving any of the other Target Parser information in Support, if we are doing this, though this creates layering issues which I've been trying to unpick. If you try to take `llvm/Support/*TargetParser*`, you find a few places where there

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

2022-11-07 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137516#3912751 , @fpetrogalli wrote: > @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 `TargetSuppor

[PATCH] D137659: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

2022-11-08 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary added a comment. This revision is now accepted and ready to land. Thanks for doing this cleanup! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137659/new/ https://reviews.llvm.org/D137659 _

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParser

2022-11-11 Thread Sam Elliott via Phabricator via cfe-commits
lenary created this revision. Herald added subscribers: hiraditya, kristof.beyls. Herald added a project: All. lenary requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. This should live with the Arm targets, given th

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

2022-11-11 Thread Sam Elliott via Phabricator via cfe-commits
lenary created this revision. Herald added subscribers: kadircet, arphaman, hiraditya, krytarowski. Herald added a project: All. lenary requested review of this revision. Herald added projects: LLVM, clang-tools-extra. Herald added subscribers: cfe-commits, llvm-commits. This change is focussed on

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

2022-11-11 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Herald added a subscriber: Michael137. Apologies to everyone who has automatically been marked as a reviewer for this change. There is more context for it here: https://discourse.llvm.org/t/targetparser-auto-generation-of-riscv-cpu-definitions/66419/4?u=lenary Repositor

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

2022-11-11 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137516#3912900 , @fpetrogalli wrote: > In D137516#3912842 , @lenary wrote: > >> In D137516#3912751 , @fpetrogalli >> wrote: >> >>> @arsenm @f

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

2022-11-11 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137838#3921828 , @thakis wrote: > This is a gigantic diff. I'd recommend keeping the .h files in the old place > for v0 and make them just forwarding headers that include the header at the > new location. That way, you don't

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParser

2022-11-15 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I'm going to move this into the newly created `ARMTargetParserCommon` files which @tmatheson created in https://reviews.llvm.org/D137924 (which has landed). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137835/new/ https://

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

2022-11-16 Thread Sam Elliott via Phabricator via cfe-commits
lenary retitled this revision from "[ARM] Move ARM::parseBranchProtection into ARMTargetParser" to "[ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon". lenary edited the summary of this revision. lenary updated this revision to Diff 475860. Repository: rG LLVM Github Monorepo C

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

2022-11-16 Thread Sam Elliott via Phabricator via cfe-commits
lenary marked an inline comment as done. lenary added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:47 #include "llvm/Option/ArgList.h" +#include "llvm/Support/ARMTargetParser.h" #include "llvm/Support/CodeGen.h" tmatheson wrote: > For cons

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

2022-11-16 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I've updated the patch to use forwarding headers (and to rebase past some changes that have happened in the interim). The patch is still huge because of the number of places using the TargetParsers, which need the component added to their CMakeLists.txt. I think after t

[PATCH] D142410: [AArch64] ARMv8.5-A implies both FEAT_SB and FEAT_SSBS

2023-01-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. SSBS is not mandatory from v8.5a onwards. More details/citation inline. NB: This will need a rebase, because @dmgreen changed how crypto is handled in these bitmaps yesterday. Comment at: llvm/include/llvm/TargetParser/AArch64TargetParser.h:311 inline

[PATCH] D142410: [AArch64] ARMv8.5-A implies both FEAT_SB and FEAT_SSBS

2023-01-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D142410#4076389 , @philipp.tomsich wrote: > When looking at "ARM DDI 0487G.a", on page A2-68 under the heading > "Additional functionality added to Armv8.0 in later releases" in the > definition-list item "FEAT_SSBS, Speculat

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

2023-01-30 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. I'm not sure I agree with "map naturally", when `CRm` is a 4-bit field, and both bit 7 and bit 5 are different in the `dsb ` vs `dsb nXS` instructions. Are you willing to put this through review for the ACLE ?

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

2023-02-03 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D140999#4030131 , @MaskRay wrote: > I don't think it is necessary to deprecate the old header then delete it > after 16.0.0 is branched. > llvm/Support/AArch64TargetParser.h has very few open-source out-of-tree uses. > Perhaps

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

2023-02-03 Thread Sam Elliott via Phabricator via cfe-commits
lenary retitled this revision from "[NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h" to "[NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h". lenary edited the summary of this revision. lenary updated this revision to Diff 494596. Repository: rG LLVM Github Monorepo

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

2023-02-03 Thread Sam Elliott via Phabricator via cfe-commits
lenary updated this revision to Diff 494600. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140999/new/ https://reviews.llvm.org/D140999 Files: clang/lib/Basic/Targets/AArch64.cpp clang/lib/Basic/Targets/AArch64.h clang/lib/CodeGen/CGBuiltin.cp

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

2023-02-03 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Most recent diff was to clang-format the patch, which has removed some duplicate includes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140999/new/ https://reviews.llvm.org/D140999

[PATCH] D140999: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h

2023-02-03 Thread Sam Elliott via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8c712296fb75: [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h (authored by lenary). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140999/new/

[PATCH] D140959: RFC: Multilib prototype

2023-01-23 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Thanks for putting together this proposal. Broadly, I think it's good, and I think it solves a lot of rough edges that I recall running into with the GCC multilib configuration files in the past. I do worry about using `-cc1` args. On the one hand, it's the best place fo

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

2023-01-23 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:8297 << Arg->getSourceRange(); } else if (IsAArch64Builtin && Fields.size() == 1) { +// If this is a write ... tmatheson wrote: > It might be more readable to outline t

[PATCH] D140222: [AArch64] Check 128-bit Sysreg Builtins

2023-01-23 Thread Sam Elliott 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 rG03ef89f8d909: [AArch64] Check 128-bit Sysreg Builtins (authored by lenary). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D127798: [AArch64] Define __ARM_FEATURE_RCPC

2022-06-16 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Yes. Please submit a PR or Issue to the ACLE, explaining the use-case (the explanation here is good enough, I think). Once the ACLE changes are merged, then you can proceed with a patch for clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D97204: [RFC] Clang 64-bit source locations

2022-06-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Herald added a project: All. Some of this patch set has been broken by https://reviews.llvm.org/D125403 and https://reviews.llvm.org/D125952 - which are size optimisations for PCH encodings of source locations. The biggest issue is new line 90 of `clang/include/clang/Ser

[PATCH] D105494: [clang] Introduce a union inside ProgramPoint.

2022-06-20 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Herald added a project: All. This change looks fine to me (though clang-format wants a reasonable change). I'm not familiar with the clang static analyzer, so I'm not sure I can +2 this, but hopefully this message will act as a ping for the existing reviewers? Repositor

[PATCH] D128415: [ARM] Add Support for Cortex-M85 r=MarkMurrayARM,dmgreen,tmatheson

2022-06-23 Thread Sam Elliott via Phabricator via cfe-commits
lenary created this revision. lenary added reviewers: MarkMurrayARM, dmgreen, tmatheson. Herald added subscribers: jdoerfert, hiraditya, kristof.beyls. Herald added a project: All. lenary requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits,

[PATCH] D127197: [ARM] Fix how size-0 bitfields affect homogeneous aggregates.

2022-06-10 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary added a comment. This revision is now accepted and ready to land. LGTM. I tested this patch in an end-to-end fashion to look at the assembly in affected cases, and it is doing the right thing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

2022-11-22 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary 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/D138488/new/ https://reviews.llvm.org/D138488 __

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

2022-11-22 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Oh, actually, I have a few nits. You can fix them on commit though. Comment at: llvm/lib/Target/AArch64/AArch64.td:497 +def FeatureCSSC : SubtargetFeature<"cssc", "HasCSSC", "true", + "Enable Common Short Sequence Compression (CSSC) instructions">; + --

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

2022-11-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision. lenary added a comment. This revision is now accepted and ready to land. Some comment nits that you can fixup on commit. Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:3913 +// are post-indexed, and the immediate values are not inside the

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

2022-11-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:8572 + def STLR_x_64 : BaseLRCPC3IntegerLoadStore<0b11, 0b10, (outs GPR64sp:$Rn_wb) , (ins GPR64:$Rt, GPR64sp:$Rn), "stlr" , "\t$Rt, [$Rn, #-8]!", "$Rn = $Rn_wb">; /* PUSH register

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary updated this revision to Diff 477904. lenary marked an inline comment as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137835/new/ https://reviews.llvm.org/D137835 Files: clang/lib/Basic/Targets/AArch64.cpp clang/lib/Basic/Targets/A

[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon

2022-11-25 Thread Sam Elliott 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 rG3e9b6adfc7ca: [ARM] Move ARM::parseBranchProtection into ARMTargetParserCommon (authored by lenary). Repository: rG LLVM Github Monorepo CHANGES

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary edited the summary of this revision. lenary updated this revision to Diff 477912. lenary marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137836/new/ https://reviews.llvm.org/D137836 Files: clang-tools-extra/c

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary 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, which i

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary 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, which i

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

2022-11-25 Thread Sam Elliott 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 rG5577207d6d3e: [Support] Move getHostNumPhysicalCores to Threading.h (authored by lenary). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137836#3950815 , @fhahn wrote: > Unfortunately it looks like this commit breaks building on ARM64 macOS. I > reverted the change for now and added more details on the error in the revert > commit. Thanks for reverting. I had

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. In D137836#3950846 , @fhahn wrote: > (it looks like this job should have sent an email: > https://green.lab.llvm.org/green/job/clang-stage1-RA/32031/console) Yeah, sorry, no email. I'll check with colleagues to see if other peopl

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary edited the summary of this revision. lenary updated this revision to Diff 477975. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137836/new/ https://reviews.llvm.org/D137836 Files: clang-tools-extra/clangd/test/Inputs/BenchmarkHeader.h llv

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a subscriber: MaskRay. lenary added a comment. New version, which changes how the move works a bit. I've updated the description with the caveats, so this is now definitely not NFC. I'm looking for re-review, and maybe input from @MaskRay who has made some minor cleanups in this co

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: clang/lib/Lex/CMakeLists.txt:3 -set(LLVM_LINK_COMPONENTS support) +set(LLVM_LINK_COMPONENTS + Support fpetrogalli wrote: > I wonder how `support` was not being detected as a linking error... Not sure, but I still think

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

2022-11-25 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. > As for the name TargetParser. @arsenm came up with the name > SubtargetRegistry. I am fine with either names. @fpetrogalli I'm going to stick with TargetParser, because I think this is less confusing, given that TargetRegistry is already a component. Repository: rG

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

2022-11-28 Thread Sam Elliott via Phabricator via cfe-commits
lenary marked 3 inline comments as done. lenary added a comment. Going to attempt to land this again today. Comment at: llvm/lib/Support/Threading.cpp:59 if (MaxThreadCount <= 0) MaxThreadCount = 1; if (ThreadsRequested == 0) tmatheson wrote: > It lo

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

2022-11-29 Thread Sam Elliott via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. lenary marked an inline comment as done. Closed by commit rG3c97f6cab92f: [Support] Move getHostNumPhysicalCores to Threading.h (authored by lenary). Changed prior to

[PATCH] D138920: [AArch64] Assembly support for VMSA

2022-11-29 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment. Two nits. One below, the other: Please include everyone who contributed to this patch in the patch description. Comment at: llvm/lib/Target/AArch64/AArch64.td:519 +def FeatureTHE : SubtargetFeature<"the", "HasTHE", +"true", "Enable Translation Harde

[PATCH] D138920: [AArch64] Assembly support for VMSA

2022-11-29 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments. Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:82 + AEK_THE = 1ULL << 50, // FEAT_THE + AEK_D128 =1ULL << 51, // FEAT_LSE128 + AEK_LSE128 = 1ULL << 52, // FEAT_D128 Comments don't correspond to na

<    1   2   3   >