Page MenuHomePhabricator

jrtc27 (James Clarke)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 4 2017, 12:12 PM (128 w, 6 d)

Recent Activity

Thu, Jun 20

jrtc27 accepted D63220: [ELF][RISCV] Support GD/LD/IE/LE TLS models.

One minor pedantic stylistic comment but otherwise this seems fine to me.

Thu, Jun 20, 8:00 AM · Restricted Project

Tue, Jun 18

jrtc27 added a comment to D57792: [RISCV] Support z and i operand modifiers.
In D57792#1549678, @asb wrote:
In D57792#1549669, @asb wrote:

Is there documentation on the operand modifiers supported by RISC-V binutils anywhere? I'm struggling to find anything...

This is GCC not binutils, and I can't find them documented anywhere other than the source itself: https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/config/riscv/riscv.c?view=markup&revision=271293#l3181

Sorry, I meant GCC.

Thanks, this patch looks good to me. Tiny nit: LLVM coding guidelines prefer full sentences for comments, so the comments added to RISCVAsmPrinter.cpp should end in a full stop.

Tue, Jun 18, 10:31 PM · Restricted Project
jrtc27 added a comment to D57332: [RISCV] Allow parsing of bare symbols with offsets.
In D57332#1549661, @asb wrote:

Is this being used in the wild and does it work sensibly with binutils? I'm seeing that binutils seems to be associating the addend with both the emitted R_RISCV_PCREL_HI20 relocation and R_RISCV_RELAX relocation. Maybe this is just a binutils bug and the addend is ignored by the linker but this does seem surprising...

$ cat t.s 
lla a5, a_symbol + (0xFF << 3)
$ ./riscv32-unknown-elf-as t.s
$ ./riscv32-unknown-elf-readelf -r a.out 

Relocation section '.rela.text' at offset 0xec contains 4 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000617 R_RISCV_PCREL_HI2 00000000   a_symbol + 7f8
00000000  00000033 R_RISCV_RELAX                7f8
00000004  00000418 R_RISCV_PCREL_LO1 00000000   .L0  + 0
00000004  00000033 R_RISCV_RELAX                0
Tue, Jun 18, 10:13 PM · Restricted Project
jrtc27 added a comment to D57792: [RISCV] Support z and i operand modifiers.
In D57792#1549669, @asb wrote:

Is there documentation on the operand modifiers supported by RISC-V binutils anywhere? I'm struggling to find anything...

Tue, Jun 18, 10:11 PM · Restricted Project
jrtc27 added inline comments to D54296: [RISCV] Lower inline asm constraint A for RISC-V.
Tue, Jun 18, 8:30 AM · Restricted Project

Mon, Jun 17

jrtc27 added a comment to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.

Does that make a visible difference?

Mon, Jun 17, 5:30 AM · Restricted Project
jrtc27 requested changes to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.

Still disagree about the behaviour of R_RISCV_CALL. You can't just choose the ABI that you want. Please at least put it back to R_PC. @ruiu do you have an opinion on this?

Mon, Jun 17, 5:06 AM · Restricted Project

Fri, Jun 14

jrtc27 added inline comments to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.
Fri, Jun 14, 6:12 AM · Restricted Project
jrtc27 added inline comments to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.
Fri, Jun 14, 6:11 AM · Restricted Project

Thu, Jun 13

jrtc27 added inline comments to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.
Thu, Jun 13, 2:09 PM · Restricted Project
jrtc27 accepted D63132: [ELF][RISCV] Create dummy .sdata for __global_pointer$ if .sdata does not exist.

@jrtc27 Do this patch and D63076 look good now?

Thu, Jun 13, 1:53 PM · Restricted Project
jrtc27 requested changes to D63220: [ELF][RISCV] Support GD/LD/IE/LE TLS models.
Thu, Jun 13, 1:55 AM · Restricted Project

Tue, Jun 11

jrtc27 added a comment to D63132: [ELF][RISCV] Create dummy .sdata for __global_pointer$ if .sdata does not exist.

GNU ld treats it as undefined:

Yes. That was why I created the patch to fix the musl -fpie -pie link error: ld.lld: error: relocation R_RISCV_PCREL_HI20 cannot refer to absolute symbol: __global_pointer$. This error is lld specific: in -pie/-shared mode, a PC relative relocation to an absolute symbol is not allowed.

In https://groups.google.com/forum/#!searchin/generic-abi/SHN_ABS|sort:date/generic-abi/MC4b4sqnKC0/oQgZxxwMmJAJ, Cary's interpretation of gABI is:

On the contrary, the only interpretation that makes sense to me is that it will not change because of relocation at link time or at load time.

If lld resolves the PC relative relocation statically, the value will be incorrect when the pie or shared object is loaded at runtime because the SHN_ABS symbol's value does not change. The best lld can do is to issue an error.

An alternative is to create a dummy .sdata in musl's Scrt1.o. I don't know how reasonable it is to have lla gp,__global_pointer$ in other code that might not define .sdata.

Tue, Jun 11, 7:53 AM · Restricted Project
jrtc27 added inline comments to D63132: [ELF][RISCV] Create dummy .sdata for __global_pointer$ if .sdata does not exist.
Tue, Jun 11, 7:30 AM · Restricted Project
jrtc27 added inline comments to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.
Tue, Jun 11, 7:14 AM · Restricted Project
jrtc27 added inline comments to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.
Tue, Jun 11, 7:14 AM · Restricted Project
jrtc27 requested changes to D63132: [ELF][RISCV] Create dummy .sdata for __global_pointer$ if .sdata does not exist.

GNU ld treats it as undefined:

Tue, Jun 11, 6:53 AM · Restricted Project
jrtc27 requested changes to D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations.
Tue, Jun 11, 5:38 AM · Restricted Project

Thu, Jun 6

jrtc27 updated the diff for D57795: [RISCV] Add FreeBSD targets.

Sorted OS case statements

Thu, Jun 6, 7:27 AM · Restricted Project
jrtc27 added a comment to D61866: [RISCV] Rename RISCVELFStreamer.{cpp,h} to RISCVTargetELFStreamer.{cpp,h}.

Hm, actually, looking more closely, there seems to be inconsistency. Some backends use FooELFStreamer.cpp for an MCELFStreamer, and others for a ELF-specific target streamer. Some, like AArch64, even put both in the same file, so if you don't like this patch, I suppose the recommendation would be to do that?

Thu, Jun 6, 5:50 AM · Restricted Project
jrtc27 added a comment to D61866: [RISCV] Rename RISCVELFStreamer.{cpp,h} to RISCVTargetELFStreamer.{cpp,h}.
In D61866#1532288, @asb wrote:

I think this is less confusing, but there's more to be gained by having a similar naming scheme to the other backends which right now all use FooELFStreamer.{cpp,h}.

I you feel strongly about this change, I'd consider making it for all in-tree backends.

Thu, Jun 6, 5:45 AM · Restricted Project

Mon, Jun 3

jrtc27 added a comment to D60785: [ELF] Align file offset for .bss if first section in a PT_LOAD.

Ping?

Mon, Jun 3, 7:26 AM · Restricted Project

Fri, May 31

jrtc27 added a comment to D60657: [RISCV] Fix evaluation of %pcrel_lo.

Thanks for this. I realize D58843 is a very heavy-handed approach so I'd be happy if we could avoid it cleanly with target-specific changes, however I'm not certain we would be able to cover all the combinations we would need to with patches like this?

For example would we be able to cover pairs that occur out of order?

Fri, May 31, 6:09 AM · Restricted Project

May 16 2019

jrtc27 added a comment to D61584: [DebugInfo] Some fields do not need relocations even relax is enabled..

Why do we need this flag; can we not already determine this (either in generic code or the target backends)? Anything where all symbols referenced are not in SHF_EXECINSTR sections can be evaluated even with relaxations, which covers all these cases:

May 16 2019, 5:06 AM · Restricted Project

May 13 2019

jrtc27 created D61866: [RISCV] Rename RISCVELFStreamer.{cpp,h} to RISCVTargetELFStreamer.{cpp,h}.
May 13 2019, 10:34 AM · Restricted Project

May 2 2019

jrtc27 added a comment to D61464: [RiscV] Typo in register aliases.

Ouch; LGTM.

May 2 2019, 5:06 PM · Restricted Project, Restricted Project
jrtc27 edited reviewers for D61464: [RiscV] Typo in register aliases, added: asb; removed: echristo.
May 2 2019, 5:04 PM · Restricted Project, Restricted Project

Apr 18 2019

jrtc27 updated the diff for D60785: [ELF] Align file offset for .bss if first section in a PT_LOAD.

Also handle .bss where the zeroes are written to the file.

Apr 18 2019, 8:52 AM · Restricted Project
jrtc27 added inline comments to D60785: [ELF] Align file offset for .bss if first section in a PT_LOAD.
Apr 18 2019, 8:20 AM · Restricted Project
jrtc27 added inline comments to D60785: [ELF] Align file offset for .bss if first section in a PT_LOAD.
Apr 18 2019, 6:00 AM · Restricted Project

Apr 17 2019

jrtc27 added a comment to D45181: [RISCV] Add diff relocation support for RISC-V.

The problem is that Length is the length of the CIE, i.e. the data structure itself; this means it isn't subject to relaxations and so the difference expression can be folded. I don't see much merit in teaching all the tools out there to check relocations for Length when we can instead just emit the "right" thing. I think the fix is to make sure we only return true from requiresDiffExpressionRelocations if a target is in a code section? MCSection::UseCodeAlign will tell you this (or you can use MCSectionELF::getFlags() & ELF::SHF_EXECINSTR to be less general), though perhaps it should be renamed to isCodeSection to match isVirtualSection.

Apr 17 2019, 6:55 AM · Restricted Project

Apr 16 2019

jrtc27 updated the diff for D60785: [ELF] Align file offset for .bss if first section in a PT_LOAD.

Updated affected tests.

Apr 16 2019, 11:15 AM · Restricted Project
jrtc27 created D60785: [ELF] Align file offset for .bss if first section in a PT_LOAD.
Apr 16 2019, 10:46 AM · Restricted Project

Apr 13 2019

jrtc27 added inline comments to D60657: [RISCV] Fix evaluation of %pcrel_lo.
Apr 13 2019, 3:11 PM · Restricted Project

Apr 3 2019

Herald added a project to D55303: [RISCV] Add lowering of addressing sequences for PIC: Restricted Project.

One minor comment, but otherwise I think this is ready to land now?

Apr 3 2019, 7:31 AM · Restricted Project

Mar 21 2019

jrtc27 added a comment to D39324: [lld] Support TLS in RISC-V.

@PkmX Ping, any chance you could address the remaining few points, please?

Mar 21 2019, 6:06 AM · lld
jrtc27 added a comment to D39323: [lld] Support dynamic linking in RISC-V.

@PkmX Ping, any chance you could address the remaining few points, please?

Mar 21 2019, 6:06 AM · Restricted Project, lld

Feb 21 2019

jrtc27 added a comment to D54093: [RISCV] Lower inline asm constraints I, J & K for RISC-V.

This probably needs some invalid tests (ie the out of range immediates mentioned in the TODO). Also, please run utils/update_llc_test_checks.py to get the RV64I test lines.

Feb 21 2019, 7:15 PM · Restricted Project
jrtc27 added a comment to D58531: [clang] Specify type of pthread_create builtin.

A separate point is whether it makes sense to be emitting this warning in the first place for GE_Missing_type. I'd argue that, if we don't know the type of the builtin, we should never emit the warning, since otherwise we always emit the warning even for legitimate instances, though that's not really important now we're not hitting that case any more.

Feb 21 2019, 4:29 PM · Restricted Project
jrtc27 added a reviewer for D58531: [clang] Specify type of pthread_create builtin: rsmith.
Feb 21 2019, 4:17 PM · Restricted Project
jrtc27 created D58531: [clang] Specify type of pthread_create builtin.
Feb 21 2019, 4:13 PM · Restricted Project

Feb 20 2019

jrtc27 added a comment to D57141: [RISCV] Add implied zero offset load/store alias patterns.

D50496 in the stack has now landed, so other than the TODO comments having been dropped this is now ready. Alex, are you happy to do that rebasing when you commit this, or would you like me to?

Feb 20 2019, 12:44 PM · Restricted Project
jrtc27 retitled D57141: [RISCV] Add implied zero offset load/store alias patterns from [RISCV] Add implied zero offset load/store alias pattern to [RISCV] Add implied zero offset load/store alias patterns.
Feb 20 2019, 12:42 PM · Restricted Project
jrtc27 retitled D57141: [RISCV] Add implied zero offset load/store alias patterns from [RISCV][WIP] Add implied zero offset load/store alias pattern to [RISCV] Add implied zero offset load/store alias pattern.
Feb 20 2019, 12:42 PM · Restricted Project

Feb 19 2019

jrtc27 added a comment to D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics.

Also worth noting that the frontend needs to know the max width otherwise warn_atomic_op_misaligned will be triggered with the message large atomic operation may incur significant performance penalty (name is misleading since the %select{large|misaligned}0 means it's triggered for both large and misaligned atomic ops, and it's also in the atomic-alignment group). This is exactly what I did in my tree (other than the tests) and it seemed to work correctly for RV64A compiling FreeBSD.

Feb 19 2019, 4:31 AM · Restricted Project

Feb 18 2019

jrtc27 added inline comments to D58335: [DebugInfo] Generate fixups as emitting DWARF .debug_frame/.eh_frame..
Feb 18 2019, 11:14 AM · Restricted Project
jrtc27 added a comment to D58335: [DebugInfo] Generate fixups as emitting DWARF .debug_frame/.eh_frame..

Perhaps we should keep getKindForSize as being in bytes, and add a new getKindForSizeInBits? This would reduce the diff, but more importantly ensures that downstream forks don't silently break until runtime due to the changed semantics of getKindForSize despite having the same type as before.

Feb 18 2019, 11:06 AM · Restricted Project

Feb 15 2019

jrtc27 added a comment to D54093: [RISCV] Lower inline asm constraints I, J & K for RISC-V.
In D54093#1397875, @asb wrote:

Did you check the behaviour of 'J' vs GCC, because for e.g. asm volatile ("sub a0, a0, %0" :: "J"(0)); I seem to see it using the integer 0 rather than the zero register?

I think it would be worth adding RV64I check lines to inline-asm.ll too.

Feb 15 2019, 5:56 AM · Restricted Project
jrtc27 added inline comments to D50496: [RISCV] Implment pseudo instructions for load/store from a symbol address..
Feb 15 2019, 5:03 AM · Restricted Project

Feb 6 2019

jrtc27 requested changes to D54295: [RISCV] Add inline asm constraint A for RISC-V.
Feb 6 2019, 7:00 AM · Restricted Project, Restricted Project

Feb 5 2019

jrtc27 added a comment to D54296: [RISCV] Lower inline asm constraint A for RISC-V.

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.

Feb 5 2019, 4:35 PM · Restricted Project
jrtc27 created D57795: [RISCV] Add FreeBSD targets.
Feb 5 2019, 4:16 PM · Restricted Project
jrtc27 requested changes to D57497: [RISCV] Passing small data limitation value to RISCV backend.

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.

Feb 5 2019, 4:00 PM · Restricted Project
jrtc27 updated the diff for D55277: [RISCV] Match GNU tools canonical JALR and add aliases.

Fixed broken test

Feb 5 2019, 3:15 PM · Restricted Project
jrtc27 updated the diff for D57141: [RISCV] Add implied zero offset load/store alias patterns.

Support all memory instructions; add test coverage

Feb 5 2019, 3:03 PM · Restricted Project
jrtc27 commandeered D57141: [RISCV] Add implied zero offset load/store alias patterns.
Feb 5 2019, 3:02 PM · Restricted Project
jrtc27 added a parent revision for D57792: [RISCV] Support z and i operand modifiers: D54296: [RISCV] Lower inline asm constraint A for RISC-V.
Feb 5 2019, 2:59 PM · Restricted Project
jrtc27 added a child revision for D54296: [RISCV] Lower inline asm constraint A for RISC-V: D57792: [RISCV] Support z and i operand modifiers.
Feb 5 2019, 2:59 PM · Restricted Project
jrtc27 created D57792: [RISCV] Support z and i operand modifiers.
Feb 5 2019, 2:58 PM · Restricted Project
jrtc27 added a comment to D57240: [RISCV] Don't incorrectly force relocation for %pcrel_lo.

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:

foo:
    j 1f
0:  addi a0, a0, %pcrel_lo(1f)
    ret
1: auipc a0, %pcrel_hi(bar)
    j 0b

.align 2
bar:
    # ...

Obviously extremely pathological code, but nonetheless valid.

Good call. It's a pathological case but maybe there are other cases as well. It's not a nice situation. Somehow we have to ensure there can never be a pcrel_lo12 relocation without a corresponding pcrel_hi20 relocation. It might help to know whether the pcrel_hi20 has been seen at all, regardless of whether it was resolved.

So I guess the intended behaviour might be:

pcrel_hi20 seen and not resolved (IE relocation emitted) -> force relocation
pcrel_hi20 seen and resolved -> don't force relocation
pcrel_hi20 not seen -> don't force relocation? If evaluatePcRelLo thinks it can be evaluated does it matter if the pcrel_hi20 can or not?

I'll have another think and take a look at your testcase.

Feb 5 2019, 10:19 AM · Restricted Project
jrtc27 added inline comments to D55341: [RISCV] Support assembling TLS add and associated modifiers.
Feb 5 2019, 10:14 AM · Restricted Project
jrtc27 requested changes to D55342: [RISCV] Support assembling %tls_{ie,gd}_pcrel_hi modifiers.
Feb 5 2019, 9:57 AM · Restricted Project
jrtc27 requested changes to D55341: [RISCV] Support assembling TLS add and associated modifiers.
Feb 5 2019, 9:52 AM · Restricted Project
Herald added a project to D55667: [RISCV] Support assembling TLS LA pseudo instructions: Restricted Project.

Please rebase now I've removed the stray whitespace in the definition of PseudoLA.

Feb 5 2019, 9:42 AM · Restricted Project
jrtc27 added a comment to D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.

Hi Lewis, please rebase this after rL351823 (the Call node is now a riscv_call node in PseudoCll's pattern.

Feb 5 2019, 9:32 AM · Restricted Project
jrtc27 added a comment to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

Hi Lewis, could you please rebase this after the RV64A patch landed and added PseudoMaskedCmpXchg64 to RISCVExpandPseudoInsts (conflicts with adding PseudoLLA)?

Feb 5 2019, 9:25 AM · Restricted Project
jrtc27 added a comment to D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.

(Added dependency due to both adding to RISCVBaseInfo.h)

Feb 5 2019, 9:22 AM · Restricted Project
jrtc27 added a parent revision for D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation: D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.
Feb 5 2019, 9:21 AM · Restricted Project
jrtc27 added a child revision for D54143: [RISCV] Generate address sequences suitable for mcmodel=medium: D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.
Feb 5 2019, 9:21 AM · Restricted Project
jrtc27 updated the diff for D55277: [RISCV] Match GNU tools canonical JALR and add aliases.

Rebased and added TODO comment.

Feb 5 2019, 9:14 AM · Restricted Project
jrtc27 updated the diff for D55325: [RISCV] Add assembler support for LA pseudo-instruction.

Rebased, fixed whitespace, removed TODO, added FIXME, added invalid tests.

Feb 5 2019, 8:48 AM · Restricted Project
jrtc27 added a comment to D55279: [RISCV] Support assembling %got_pcrel_hi operator.
In D55279#1367361, @asb wrote:

Hi James, can you please rebase and confirm that you are aware of the new license put in place last Friday and intend to contribute under it. See http://lists.llvm.org/pipermail/llvm-dev/2019-January/129344.html https://llvm.org/docs/DeveloperPolicy.html#new-llvm-project-license-framework

Feb 5 2019, 8:30 AM · Restricted Project
jrtc27 updated the diff for D55279: [RISCV] Support assembling %got_pcrel_hi operator.

Rebased (sorrt for the delay).

Feb 5 2019, 8:30 AM · Restricted Project
Herald added a project to D57240: [RISCV] Don't incorrectly force relocation for %pcrel_lo: Restricted Project.

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:

Feb 5 2019, 8:25 AM · Restricted Project

Jan 28 2019

jrtc27 requested changes to D57319: [RISCV] Fix failure to parse parenthesized immediates.

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.

Jan 28 2019, 4:55 AM · Restricted Project
jrtc27 requested changes to D57320: [RISCV] Allow parsing immediates that use tilde & exclaim.

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?

Jan 28 2019, 4:49 AM · Restricted Project

Jan 24 2019

jrtc27 added a comment to D57141: [RISCV] Add implied zero offset load/store alias patterns.

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?

Jan 24 2019, 8:50 AM · Restricted Project

Jan 22 2019

jrtc27 requested changes to D57055: [RISCV] Mark TLS as supported.
Jan 22 2019, 7:44 AM · Restricted Project, Restricted Project
jrtc27 added inline comments to D53235: [RISCV] Add RV64F codegen support.
Jan 22 2019, 6:39 AM

Jan 15 2019

jrtc27 added a comment to D50496: [RISCV] Implment pseudo instructions for load/store from a symbol address..

Just noted that this is failing:

"sw zero, (a0)"   
<stdin>:1:15: error: operand must be a symbol with %lo/%pcrel_lo modifier or an integer in the range [-2048, 2047]
sw      zero, (a0)
              ^

We should also accept it as sw zero, 0(a0).
I verified GNU accepts it.

Jan 15 2019, 12:01 PM · Restricted Project

Jan 8 2019

jrtc27 added inline comments to D51206: [Sparc] Add tail call support.
Jan 8 2019, 10:17 AM

Jan 7 2019

jrtc27 added inline comments to D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.
Jan 7 2019, 3:56 AM

Dec 14 2018

jrtc27 added reviewers for D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux: hfinkel, rsmith.
Dec 14 2018, 8:02 AM · Restricted Project
jrtc27 added a comment to D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.

Thanks, looks good to me now.

Dec 14 2018, 5:44 AM · Restricted Project

Dec 13 2018

jrtc27 added a comment to D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.

@jrtc27 Actually I agree it should produce an error regardless of whether relaxation is enabled, since it is most likely an user error to write a %pcrel_lo that doesn't point to an auipc, and the earlier we can catch it the better.

Dec 13 2018, 8:55 AM · Restricted Project
jrtc27 added a comment to D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12.
In D54029#1329560, @asb wrote:

This looks good to me, but there's one addition test I'd like to see. You explain that the intended behaviour is that there will be no compile-time error if linker relaxation is enabled. It would be good to have this reflected in a test. Maybe adding extra RUN lines to pcrel-lo12-invalid.s and adding a comment that explains that is expected with linker relaxation enabled.

Dec 13 2018, 8:21 AM · Restricted Project
jrtc27 added a comment to D55296: [Support] Fix GNU/kFreeBSD build.

Okay, then LGTM

I don't have commit rights, so could somebody please commit on my behalf?

Dec 13 2018, 8:03 AM
jrtc27 added inline comments to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.
Dec 13 2018, 5:06 AM · Restricted Project
jrtc27 added inline comments to D55303: [RISCV] Add lowering of addressing sequences for PIC.
Dec 13 2018, 5:01 AM · Restricted Project

Dec 12 2018

jrtc27 added inline comments to D55303: [RISCV] Add lowering of addressing sequences for PIC.
Dec 12 2018, 8:36 AM · Restricted Project

Dec 11 2018

jrtc27 removed a parent revision for D55303: [RISCV] Add lowering of addressing sequences for PIC: D55279: [RISCV] Support assembling %got_pcrel_hi operator.
Dec 11 2018, 4:16 PM · Restricted Project
jrtc27 removed a child revision for D55279: [RISCV] Support assembling %got_pcrel_hi operator: D55303: [RISCV] Add lowering of addressing sequences for PIC.
Dec 11 2018, 4:16 PM · Restricted Project
jrtc27 added inline comments to D55560: [RISCV] Attach VK_RISCV_CALL to symbols upon creation.
Dec 11 2018, 9:07 AM · Restricted Project
jrtc27 added a comment to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

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.

Dec 11 2018, 8:57 AM · Restricted Project
jrtc27 added a comment to D54143: [RISCV] Generate address sequences suitable for mcmodel=medium.

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.

Dec 11 2018, 8:55 AM · Restricted Project
jrtc27 added a comment to D55303: [RISCV] Add lowering of addressing sequences for PIC.

Almost there now from my point of view, but we'll see what the others say (especially Alex).

Dec 11 2018, 8:52 AM · Restricted Project
jrtc27 updated the diff for D55325: [RISCV] Add assembler support for LA pseudo-instruction.

Rebased

Dec 11 2018, 8:41 AM · Restricted Project
jrtc27 added a comment to D55279: [RISCV] Support assembling %got_pcrel_hi operator.

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

Dec 11 2018, 8:39 AM · Restricted Project
jrtc27 updated the diff for D55279: [RISCV] Support assembling %got_pcrel_hi operator.

Rebased on top of latest changes to D54029

Dec 11 2018, 8:35 AM · Restricted Project
jrtc27 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.

RISCV::fixup_riscv_pcrel_hi20 ->RISCV::fixup_riscv_got_hi20

Dec 11 2018, 2:49 AM · Restricted Project