Page MenuHomePhabricator

jrtc27 (James Clarke)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Thu, Jan 23

jrtc27 updated the diff for D73170: Handle subregs and superregs in callee-saved register mask.

Switch from explicit iterator to recently-added range-based for loop interface. Ok to merge?

Thu, Jan 23, 3:01 PM · Restricted Project
jrtc27 added a comment to D73170: Handle subregs and superregs in callee-saved register mask.

Thanks for doing this! It's been on my TODO list for a little while already for SPE, but I never got around to it.

Is there any provision for spilling only the subreg, not the superreg? It seems to me that whenever the subreg needs spilled, it always also spills the superreg, even if the full superreg is not used.

Thu, Jan 23, 2:39 AM · Restricted Project

Wed, Jan 22

jrtc27 committed rGddfe8751b16a: [test] Fix lld/test/ELF/riscv-pcrel-hilo-error.s after D73211 (authored by jrtc27).
[test] Fix lld/test/ELF/riscv-pcrel-hilo-error.s after D73211
Wed, Jan 22, 6:36 PM
jrtc27 committed rG3f5976c97dbf: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols (authored by jrtc27).
[RISCV] Fix evaluating %pcrel_lo against global and weak symbols
Wed, Jan 22, 6:09 PM
jrtc27 closed D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols.
Wed, Jan 22, 6:08 PM · Restricted Project
jrtc27 added inline comments to D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols.
Wed, Jan 22, 5:23 PM · Restricted Project
jrtc27 updated the diff for D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols.

Pass correct target to shouldForceRelocation (although unused).

Wed, Jan 22, 5:23 PM · Restricted Project
jrtc27 updated the summary of D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols.
Wed, Jan 22, 5:23 PM · Restricted Project
jrtc27 updated the diff for D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols.

Addressed review comments.

Wed, Jan 22, 5:23 PM · Restricted Project
jrtc27 added a comment to D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols.

This approach seems much easier to understand.

Can you add a testcase for the case where the auipc and addi are in different fragments, like I mentioned in D71978?

Wed, Jan 22, 4:46 PM · Restricted Project
jrtc27 created D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols.
Wed, Jan 22, 9:27 AM · Restricted Project
jrtc27 created D73170: Handle subregs and superregs in callee-saved register mask.
Wed, Jan 22, 3:49 AM · Restricted Project

Mon, Jan 20

jrtc27 committed rGd1da63664f4e: [lld][RISCV] Print error when encountering R_RISCV_ALIGN (authored by jrtc27).
[lld][RISCV] Print error when encountering R_RISCV_ALIGN
Mon, Jan 20, 6:51 PM
jrtc27 closed D71820: [lld][RISCV] Print error when encountering R_RISCV_ALIGN.
Mon, Jan 20, 6:51 PM · Restricted Project
jrtc27 abandoned D73066: [RISCV] Don't always execute SC for min/max masked AMOs.

Never mind, I see why this is done, we need to respect the ordering, so skipping the SC misses the release.

Mon, Jan 20, 1:22 PM · Restricted Project
jrtc27 created D73066: [RISCV] Don't always execute SC for min/max masked AMOs.
Mon, Jan 20, 1:13 PM · Restricted Project

Tue, Jan 14

jrtc27 added a comment to D71820: [lld][RISCV] Print error when encountering R_RISCV_ALIGN.

@MaskRay Do you think we could get this landed (with or without D71822) before 10 branches tomorrow?

Tue, Jan 14, 3:30 AM · Restricted Project
jrtc27 updated the diff for D71820: [lld][RISCV] Print error when encountering R_RISCV_ALIGN.

Rebased and addressed review comments.

Tue, Jan 14, 3:30 AM · Restricted Project
jrtc27 committed rG3d6c492d7a98: [RISCV] Fix ILP32D lowering for double+double/double+int return types (authored by jrtc27).
[RISCV] Fix ILP32D lowering for double+double/double+int return types
Tue, Jan 14, 3:21 AM
jrtc27 closed D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types.
Tue, Jan 14, 3:21 AM · Restricted Project
jrtc27 added a comment to D71820: [lld][RISCV] Print error when encountering R_RISCV_ALIGN.

@MaskRay Do you think we could get this landed (with or without D71822) before 10 branches tomorrow?

Tue, Jan 14, 3:21 AM · Restricted Project
jrtc27 accepted D72134: [RISCV] Fix test for inline asm z constraint modifier.

Oh, yes, that was silly.

Tue, Jan 14, 3:14 AM · Restricted Project
jrtc27 accepted D71777: [RISCV][NFC] Deduplicate Atomic Intrinsic Definitions.

I think this is good now (and definitely better now the big comment has moved outside of the let).

Tue, Jan 14, 3:11 AM · Restricted Project

Sun, Jan 12

jrtc27 committed rG0113cf193f06: [RISCV] Check register class for AMO memory operands (authored by jrtc27).
[RISCV] Check register class for AMO memory operands
Sun, Jan 12, 5:00 PM
jrtc27 closed D72471: [RISCV] Check register class for AMO memory operands.
Sun, Jan 12, 5:00 PM · Restricted Project

Thu, Jan 9

jrtc27 created D72471: [RISCV] Check register class for AMO memory operands.
Thu, Jan 9, 11:48 AM · Restricted Project

Tue, Jan 7

jrtc27 added a comment to D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.

LGTM

the approach that's here has proved sufficient in all the cases we can come up with

I'm still sort of concerned we silently miscompile when the addi is in a different fragment from the auipc, but maybe that doesn't matter much in practice.

Tue, Jan 7, 9:20 PM · Restricted Project
jrtc27 committed rG917f46db04b8: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment (authored by jrtc27).
[RISCV] Fix evalutePCRelLo for symbols at the end of a fragment
Tue, Jan 7, 8:35 PM
jrtc27 closed D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.
Tue, Jan 7, 8:35 PM · Restricted Project
jrtc27 updated the diff for D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.

Simplified to avoid duplicating logic.

Tue, Jan 7, 4:46 PM · Restricted Project
jrtc27 added a comment to D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.

it requires . and .Ltmp0 be in the same fragment [...] This is checked earlier by ensuring findAssociatedFragment() matches for each.

That check you're mentioning doesn't actually do what you say it does; both findAssociatedFragment() actually return the fragment containing .Ltmp0.

Tue, Jan 7, 4:37 PM · Restricted Project
jrtc27 added a comment to D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.

Perhaps this helps. Let's take:

Tue, Jan 7, 2:15 PM · Restricted Project
jrtc27 added a comment to D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.

Yes, FK_IsPCRel isn't really accurate for this, and we have talked in the past about maybe teaching LLVM about indirection in fixups, but the approach that's here has proved sufficient in all the cases we can come up with, and has the advantage of keeping this RISC-V-specific fixup internal to the backend.

Tue, Jan 7, 1:56 PM · Restricted Project
jrtc27 added a comment to D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.

If you look at the implementation of getPCRelHiFixup, the fixup returned is normally the same fragment as the AUIPC symbol, which is what this code was relying on. However, the one case where it doesn't is the special case of being at the end of a fragment, where getPCRelHiFixup gets the fixup from the *next* fragment *at offset 0*. This logic therefore needs to be mirrored in evaluatePCRelLo so that they agree on what fragment they're talking about for the AUIPC fixup. The issue arises because .option (and other directives) are delayed until the next code/data, but emitting a symbol does not flush that, so the local symbols end up at the end of the previous fragment.

Tue, Jan 7, 1:46 PM · Restricted Project
jrtc27 added a comment to D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.

Ping; it would be good to get this in before 10.0 branches.

Tue, Jan 7, 10:14 AM · Restricted Project
jrtc27 added a comment to D61907: [RISCV] Leave pcrel_hi/pcrel_lo fixup pairs unresolved.

Lewis, I believe this should now be marked abandoned in light of the other fixes?

Tue, Jan 7, 10:14 AM · Restricted Project

Thu, Jan 2

jrtc27 added inline comments to D71499: Add builtins for aligning and checking alignment of pointers and integers.
Thu, Jan 2, 8:04 AM · Restricted Project

Wed, Jan 1

jrtc27 added inline comments to D72056: [RISCV] Generate PIC address sequence for medany -fno-pic.
Wed, Jan 1, 11:45 AM · Restricted Project, Restricted Project
jrtc27 added a comment to D72056: [RISCV] Generate PIC address sequence for medany -fno-pic.

I am still of the view that we should support rewriting the instruction stream in the linker when necessary like BFD does. We need to do this to be able to link in GCC-compiled code, including libraries. It is a very simple thing to do with a patch available that provides consistency between the GNU world and the LLVM world.

Wed, Jan 1, 11:07 AM · Restricted Project, Restricted Project

Tue, Dec 31

jrtc27 added a comment to D72046: [ELF][RISCV] Add test for absolute/relative/branch relocations to undefined weak symbols.

For calls, whilst it differs from BFD, it can work just fine as it’s what AArch64 does.

For data references, however, this is highly inappropriate. They *need* to resolve to 0 as *real world code* relies on that, and it’s what happens on every architecture out there. Code generated by both GCC and LLVM uses PC-relative relocations to refer to external weak symbols. We can argue all we like about whether that’s good or not, but at the end of the day *that is what happens* and we need to not break that.

No. GCC/clang emit either absolute or GOT-generating relocations (R_RISCV_HI20+R_RISCV_LO12_I; R_RISCV_GOT_HI20+R_RISCV_PCREL_LO12_I). Such relocations to weak undefined symbols can be relied on by real world code. The linker only has to resolve absolute relocations to 0 and fill GOT entries with 0.

PC-relative relocations to undefined weak symbols should not be emitted by compilers. Such hand-written assembly is just broken.

Tue, Dec 31, 6:47 PM · Restricted Project
jrtc27 added a comment to D71100: [lld][RISCV] Fixup PC-relative relocations to undefined weak symbols..

Created D72046. As per my interpretation at https://github.com/riscv/riscv-elf-psabi-doc/issues/126#issuecomment-568160758 , we have to assume

Compilers should not emit PC-relative relocations in -fpic/-fpie mode.

GCC/clang emit either absolute or GOT relocations, so the assumption is true. (If not in edge cases, we have to fix the compilers.) I don't think errors on branch relocations are useful. So, D72046 just does what I did to PPC32 (the patch changes p to p+a for PPC32 as well. It is a minor issue anyway).

Tue, Dec 31, 6:38 PM · Restricted Project
jrtc27 added a comment to D72046: [ELF][RISCV] Add test for absolute/relative/branch relocations to undefined weak symbols.

For calls, whilst it differs from BFD, it can work just fine as it’s what AArch64 does. For data references, however, this is highly inappropriate. They *need* to resolve to 0 as *real world code* relies on that, and it’s what happens on every architecture out there. Code generated by both GCC and LLVM uses PC-relative relocations to refer to external weak symbols. We can argue all we like about whether that’s good or not, but at the end of the day *that is what happens* and we need to not break that. Non-CALL(_PLT) PC-relative relocations *must not* be mislinked like this. Either do the same rewriting as in John’s patch that BFD does, or produce a suitable error, but it is not acceptable to silently mis-link this code. And I would really like LLD to support linking GCC-produced code; we should not be going off and doing our own incompatible thing. Maybe you don’t want to support gcc -fuse-ld=lld, but what about when using Clang and LLD and you’re statically-linking against a library compiled by GCC? That means coordination; either support what GCC produces (which a patch already exists for), or convince GCC to change.

Tue, Dec 31, 6:20 PM · Restricted Project

Sun, Dec 29

jrtc27 created D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment.
Sun, Dec 29, 1:18 PM · Restricted Project

Dec 23 2019

jrtc27 added inline comments to D71777: [RISCV][NFC] Deduplicate Atomic Intrinsic Definitions.
Dec 23 2019, 2:41 PM · Restricted Project
jrtc27 added inline comments to D71777: [RISCV][NFC] Deduplicate Atomic Intrinsic Definitions.
Dec 23 2019, 2:41 PM · Restricted Project
jrtc27 added a comment to D71777: [RISCV][NFC] Deduplicate Atomic Intrinsic Definitions.

I would personally rather this remain unindented, much like we do with namespace llvm etc.

I too was a bit surprised by the formatting (especially the indented section separator) but @lenary told me he just ran clang-format on the file. And I looked around and saw that some other tablegen files in LLVM also used this style of formatting, so taken together I deemed the patch style reasonable during the review. But it's true that not indenting is the most common style and has its advantages, so I would also favour keeping it unindented.

Dec 23 2019, 2:32 PM · Restricted Project
jrtc27 added a comment to D71777: [RISCV][NFC] Deduplicate Atomic Intrinsic Definitions.

I would personally rather this remain unindented, much like we do with namespace llvm etc.

Dec 23 2019, 12:04 PM · Restricted Project

Dec 22 2019

jrtc27 added inline comments to D71820: [lld][RISCV] Print error when encountering R_RISCV_ALIGN.
Dec 22 2019, 5:22 PM · Restricted Project
jrtc27 created D71820: [lld][RISCV] Print error when encountering R_RISCV_ALIGN.
Dec 22 2019, 3:58 PM · Restricted Project

Dec 21 2019

jrtc27 committed rG189b7393d545: [lld][RISCV] Use an e_flags of 0 if there are only binary input files. (authored by bsdjhb).
[lld][RISCV] Use an e_flags of 0 if there are only binary input files.
Dec 21 2019, 10:02 AM
jrtc27 closed D71101: [lld][RISCV] Use an e_flags of 0 if there are only binary input files..
Dec 21 2019, 10:01 AM · Restricted Project
jrtc27 updated the summary of D71101: [lld][RISCV] Use an e_flags of 0 if there are only binary input files..
Dec 21 2019, 9:59 AM · Restricted Project

Dec 17 2019

jrtc27 added inline comments to D71519: [ELF] Add IpltSection.
Dec 17 2019, 3:38 AM · Restricted Project

Dec 16 2019

jrtc27 added inline comments to D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls.
Dec 16 2019, 9:20 AM · Restricted Project, Restricted Project

Dec 5 2019

jrtc27 added inline comments to D71101: [lld][RISCV] Use an e_flags of 0 if there are only binary input files..
Dec 5 2019, 6:35 PM · Restricted Project
jrtc27 added inline comments to D71100: [lld][RISCV] Fixup PC-relative relocations to undefined weak symbols..
Dec 5 2019, 5:47 PM · Restricted Project
jrtc27 added inline comments to D71100: [lld][RISCV] Fixup PC-relative relocations to undefined weak symbols..
Dec 5 2019, 5:39 PM · Restricted Project
jrtc27 added inline comments to D71100: [lld][RISCV] Fixup PC-relative relocations to undefined weak symbols..
Dec 5 2019, 5:38 PM · Restricted Project
jrtc27 added inline comments to D71101: [lld][RISCV] Use an e_flags of 0 if there are only binary input files..
Dec 5 2019, 5:11 PM · Restricted Project

Dec 3 2019

jrtc27 committed rGda7b129b1b52: [RISCV] Don't force Local Exec TLS for non-PIC (authored by jrtc27).
[RISCV] Don't force Local Exec TLS for non-PIC
Dec 3 2019, 2:08 PM
jrtc27 closed D70649: [RISCV] Don't force Local Exec TLS for non-PIC.
Dec 3 2019, 2:08 PM · Restricted Project

Nov 25 2019

jrtc27 added inline comments to D70666: [RISCV] Machine Operand Flag Serialization.
Nov 25 2019, 12:09 PM · Restricted Project

Nov 24 2019

jrtc27 created D70649: [RISCV] Don't force Local Exec TLS for non-PIC.
Nov 24 2019, 3:26 PM · Restricted Project

Nov 21 2019

jrtc27 abandoned D70398: [lld] Support copy relocations for TLS symbols.

EDIT: I see the discussion linked by MaskRay has already reached the conclusion that GCC will stop emitting these. I will file a patch for LLVM to do the same if nobody beats me to it (relaxations can come later once they are finalised).

I wanted to do this, but you claimed this first. Go ahead:) Given the consensus that TLS copy relocation is a bad idea and newer toolchains do not need to support it, can be drop this patch?

Nov 21 2019, 4:56 PM · Restricted Project
jrtc27 added a comment to D70398: [lld] Support copy relocations for TLS symbols.

I think the fact that many runtime linkers out there don't handle it correctly is a pretty compelling reason in and of itself (but I agree that, being a new ABI, it should avoid introducing more copy relocations). The fact that GCC relies on this behaviour (which glibc and bfd correctly implement) but doesn't document it anywhere other than in the compiler source itself is worrying, since it leads to the situation we have now where binaries built by GCC+bfd for other systems (such as FreeBSD) may not run correctly, as they will copy garbage from early on in the mapping.

Nov 21 2019, 10:14 AM · Restricted Project

Nov 19 2019

jrtc27 added a comment to D70398: [lld] Support copy relocations for TLS symbols.

I would suggest stepping back and looking at the whole picture. TLS is not special here; for normal globals, both code models also rely on copy relocations, and there are likely to be many more of those. If you think TLS copy relocations are bad then you should also be against using HI/LO and PCREL_HI/LO for extern symbols and require those to be indirected through the GOT.

Nov 19 2019, 10:43 AM · Restricted Project
jrtc27 added a comment to D70398: [lld] Support copy relocations for TLS symbols.

What's strange is that TLS GD(/LD)/IE *don't* have linker relaxations currently even in BFD (and no R_RISCV_RELAX relocations are emitted since both compilers know it's not currently a relaxable sequence) despite RISC-V's love of linker relaxations for other things, so the model chosen at compile-time is set in stone. The only exception is that a 32-bit LE offset gets relaxed into a 12-bit LE offset when possible, but that's not changing the model. It really feels like they should have gone with IE and defined linker relaxations (something that even other architectures have here, albeit they pad with NOPs rather than deleting code) instead of this approach, but alas that's what we have.

Nov 19 2019, 7:00 AM · Restricted Project

Nov 18 2019

jrtc27 updated subscribers of D70398: [lld] Support copy relocations for TLS symbols.

I would agree that, for other architectures, not supporting copy relocations for TLS makes sense, and a world without copy relocations would be good. However, at the end of the day, this is what the RISC-V toolchains out there currently rely upon, and it would be very sad to force an incompatibility, especially since -fPIC introduces extra indirection in many cases that aren't needed, in particular because code generation has to assume symbol preemption. It does seem at least however that -fPIE yields the saner IE model again for both LLVM and GCC.... which begs the question why, as PIE vs non-PIE makes absolutely zero difference to the TLS addresses and semantics. I haven't yet found a discussion as to why this decision was made; the comment in the GCC source was the only reference I could even find that acknowledged this would require copy relocations. Ultimately, Clang+LLD can't currently be used to build FreeBSD as a result of this (nor can GCC+LLD), and telling people they can't use the default compiler mode that has always worked doesn't seem like the right answer; either LLVM and GCC need to stop forcing TLS copy relocations for position-dependent executables, or we need to support it here in LLD (or both...).

Nov 18 2019, 7:11 PM · Restricted Project
jrtc27 added a comment to D70398: [lld] Support copy relocations for TLS symbols.

Note that, for both GCC and LLVM, the default model used for TLS variables in position-depedent code is LE even if extern, relying on TLS copy relocations. GCC explicitly mentions this in gcc/config/riscv/riscv.c with /* Since we support TLS copy relocs, non-PIC TLS accesses may all use LE. */, and LLVM says // Non-PIC TLS lowering should always use the LocalExec model. (although perhaps that should mention *why* this is ok). Hence this is needed in order to link some real-world code.

Nov 18 2019, 11:53 AM · Restricted Project
jrtc27 added a comment to D70396: [Object][RISCV] Fix R_RISCV_SET6 and R_RISCV_SUB6 relocations resolution.

Yes that looks rather better. Especially SET6 which was previously an ADD6 (although that aspect was likely harmless as nothing sensible would put something there just to be clobbered by a SET6).

Nov 18 2019, 8:27 AM · Restricted Project
jrtc27 created D70398: [lld] Support copy relocations for TLS symbols.
Nov 18 2019, 8:00 AM · Restricted Project
jrtc27 added a comment to D70292: Make it possible to redirect not only errs() but also outs().

This broke -DBUILD_SHARED_LIBS=ON for me; lld::errs() lives in Common, but Core has also been modified to use it, and Common depends on Core, not vice versa:

Nov 18 2019, 4:25 AM · Restricted Project
jrtc27 added a comment to D70362: Fix fatal linker error on R_MIPS_JALR against a local TLS symbol.

R_MIPS_JALR really should not be being emitted here; its semantics are a hint to say that you are making a direct call to the referenced symbol (albeit by indirecting through a loaded GOT entry), but this is an indirect call. Interestingly, Local Dynamic is the only case where LLVM wants to emit the relocation; the three other models all leave it out, as does a normal global. This is because it happens to have the same structure as a direct call in the SelectionDAG representation, with the second operand for the instruction defining $t9 being a GlobalAddressSDNode, just this time an MO_DTPREL_LO rather than the expected MO_GOT_CALL (%call16)/MO_CALL_LO16 (%call_lo).

Nov 18 2019, 4:17 AM · Restricted Project
jrtc27 committed rG816ff985f51e: [Sparc] Fix "Cannot select" error for AtomicFence on 32-bit V9 (authored by jrtc27).
[Sparc] Fix "Cannot select" error for AtomicFence on 32-bit V9
Nov 18 2019, 1:47 AM
jrtc27 closed D69352: [Sparc] Fix "Cannot select" error for AtomicFence on 32-bit V9.
Nov 18 2019, 1:47 AM · Restricted Project

Nov 15 2019

jrtc27 added a comment to D69987: [RISCV] Assemble/Disassemble v-ext instructions..

I think we will want decoding tests as well, at least one for instruction.

One example follows:

# RUN: llvm-mc --disassemble --triple riscv64 -mattr +v < %s | \
# RUN:         FileCheck %s

# CHECK: vmerge.vvm v1, v3, v2, v0
[0xd7,0x00,0x31,0x5c]
Nov 15 2019, 10:41 AM · Restricted Project

Oct 29 2019

jrtc27 created D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types.
Oct 29 2019, 3:36 PM · Restricted Project

Oct 23 2019

jrtc27 created D69352: [Sparc] Fix "Cannot select" error for AtomicFence on 32-bit V9.
Oct 23 2019, 12:23 PM · Restricted Project

Oct 22 2019

jrtc27 added a comment to D54214: [RISCV] Set triple based on -march flag.

Ping, before I rebased this did anyone have any other thoughts on flag precedence?

Oct 22 2019, 9:09 AM · Restricted Project

Oct 21 2019

jrtc27 added a comment to D69212: [RISCV] Sign-extend 32-bit integer inline assembly operands on RV64I.

I was not aware of that aside in the spec, which does indeed suggest this specific case should sign-extend, and I guess gives us a reason why this isn't a slippery slope. However, even if this is the case, I still feel code shouldn't be relying on this, simply because it's unclear (and perhaps surprising to those who don't know this particular note from the spec). One might perfectly reasonably assume that both signed and unsigned i32 types (in the original source) are zero-extended, or (probably the default position of people) that the signed type is sign-extended and the unsigned type is zero-extended (which would require passing the extension information through via the asm call instruction's operands).

Oct 21 2019, 10:55 AM · Restricted Project

Oct 19 2019

jrtc27 added a comment to D69212: [RISCV] Sign-extend 32-bit integer inline assembly operands on RV64I.

So, I'm not sure whether this is actually something we want to make work or not. You can already end up with both inconsistencies between compilers and inconsistencies within a compiler for how 16-bit values get represented when passed as register operands, and the fact that we're going against the grain and making i32 an illegal type just makes that also apply to i32; it's nothing new, just more widespread. I wrote a bunch of stuff exploring how this is all already broken for i16 over on the corresponding FreeBSD revision that found this issue (https://reviews.freebsd.org/D22084#482766), but trying to support this sounds like opening a can of worms, and it seems likely that we will still end up with subtle differences, ones which might rear their heads only once there is sufficient inlining (and perhaps due to LTO), removing the sanitisation applied by the calling convention. Moreover, only guaranteeing GCC compatibility (and self-consistency) for i32 and iPTR (whatever that may be) across architectures seems a bit arbitrary. Yes, i32 is a bit special due to C's integer promotion rules, but it feels like a slippery slope towards standardising on i16 and i8 too. Yes, I went and wrote this patch, but having further explored the issue and thought about it, my inclination is that we should just tell people to not do this, and put in the explicit sign-extending or zero-extending casts they want in order to get an XLEN-sized value. Especially since there is not likely to be much RISC-V inline assembly out there at the moment that relies on this. What do others think? Perhaps Clang could gain more than the current warn_asm_mismatched_size_modifier (which is currently about whether the register you're getting due to your modifiers matches what your input is, e.g. if the input gets promoted to an i32 on AArch64 you should be using a W register with %w0 rather than %0; it's not warning about the promotion itself).

Oct 19 2019, 5:48 PM · Restricted Project

Oct 18 2019

jrtc27 created D69212: [RISCV] Sign-extend 32-bit integer inline assembly operands on RV64I.
Oct 18 2019, 7:20 PM · Restricted Project
jrtc27 added a comment to D68685: [RISCV] Scheduler description for Rocket Core.

Do we need separate schedule information for compressed and uncompressed instructions? I would assume that any sensible RVC implementation would decode the two to exactly the same control logic (other than for PC incrementing and the original instruction for mtval).

Oct 18 2019, 8:10 AM · Restricted Project
jrtc27 added a comment to D68685: [RISCV] Scheduler description for Rocket Core.

I wonder whether, instead of putting all the scheduling resource information as part of the instruction definition, we should be doing something like we do with patterns, ie declaring them separately (either in each RISCVInstrInfoX.td, or in RISCVSchedule.td to keep scheduling completely separate from encoding and codegen). For example (formatting aside):

Oct 18 2019, 6:25 AM · Restricted Project
jrtc27 added a comment to D68685: [RISCV] Scheduler description for Rocket Core.

If you have let UnsupportedFeatures = [IsRV32] (or similarly for RV64) in your model definition, you can avoid needing to list a bunch of scheduler resources with let Unsupported = 1;.

Oct 18 2019, 6:15 AM · Restricted Project

Oct 15 2019

jrtc27 committed rG1ab27c74d4b1: [lld][WebAssembly] Fix static linking of -fPIC code with external undefined data (authored by jrtc27).
[lld][WebAssembly] Fix static linking of -fPIC code with external undefined data
Oct 15 2019, 10:11 AM
jrtc27 committed rL374913: [lld][WebAssembly] Fix static linking of -fPIC code with external undefined data.
[lld][WebAssembly] Fix static linking of -fPIC code with external undefined data
Oct 15 2019, 10:11 AM
jrtc27 closed D68991: [lld][WebAssembly] Fix static linking of -fPIC code with external undefined data.
Oct 15 2019, 10:11 AM · Restricted Project
jrtc27 created D68991: [lld][WebAssembly] Fix static linking of -fPIC code with external undefined data.
Oct 15 2019, 7:59 AM · Restricted Project

Oct 8 2019

jrtc27 added a comment to D68559: [RISCV] Support fast calling convention.

I wonder whether it would be nicer to add CC_RISCV_FastCC as a function, much like CC_RISCV, instead, to match the current code style.

Oct 8 2019, 8:45 AM · Restricted Project
jrtc27 added a comment to D58531: [clang] Specify type of pthread_create builtin.

I like the patch and I think it is fine.

Small nits:
Could we have a test for " we can detect dodgy pthread_create declarations" and maybe pthread[_attr]_t?
There is an unresolved comment by @shrines.
@probinson: "it seems straightforward enough although clearly needs clang-format-diff run over it."

I'll accept this assuming the above points are easy to fix and given that no one expressed concerns but only positive comments were made.

Oct 8 2019, 8:26 AM · Restricted Project
jrtc27 updated the diff for D58531: [clang] Specify type of pthread_create builtin.

Rebased, added explicit no-warning test, clang-format'ed, updated assertion message.

Oct 8 2019, 8:26 AM · Restricted Project
jrtc27 committed rG67f542aba72f: [Diagnostics] Silence -Wsizeof-array-div for character buffers (authored by jrtc27).
[Diagnostics] Silence -Wsizeof-array-div for character buffers
Oct 8 2019, 4:33 AM
jrtc27 closed D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers.
Oct 8 2019, 4:33 AM · Restricted Project
jrtc27 committed rL374035: [Diagnostics] Silence -Wsizeof-array-div for character buffers.
[Diagnostics] Silence -Wsizeof-array-div for character buffers
Oct 8 2019, 4:33 AM
jrtc27 updated the diff for D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers.

Rebased

Oct 8 2019, 4:23 AM · Restricted Project

Oct 7 2019

jrtc27 committed rG66e276862781: [ItaniumMangle] Fix mangling of GNU __null in an expression to match GCC (authored by jrtc27).
[ItaniumMangle] Fix mangling of GNU __null in an expression to match GCC
Oct 7 2019, 10:27 PM
jrtc27 committed rL374013: [ItaniumMangle] Fix mangling of GNU __null in an expression to match GCC.
[ItaniumMangle] Fix mangling of GNU __null in an expression to match GCC
Oct 7 2019, 10:27 PM
jrtc27 committed rL374014: Request commit access for jrtc27.
Request commit access for jrtc27
Oct 7 2019, 10:27 PM
jrtc27 closed D68368: [ItaniumMangle] Fix mangling of GNU __null in an expression to match GCC.
Oct 7 2019, 10:27 PM · Restricted Project, Restricted Project