Page MenuHomePhabricator

lewis-revill (Lewis Revill)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 31 2018, 10:55 AM (20 w, 12 h)

Recent Activity

Thu, Dec 13

lewis-revill edited parent revisions for D55305: [RISCV, WIP] Add lowering of global TLS addresses, added: 2; removed: 1.
Thu, Dec 13, 1:28 PM
lewis-revill removed a child revision for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers: D55305: [RISCV, WIP] Add lowering of global TLS addresses.
Thu, Dec 13, 1:28 PM
lewis-revill added a child revision for D55303: [RISCV] Add lowering of addressing sequences for PIC: D55305: [RISCV, WIP] Add lowering of global TLS addresses.
Thu, Dec 13, 1:28 PM
lewis-revill added a child revision for D55667: [RISCV] Support assembling TLS LA pseudo instructions: D55305: [RISCV, WIP] Add lowering of global TLS addresses.
Thu, Dec 13, 1:28 PM
lewis-revill updated the diff for D55305: [RISCV, WIP] Add lowering of global TLS addresses.

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

Thu, Dec 13, 1:27 PM
lewis-revill added a parent revision for D55667: [RISCV] Support assembling TLS LA pseudo instructions: D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.
Thu, Dec 13, 12:56 PM
lewis-revill added a child revision for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers: D55667: [RISCV] Support assembling TLS LA pseudo instructions.
Thu, Dec 13, 12:56 PM
lewis-revill created D55667: [RISCV] Support assembling TLS LA pseudo instructions.
Thu, Dec 13, 12:55 PM
lewis-revill added inline comments to D55325: [RISCV] Add assembler support for LA pseudo-instruction.
Thu, Dec 13, 12:21 PM
lewis-revill added inline comments to D55325: [RISCV] Add assembler support for LA pseudo-instruction.
Thu, Dec 13, 12:18 PM
lewis-revill edited parent revisions for D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers, added: 1; removed: 1.
Thu, Dec 13, 12:13 PM
lewis-revill added a child revision for D55325: [RISCV] Add assembler support for LA pseudo-instruction: D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.
Thu, Dec 13, 12:13 PM
lewis-revill removed a child revision for D55279: [RISCV] Support assembling %got_pcrel_hi operator: D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.
Thu, Dec 13, 12:13 PM
lewis-revill removed a parent revision for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers: D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
Thu, Dec 13, 12:09 PM
lewis-revill removed a child revision for D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12: D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.
Thu, Dec 13, 12:09 PM
lewis-revill updated the diff for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.

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.

Thu, Dec 13, 12:08 PM
lewis-revill updated the diff for D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.

Fix bad merge and failing tests

Thu, Dec 13, 11:43 AM
lewis-revill added a child revision for D55335: [RISCV, WIP] Support assembling @plt symbol operands: D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.
Thu, Dec 13, 11:20 AM
lewis-revill added parent revisions for D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers: D55335: [RISCV, WIP] Support assembling @plt symbol operands, D55279: [RISCV] Support assembling %got_pcrel_hi operator.
Thu, Dec 13, 11:20 AM
lewis-revill added a child revision for D55279: [RISCV] Support assembling %got_pcrel_hi operator: D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.
Thu, Dec 13, 11:20 AM
lewis-revill updated the diff for D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.

Rebased with updated dependencies

Thu, Dec 13, 11:19 AM
lewis-revill added inline comments to D55303: [RISCV] Add lowering of addressing sequences for PIC.
Thu, Dec 13, 9:50 AM
lewis-revill updated the diff for D55303: [RISCV] Add lowering of addressing sequences for PIC.

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

Thu, Dec 13, 9:49 AM
lewis-revill added inline comments to D55303: [RISCV] Add lowering of addressing sequences for PIC.
Thu, Dec 13, 1:33 AM
lewis-revill updated the diff for D55303: [RISCV] Add lowering of addressing sequences for PIC.

Remove unnecessary Flags operand

Thu, Dec 13, 1:23 AM
lewis-revill updated the diff for D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

Remove unnecessary Flags operand.

Thu, Dec 13, 1:06 AM

Tue, Dec 11

lewis-revill updated the diff for D55335: [RISCV, WIP] Support assembling @plt symbol operands.

Rebase following update to D55560.

Tue, Dec 11, 1:04 PM
lewis-revill updated the diff for D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.

Do not allow VK_RISCV_None on call symbols

Tue, Dec 11, 1:01 PM
lewis-revill added inline comments to D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.
Tue, Dec 11, 12:54 PM
lewis-revill removed a child revision for D55303: [RISCV] Add lowering of addressing sequences for PIC: D55304: [RISCV, WIP] Lower calls through PLT.
Tue, Dec 11, 12:52 PM
lewis-revill updated the diff for D55304: [RISCV, WIP] Lower calls through PLT.

Rebased to use D55560 and with fewer dependencies

Tue, Dec 11, 12:52 PM
lewis-revill updated the summary of D55304: [RISCV, WIP] Lower calls through PLT.
Tue, Dec 11, 12:52 PM
lewis-revill removed a parent revision for D55304: [RISCV, WIP] Lower calls through PLT: D55303: [RISCV] Add lowering of addressing sequences for PIC.
Tue, Dec 11, 12:52 PM
lewis-revill added a parent revision for D55335: [RISCV, WIP] Support assembling @plt symbol operands: D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.
Tue, Dec 11, 9:06 AM
lewis-revill added a child revision for D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation: D55335: [RISCV, WIP] Support assembling @plt symbol operands.
Tue, Dec 11, 9:06 AM
lewis-revill updated the diff for D55335: [RISCV, WIP] Support assembling @plt symbol operands.

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

Tue, Dec 11, 9:05 AM
lewis-revill created D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.
Tue, Dec 11, 9:03 AM
lewis-revill added a comment to D55279: [RISCV] Support assembling %got_pcrel_hi operator.

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.

Tue, Dec 11, 6:57 AM

Mon, Dec 10

lewis-revill added a child revision for D55325: [RISCV] Add assembler support for LA pseudo-instruction: D55303: [RISCV] Add lowering of addressing sequences for PIC.
Mon, Dec 10, 7:46 PM
lewis-revill added a parent revision for D55303: [RISCV] Add lowering of addressing sequences for PIC: D55325: [RISCV] Add assembler support for LA pseudo-instruction.
Mon, Dec 10, 7:46 PM
lewis-revill updated the diff for D55303: [RISCV] Add lowering of addressing sequences for PIC.

Rebased and updated to use and expand PseudoLA.

Mon, Dec 10, 7:42 PM
lewis-revill added inline comments to D55279: [RISCV] Support assembling %got_pcrel_hi operator.
Mon, Dec 10, 7:24 PM
lewis-revill added a comment to D55279: [RISCV] Support assembling %got_pcrel_hi operator.

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.

Mon, Dec 10, 7:16 PM
lewis-revill requested changes to D55279: [RISCV] Support assembling %got_pcrel_hi operator.

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.

Mon, Dec 10, 7:14 PM
lewis-revill planned changes to D55303: [RISCV] Add lowering of addressing sequences for PIC.

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

Mon, Dec 10, 8:49 AM
lewis-revill updated the diff for D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

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

Mon, Dec 10, 8:49 AM
lewis-revill planned changes to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

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

Mon, Dec 10, 4:36 AM

Fri, Dec 7

lewis-revill updated the diff for D55335: [RISCV, WIP] Support assembling @plt symbol operands.

Address minor issues.

Fri, Dec 7, 2:05 PM
lewis-revill added inline comments to D55335: [RISCV, WIP] Support assembling @plt symbol operands.
Fri, Dec 7, 1:49 PM

Thu, Dec 6

lewis-revill added a comment to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

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.

Thu, Dec 6, 4:10 PM
lewis-revill updated the diff for D55305: [RISCV, WIP] Add lowering of global TLS addresses.

Rebased with updated dependencies.

Thu, Dec 6, 2:20 PM
lewis-revill updated the diff for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.

Rebased with updated dependency.

Thu, Dec 6, 2:20 PM
lewis-revill updated the diff for D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.

Use parseOperandWithModifier to parse TPRelAddSymbol.

Thu, Dec 6, 2:17 PM
lewis-revill updated the diff for D55335: [RISCV, WIP] Support assembling @plt symbol operands.

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.

Thu, Dec 6, 11:33 AM
lewis-revill added a parent revision for D55305: [RISCV, WIP] Add lowering of global TLS addresses: D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.
Thu, Dec 6, 11:06 AM
lewis-revill added a child revision for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers: D55305: [RISCV, WIP] Add lowering of global TLS addresses.
Thu, Dec 6, 11:06 AM
lewis-revill updated the diff for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.

Rebased with updated dependency.

Thu, Dec 6, 10:54 AM
lewis-revill updated the diff for D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.

Rebased with updated dependency.

Thu, Dec 6, 10:53 AM
lewis-revill updated the diff for D55303: [RISCV] Add lowering of addressing sequences for PIC.

Rebased with updated dependency.

Thu, Dec 6, 10:48 AM
lewis-revill updated the diff for D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

Rebased with dependency and marked helper functions as static.

Thu, Dec 6, 10:45 AM
lewis-revill added inline comments to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.
Thu, Dec 6, 10:44 AM
lewis-revill added a parent revision for D54143: [RISCV] Generate address sequences suitable for mcmodel=medium: D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
Thu, Dec 6, 10:34 AM
lewis-revill added a child revision for D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12: D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.
Thu, Dec 6, 10:34 AM
lewis-revill removed a parent revision for D55303: [RISCV] Add lowering of addressing sequences for PIC: D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
Thu, Dec 6, 10:33 AM
lewis-revill removed a child revision for D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12: D55303: [RISCV] Add lowering of addressing sequences for PIC.
Thu, Dec 6, 10:33 AM
lewis-revill added inline comments to D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.
Thu, Dec 6, 10:02 AM
lewis-revill added a parent revision for D55303: [RISCV] Add lowering of addressing sequences for PIC: D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
Thu, Dec 6, 9:55 AM
lewis-revill added a child revision for D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12: D55303: [RISCV] Add lowering of addressing sequences for PIC.
Thu, Dec 6, 9:55 AM
lewis-revill added a comment to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

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?

Thu, Dec 6, 9:54 AM

Wed, Dec 5

lewis-revill updated the diff for D55304: [RISCV, WIP] Lower calls through PLT.

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

Wed, Dec 5, 3:45 PM
lewis-revill updated the diff for D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

Rearranged RISCVExpandPseudoInsts.cpp

Wed, Dec 5, 2:54 PM
lewis-revill added a comment to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

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).

Wed, Dec 5, 2:38 PM
lewis-revill added a comment to D55335: [RISCV, WIP] Support assembling @plt symbol operands.

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.

Wed, Dec 5, 2:23 PM
lewis-revill updated the diff for D55305: [RISCV, WIP] Add lowering of global TLS addresses.

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

Wed, Dec 5, 2:15 PM
lewis-revill added a parent revision for D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers: D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.
Wed, Dec 5, 2:12 PM
lewis-revill added a child revision for D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers: D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.
Wed, Dec 5, 2:12 PM
lewis-revill created D55342: [RISCV, WIP] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.
Wed, Dec 5, 2:12 PM
lewis-revill created D55341: [RISCV, WIP] Support assembling TLS add and associated modifiers.
Wed, Dec 5, 2:09 PM
lewis-revill added a comment to D55335: [RISCV, WIP] Support assembling @plt symbol operands.

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.

Wed, Dec 5, 2:00 PM
lewis-revill added a child revision for D55335: [RISCV, WIP] Support assembling @plt symbol operands: D55304: [RISCV, WIP] Lower calls through PLT.
Wed, Dec 5, 12:13 PM
lewis-revill added a parent revision for D55304: [RISCV, WIP] Lower calls through PLT: D55335: [RISCV, WIP] Support assembling @plt symbol operands.
Wed, Dec 5, 12:13 PM
lewis-revill updated the diff for D55304: [RISCV, WIP] Lower calls through PLT.

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

Wed, Dec 5, 12:11 PM
lewis-revill created D55335: [RISCV, WIP] Support assembling @plt symbol operands.
Wed, Dec 5, 12:08 PM
lewis-revill abandoned D55306: [RISCV, WIP] Lower TLS addresses using localexec.

Merged into D55305

Wed, Dec 5, 11:35 AM
lewis-revill updated the diff for D55305: [RISCV, WIP] Add lowering of global TLS addresses.

Merged D55306 into this patch.

Wed, Dec 5, 11:34 AM
lewis-revill added a parent revision for D55303: [RISCV] Add lowering of addressing sequences for PIC: D55279: [RISCV] Support assembling %got_pcrel_hi operator.
Wed, Dec 5, 11:07 AM
lewis-revill updated the diff for D55303: [RISCV] Add lowering of addressing sequences for PIC.

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.

Wed, Dec 5, 11:07 AM
lewis-revill added a child revision for D55279: [RISCV] Support assembling %got_pcrel_hi operator: D55303: [RISCV] Add lowering of addressing sequences for PIC.
Wed, Dec 5, 11:07 AM
lewis-revill planned changes to D55305: [RISCV, WIP] Add lowering of global TLS addresses.

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

Wed, Dec 5, 10:12 AM
lewis-revill added inline comments to D55306: [RISCV, WIP] Lower TLS addresses using localexec.
Wed, Dec 5, 10:12 AM
lewis-revill planned changes to D55304: [RISCV, WIP] Lower calls through PLT.

Splitting this patch into MC layer and Codegen.

Wed, Dec 5, 10:11 AM
lewis-revill requested changes to D55279: [RISCV] Support assembling %got_pcrel_hi operator.

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.

Wed, Dec 5, 9:46 AM
lewis-revill added a comment to D55306: [RISCV, WIP] Lower TLS addresses using localexec.

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

Wed, Dec 5, 9:28 AM
lewis-revill added a comment to D55305: [RISCV, WIP] Add lowering of global TLS addresses.

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.

Wed, Dec 5, 9:26 AM
lewis-revill added inline comments to D55304: [RISCV, WIP] Lower calls through PLT.
Wed, Dec 5, 9:22 AM
lewis-revill added a comment to D55303: [RISCV] Add lowering of addressing sequences for PIC.

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.

Wed, Dec 5, 9:15 AM
lewis-revill planned changes to D55303: [RISCV] Add lowering of addressing sequences for PIC.

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.

Wed, Dec 5, 9:09 AM

Tue, Dec 4

lewis-revill updated the summary of D55305: [RISCV, WIP] Add lowering of global TLS addresses.
Tue, Dec 4, 5:23 PM
lewis-revill added a child revision for D55304: [RISCV, WIP] Lower calls through PLT: D55305: [RISCV, WIP] Add lowering of global TLS addresses.
Tue, Dec 4, 5:23 PM
lewis-revill created D55306: [RISCV, WIP] Lower TLS addresses using localexec.
Tue, Dec 4, 5:22 PM