Rebase with updated dependencies. Use and expand the la.tls.ie and la.tls.gd pseudo instructions.

Rebased with updated dependencies. Renamed ie_hi to got_hi to match relocation, and rearrange to be adjacent to gd_hi in most files. Added tests to relocations.s, and ensured that fixups evaluate correctly.

Fix bad merge and failing tests

Rebased with updated dependencies

Move getAddrPIC() and isPositionIndependent() checks to getAddr()

Remove unnecessary Flags operand

Remove unnecessary Flags operand.

Rebase following update to D55560.

Do not allow VK_RISCV_None on call symbols

Rebased to use D55560 and with fewer dependencies

Rebased removing unnecessary dependencies and to use the cleaner CallSymbol patch.

I think that is a change that should be implemented in this revision, since this is the revision that adds the fixup and the tests. Otherwise we would end up with D54029 depending on this, which itself depends on D54029.

Rebased and updated to use and expand PseudoLA.

In D55279#1326556, @lewis-revill wrote:

Do you need to add a check for RISCV::fixup_riscv_pcrel_hi20 in getPcRelHiExpr? Currently this is returning null and causing a segfault when running your relocations.s tests.

Do you need to add a check for RISCV::fixup_riscv_pcrel_hi20 in getPcRelHiExpr? Currently this is returning null and causing a segfault when running your relocations.s tests.

Rebasing with latest changes to D55279 and modifying this patch to use and expand PseudoLA (D55325).

Rebased and updated to use and expand PseudoLLA for PC-relative addressing.

Rebasing and modifying this patch to use and expand PseudoLLA instead of introducing a new wrapper.

Address minor issues.

In D54143#1322155, @jrtc27 wrote:

In D54143#1321605, @lewis-revill wrote:

So my problem is that (if we expand PseudoLLA in codegen in the same way as we do in RISCVAsmParser) then we lose some flexibility for later addressing uses. Currently I can use the wrapper to do (WrapperPCRel %pcrel_hi(sym)), (WrapperPCRel %got_pcrel_hi(sym)), (WrapperPCRel %tls_ie_pcrel_hi(sym)) and (WrapperPCRel %tls_gd_pcrel_hi(sym)), whereas with PseudoLLA I am limited to just %pcrel_hi addressing. The addition of PseudoLA would help greatly, but only for PIC, not for TLS, meaning another wrapper would be required anyway. Maybe this is an acceptable approach to use PseudoLLA and PseudoLA where it fits then add a wrapper later?

For TLS you'd then use the yet-to-be-implemented PseudoLA_TLS_GD and PseudoLA_TLS_IE (or whatever they end up being called) representing the la.tls.gd and la.tls.ie macros/pseudo-instructions. Anything you could want to use WrapperPCRel will need a corresponding PseudoFOO to exist for the assembly parser, so I still fail to see why we would need this extra wrapper. I really like having the exact correspondence between what CodeGen produces and what the equivalent hand-written assembly would be parsed to.

Rebased with updated dependencies.

Rebased with updated dependency.

Use parseOperandWithModifier to parse TPRelAddSymbol.

Rebased with updated dependency and added checking for tail or call instruction mnemonic in Operands[0] when parsing '@plt' symbols, with appropriate tests added. This is just a proof of concept for this implementation.

Rebased with updated dependency.

Rebased with updated dependency.

Rebased with updated dependency.

Rebased with dependency and marked helper functions as static.

So my problem is that (if we expand PseudoLLA in codegen in the same way as we do in RISCVAsmParser) then we lose some flexibility for later addressing uses. Currently I can use the wrapper to do (WrapperPCRel %pcrel_hi(sym)), (WrapperPCRel %got_pcrel_hi(sym)), (WrapperPCRel %tls_ie_pcrel_hi(sym)) and (WrapperPCRel %tls_gd_pcrel_hi(sym)), whereas with PseudoLLA I am limited to just %pcrel_hi addressing. The addition of PseudoLA would help greatly, but only for PIC, not for TLS, meaning another wrapper would be required anyway. Maybe this is an acceptable approach to use PseudoLLA and PseudoLA where it fits then add a wrapper later?

Removed capability of lowering through GOT since there is no apparent need to do so.

Rearranged RISCVExpandPseudoInsts.cpp

In D54143#1319641, @jrtc27 wrote:

In D54143#1319494, @lewis-revill wrote:

I've tried re-using PseudoLLA, but I just cannot get around the problem of expanding it in the MC layer. If I try expanding it in RISCVMCCodeEmitter there is no way to get/create an appropriate expression to use for the %pcrel_lo relocation. It would be nice if it was possible to create a <.text+offset> expression for the AUIPC instruction but I just don't know how. Otherwise I have also tried splitting up the instruction earlier, but the AUIPC/ADDI get split up too often to make it feasible. Also there's no real benefit because RISCVMergeBaseOffset cannot work on the %pcrel_lo base symbols without a great deal of modification. @rogfer01 what do you do differently to this patch for the PC-relative case?

Why can't it be expanded in RISCVExpandPseudo? If it works for a new PseudoAddrPCRel, it works if you instead use PseudoLLA. All you need is a find/replace of PseudoAddrPCRel with PseudoLLA (and of course removing all the definitions of PseudoAddrPCRel).

Ideally no. I guess it would be good to error when parsing rather than trying to emit a R_RISCV_CALL_PLT relocation for a non-call... I'm leaning towards checking Operands[0] at the moment but it depends what others think.

Split into MC layer (patches D55341 D55342) and codegen (this patch)

I agree that it would be good to prevent other instructions using '@plt' (this was mentioned in the original MC+codegen patch). The difficulty is that getParser().parseIdentifier consumes the '@' as a valid character. This would mean that the new parseBareSymbolOrPLT would be trivial to do from what I have implemented here but a check would have to be added in parseBareSymbol for whether the identifier ends in '@plt'. The other possibility is to keep the logic in 'parseBareSymbol' and if it is an identifier with '@plt' then it should check Operands[0] for 'call' or 'tail', which doesn't seem very nice to me.

Split into MC layer (patch D55335) and codegen (this patch).

Merged into D55305

Merged D55306 into this patch.

Update to use the MC layer changes in D55279. There are still some MC layer changes left in this patch waiting for D55279 to be updated.

Merging D55306 into this, then splitting this patch into MC layer and Codegen

Splitting this patch into MC layer and Codegen.

Could you correct the error message for InvalidUImm20AUIPC in RISCVAsmParser to include this new operator & change rv32i-invalid.s appropriately? When I did this I also added a test line in rv32i-valid.s but it's mostly testing the same thing as the test in relocations.s. Maybe you want to add that as well for completeness though.

I'm going to merge these changes into D55305 (followed by splitting up MC layer changes into separate patches).

In D55305#1319651, @jrtc27 wrote:

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.

It's probably worth noting that the %pcrel_lo relocations to the label appear to be evaluated to a constant when -mattr=+relax is not provided... I remember someone said that this was going to be made default? If so I think it would be good to wait for that before this is commited.

That's an interesting coincidence! I think I'll rebase this patch on top of that patch since it seems likely to make it upstream.