- User Since
- Jan 4 2017, 12:12 PM (110 w, 4 d)
Fri, Feb 15
Wed, Feb 6
Tue, Feb 5
I think we should introduce a new Constraint_A enum member for this case. Whilst "A" currently behaves the same for the RISC-V backend as "m", that won't necessarily be the case forever. "A" is required to always be a single GPR (and so can be used for the atomic instructions, or ones with reduced immediate encoding space), but "m" (at least according to GCC) is "any kind of address that the machine supports in general", which I assume for RISC-V means reg+simm12.
Please update docs/ClangCommandLineReference.rst. Also, in my limited testing, GCC just seems to pass the -msmall-data-limit=... flag straight through to the linker through COLLECT_GCC_OPTIONS. Finally, please add tests for -msmall-data-limit=... instead of or as well as -G, since GCC only recognises the former on RISC-V.
Fixed broken test
Support all memory instructions; add test coverage
Please rebase now I've removed the stray whitespace in the definition of PseudoLA.
Hi Lewis, please rebase this after rL351823 (the Call node is now a riscv_call node in PseudoCll's pattern.
Hi Lewis, could you please rebase this after the RV64A patch landed and added PseudoMaskedCmpXchg64 to RISCVExpandPseudoInsts (conflicts with adding PseudoLLA)?
(Added dependency due to both adding to RISCVBaseInfo.h)
Rebased and added TODO comment.
Rebased, fixed whitespace, removed TODO, added FIXME, added invalid tests.
Rebased (sorrt for the delay).
Could we not have a situation where the pcrel_hi20 is seen *after* the lo12, in which case the lo12 would be forced but the hi20 could still be evaluated? E.g. I think the following would be problematic:
Mon, Jan 28
You should save the original AsmToken rather than reconstructing it, so as to retain the SMLoc, and Buf no longer needs to be at function scope. Also, I had some more exhaustive tests in mine; I think the important one is addi a1, a0, (1) to ensure that we can parse parenthesised immediate expressions when *not* part of a memory reference.
There's also AsmToken::Exclaim for logical not that we could allow (although that's not one I've ever seen in real-world assembly). Please add tests for the CSR change?
Thu, Jan 24
Ah, I already did this in my fork, covering all the compressed and floating-point instructions too, with a complete set of tests. Probably easiest if I just post that rather than you working to produce what would turn out to be nearly-identical code?
Tue, Jan 22
Jan 15 2019
Jan 8 2019
Jan 7 2019
Dec 14 2018
Thanks, looks good to me now.
Dec 13 2018
Dec 12 2018
Dec 11 2018
Oh, probably also worth mentioning the small <-> medlow and medium <-> medany GCC code model mapping in the commit message, and probably a comment too in the source code of getAddr.
Seems fine from my point of view but we'll wait to hear from the others. I'd suggest removing the "WIP" from the subject to reflect the true state.
Almost there now from my point of view, but we'll see what the others say (especially Alex).
Slight annoying bit of churn turning getPCRelHiExpr into getPCRelHiFixup (when I tweaked D54029 I simply didn't make shouldForceRelocation give an error for a null getPCRelHiExpr return value, and therefore it could remain unchanged, but with the increased error checking we now need to get the entire MCFixup back).
Rebased on top of latest changes to D54029
Dec 7 2018
Please also add some tests to relocations.s like I have done for %got_pcrel_hi.
Rebased on top of D54029, added test case to ensure R_RISCV_GOT_HI20 is always preserved for the linker and updated error message for InvalidUImm20AUIPC.
Dec 6 2018
I realise now this is also dependent on D54029, otherwise the corresponding %pcrel_lo gets evaluated to the offset of the auipc. I'll rebase on top of that.
Dec 5 2018
Yes, this is a stupid situation to be in, but 32-bit PowerPC on SUSE really does use /usr/lib/gcc/powerpc64-suse-linux:
Dec 4 2018
The general strategy of adding a pseudo-instruction seems fine; I checked what the Sparc backend does and it defines a tlsadd node with its extra %tie_add(sym) operand.
I'd rather see all the local-exec support moved into D55306; I don't think it makes much sense to add things here that don't yet work. Alternatively, to avoid lines that are then going to be modified again in D55306, you could merge the two? There aren't many extra changes in D55306.
Half of this (the MC side of the GOT modifier) is already proposed in D55279. Also note that the assembly modifier is not %got_hi but %got_pcrel_hi after a discussion with upstream (check the assembler manual) to clearly convey that it's a PC-relative relocation to the GOT entry rather than generating a GOT index as would be the case on some older architectures. I did however, like you, use GOT_HI in the various LLVM-internal enumeration entries to match the name of the relocation.
Nov 30 2018
Ah, never mind, D54421 was updated to also patch lld at the same time, so this has been merged already.
Ping? Stack is now empty.