[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook added a comment. In D67185#1718459 , @luismarques wrote: > @simoncook: your commit doesn't include handling the case of TLS lowering > when `-ffixed-x4` is used. I looked at this, and did start writing the patch that covers the use of TP/X4

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

luismarques added a comment. @simoncook: your commit doesn't include handling the case of TLS lowering when `-ffixed-x4` is used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67185/new/ https://reviews.llvm.org/D67185

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

This revision was automatically updated to reflect the committed changes. Closed by commit rGaed9d6d64a38: [RISCV] Add support for -ffixed-xX flags (authored by simoncook). Changed prior to commit: https://reviews.llvm.org/D67185?vs=223005=226081#toc Repository: rG LLVM Github Monorepo

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook added a comment. In D67185#1708177 , @asb wrote: > In D67185#1707849 , @lenary wrote: > > > Note, D68862 is in-progress at the > > moment, which is related to this

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

luismarques added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:97 RISCVABI::ABI getTargetABI() const { return TargetABI; } + bool isRegisterReservedByUser(size_t i) const { +return UserReservedRegister[i]; This should take a

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

asb added a comment. In D67185#1707849 , @lenary wrote: > Note, D68862 is in-progress at the moment, > which is related to this patch. Indeed - Simon, could you please go through that patch and ensure that the

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook marked 2 inline comments as done. simoncook added inline comments. Comment at: clang/include/clang/Driver/Options.td:2224 HelpText<"Don't workaround Cortex-A53 erratum 835769 (AArch64 only)">; -foreach i = {1-7,9-15,18,20-28} in - def ffixed_x#i : Flag<["-"],

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

lenary added a comment. Note, D68862 is in-progress at the moment, which is related to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67185/new/ https://reviews.llvm.org/D67185

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

luismarques accepted this revision. luismarques added a comment. This revision is now accepted and ready to land. Overall LGTM. Caveats: - Address the issues in the inline comments; - Shouldn't the TLS lowering also complain when `-ffixed-x4` is used? - Is there a way to ensure we don't forget

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook updated this revision to Diff 223005. simoncook added a reviewer: luismarques. simoncook added a comment. Rebase on top of tree, add @luismarques as reviewer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67185/new/

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook updated this revision to Diff 220876. simoncook added a comment. Update to reflect comments about the fact registers are explicitly reserved. In addition to @lenary 's suggested change, I renamed `isReservedReg` to note the check that we are checking if its a user provided

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

lenary added a subscriber: luismarques. lenary added a comment. Nice, I like this new approach! One naming nit, but overall I think this is much better than the first version of the patch. LGTM but I would like @luismarques to take a look too. Comment at:

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook updated this revision to Diff 219068. simoncook edited the summary of this revision. simoncook added a comment. Update based on initial feedback/going down the providing error route. Unlike AArch64, which provides an error if a function tries to call a function with arguments and any

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook added a comment. For added context, I have gone and double-checked with GCC's implementation both for AArch64 and RISC-V and for registers used by the calling convention the compiler will still use them for argument passing and return values, but otherwise won't use it for any

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook planned changes to this revision. simoncook added a comment. Thanks for the feedback. I will improve the test so it more reliably tests what it intends to. With regards to behaviour surrounding things such as argument registers, before submitting I checked what the riscv port of GCC

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/reserved-regs.ll:71 + +; X1-NOT: lw ra, +; X1-NOT: ld ra, lenary wrote: > These tests aren't going to test what you think they are, or at least aren't > going to fail when you hope they are, as

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

lenary added a comment. I don't quite understand all the details of this patch. I understand reserving registers that the compiler would otherwise be using as general-purpose registers. But what do we do about using registers within the calling convention when someone says they should be

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

simoncook created this revision. simoncook added reviewers: asb, lenary. Herald added subscribers: llvm-commits, cfe-commits, pzheng, s.egerton, Jim, benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, MaskRay, jrtc27, shiva0217, kito-cheng,