Page MenuHomePhabricator

jrtc27 (Jessica Clarke)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

jrtc27 added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

Yeah my concern is not about m68k doing weird things, it's about the code that's added in the target-independent places to support m68k that I am most concerned with, as it affects every target regardless of whether m68k is enabled.

Exactly, that's why I prefer to keep the CodeBeads implementation for now (that only affects the m68k target) than refactor table-gen (that would affect all).

We should avoid mixing generic refactory with experimental code, unless the refactory is a clear benefit to all targets, not just the experimental one.

I concur with @RKSimon that we should approve this code as is and get the target in.

Fri, Jan 15, 10:22 AM · Restricted Project

Thu, Jan 14

jrtc27 added inline comments to D70401: [WIP][RISCV] Implement ilp32e ABI.
Thu, Jan 14, 4:08 PM · Restricted Project, Restricted Project
jrtc27 added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

FWIW I still think we're better off keeping (m68k only) CodeBeads rather than refactoring a lot of code that will affect mainstream targets. After m68k has been pushed, we can reinvestigate whether to keep CodeBeads (and whether other targets would benefit from using it), or moving m68k to a refactored TSFlag mechanism - we can make either a pre-requisite for it losing its experimental status if necessary.

I very much agree with that stance and I think this is the most straight-forward approach. I think getting the backend merged so becomes visible to a broader audience - both testers and developers - will help improve the quality and squeeze out more bugs. After all, it's supposed to be marked as experimental so it's expected to not be production-ready yet.

Thu, Jan 14, 11:12 AM · Restricted Project

Wed, Jan 13

jrtc27 added a comment to D91460: [AsmParser] make .ascii support spaces as separators.

Which GDB tests? How are they broken? I'm extremely surprised by the given this patch implements what binutils has done for a very long time (only .asciz/.string have changed behaviour in binutils, and those are not modified by this patch here so should not have regressions).

Wed, Jan 13, 2:48 PM · Restricted Project

Tue, Jan 12

jrtc27 added a comment to D94579: [RISCV] add the MC layer support of P extension.

Please make sure there are tests for invalid instructions (especially checking you have the immediate ranges and predicates correct).

Tue, Jan 12, 8:08 PM · Restricted Project
jrtc27 added inline comments to D94546: [RISCV] Optimize select_cc after fp compare expansion.
Tue, Jan 12, 12:59 PM · Restricted Project
jrtc27 added inline comments to D94535: [RISCV] Optimize Branch Comparisons.
Tue, Jan 12, 11:44 AM · Restricted Project
jrtc27 added a comment to D94475: [MC] Emit ELF files with ELFOSABI_GNU if specified in triple.

Having said that I do think it makes sense to push the getOS call into getOSABI by passing the full triple as it simplifies all the callers (and that also decouples the NFC refactor from your proposed behavioural change).

Tue, Jan 12, 10:00 AM · Restricted Project
jrtc27 added a comment to D94475: [MC] Emit ELF files with ELFOSABI_GNU if specified in triple.

Looking at the original motivation for the patch I'm not convinced that llvm-readelf etc shouldn't just fall back on displaying the "normal" set of GNU extensions even for ELFOSABI_NONE (i.e. those that FreeBSD and GNU have in common). Things like that are really just part of de-facto generic ELF these days. What does binutils do?

Tue, Jan 12, 9:58 AM · Restricted Project

Mon, Jan 11

jrtc27 added inline comments to D94465: [RISCV] Frame handling for RISC-V V extension. (2nd. version).
Mon, Jan 11, 11:16 PM · Restricted Project

Fri, Jan 8

jrtc27 added inline comments to D94345: [SPARC] Fix fp128 load/stores.
Fri, Jan 8, 3:35 PM · Restricted Project
jrtc27 added a comment to D94345: [SPARC] Fix fp128 load/stores.

The test is not so solid (is it?), ideally we should check the MI to make sure the load ops have the correct offset but I have no idea of what kind of test does that.

Fri, Jan 8, 3:30 PM · Restricted Project
jrtc27 added a comment to D94344: [CodeGen] Try to make the print of memory operand alignment a little more user friendly..

🎉 (looks sensible to me but don't have a huge amount of experience with MIR)

Fri, Jan 8, 3:23 PM · Restricted Project

Wed, Jan 6

jrtc27 added inline comments to D93619: [RISCV] Optimize multiplication with constant.
Wed, Jan 6, 10:00 PM · Restricted Project

Mon, Jan 4

jrtc27 added inline comments to D93619: [RISCV] Optimize multiplication with constant.
Mon, Jan 4, 10:57 PM · Restricted Project
jrtc27 added a comment to D93750: [RISCV] Frame handling for RISC-V V extension..

One thing I don't understand is why RVV incoming/outgoing arguments need their own stack slots. I assume they're just passed indirectly, so why do we need to distinguish between arguments and locals rather than just treat them like anything else that ends up being passed indirectly?

Mon, Jan 4, 5:35 PM · Restricted Project
jrtc27 added inline comments to D93750: [RISCV] Frame handling for RISC-V V extension..
Mon, Jan 4, 5:32 PM · Restricted Project
jrtc27 added a comment to D93298: [RISCV] add the MC layer support of Zfinx extension.

Your tests look like copies of the F/D/Zfh tests with not all the comments updated and instances of tests that just don't make sense for Zfinx. I only skimmed them and picked up a few issues, I haven't gone through them thoroughly, please do that yourself.

Mon, Jan 4, 3:47 AM · Restricted Project, Restricted Project

Sun, Jan 3

jrtc27 added inline comments to D93750: [RISCV] Frame handling for RISC-V V extension..
Sun, Jan 3, 5:59 PM · Restricted Project

Sat, Jan 2

jrtc27 added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

I was thinking of using a bit in the other flags member, Flags, as opposed to TSFlags. Then there would be nothing sneaky going on in the union itself.

Making it an anonymous union would mean that all current references to TSFlags would still work, correct?

Sat, Jan 2, 5:07 PM · Restricted Project
jrtc27 added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

@jrtc27 Will some sort of union of a uint64_t and a pointer work? One of the Flags bits could specify which one it is.

I agree that it would be cleaner to encode the information as flags, if possible.

Sat, Jan 2, 3:09 PM · Restricted Project
jrtc27 added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

If it isn't possible to encode the information in TSFlags, another possibility is to add a pointer to the MCInstrDesc class that points to some kind of auxiliary class/struct.

This actually gives me an idea to directly use the MCInstrDesc::TSFlags as a pointer to auxiliary data structures. That is:

auto* AuxTable =  reinterpret_cast<uint8_t*>(MD.TSFlags);
Sat, Jan 2, 2:52 PM · Restricted Project

Wed, Dec 30

jrtc27 added inline comments to D88391: [M68k] (Patch 5/8) Target lowering.
Wed, Dec 30, 12:57 PM · Restricted Project
jrtc27 added a comment to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.

I still don't understand what problem code beads are trying to solve that isn't already solved by existing backends like X86. Why can't you just assign operands to instruction encoding bits like a normal backend?

Wed, Dec 30, 12:51 PM · Restricted Project

Wed, Dec 23

jrtc27 added inline comments to D93750: [RISCV] Frame handling for RISC-V V extension..
Wed, Dec 23, 6:23 AM · Restricted Project

Tue, Dec 22

jrtc27 added inline comments to D93691: [RISCV] Add intrinsics for vf[n]macc/vf[n]msac/vf[n]madd/vf[n]msub instructions.
Tue, Dec 22, 11:12 AM · Restricted Project
jrtc27 added inline comments to D93691: [RISCV] Add intrinsics for vf[n]macc/vf[n]msac/vf[n]madd/vf[n]msub instructions.
Tue, Dec 22, 11:10 AM · Restricted Project
jrtc27 added a comment to D52050: [Driver] Fix architecture triplets and search paths for Linux x32.

What gets done currently for i386? That suffers potentially the same problem given both /usr/lib/gcc/x86_64-linux-gnu/$V/32 and /usr/lib/gcc/i386-linux-gnu/$V are possible locations for an i386 GCC toolchain.

Very good suggestion. I will look into that tomorrow. Thanks for the pointer!

I have been wrapping my head around this for some time and I have run into one problem trying to apply the suggested approach.

The problem is that I don't know how to tell whether I'm on an x86_64 system or an x32 system there is no `case llvm::Triple::x32:` which would be needed here (we have it for x86).

x32 takes the switch case for x86_64, so x32 and x86_64 targeting x32 are identical.

However, that's not the same as whether we're on an x86_64 system or on an x32 system determines which GNU triplet to use and which include and library search paths are our primary ones.

Tue, Dec 22, 9:52 AM · Restricted Project

Mon, Dec 21

jrtc27 accepted D92097: [RISCV] Basic jump table lowering.

I agree with @lenary, I think this is fine to land (with the whitespace fix :)) and if it turns out the threshold is too low then we'll soon find out and can adjust accordingly (but I imagine not, 5 feels like a sensible default). Or if it's too high!

Mon, Dec 21, 5:12 PM · Restricted Project
jrtc27 added inline comments to D93624: [RISCV] Fix rounding mode in lowering of float operations.
Mon, Dec 21, 9:46 AM · Restricted Project
jrtc27 added a comment to D93624: [RISCV] Fix rounding mode in lowering of float operations.

Doesn't this render fesetround useless?

No. fesetround sets dynamic rounding mode (register frm), which may be any mode. However in the case of non-default rounding mode (all other than RNE), floating operations must be represented by constrained intrinsics, for example llvm.experimental.constrained.fadd should be used instead of fadd.

Mon, Dec 21, 7:00 AM · Restricted Project
jrtc27 added a comment to D93624: [RISCV] Fix rounding mode in lowering of float operations.

Doesn't this render fesetround useless?

Mon, Dec 21, 4:58 AM · Restricted Project

Sun, Dec 20

jrtc27 added inline comments to D88391: [M68k] (Patch 5/8) Target lowering.
Sun, Dec 20, 11:14 AM · Restricted Project
jrtc27 added inline comments to D88390: [M68k] (Patch 4/8) MC layer and object file support.
Sun, Dec 20, 11:14 AM · Restricted Project
jrtc27 added inline comments to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.
Sun, Dec 20, 11:13 AM · Restricted Project
jrtc27 added inline comments to D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k.
Sun, Dec 20, 11:02 AM · Restricted Project
jrtc27 added inline comments to D88393: [cfe][M68k] (Patch 7/8) Basic Clang support.
Sun, Dec 20, 10:56 AM · Restricted Project
jrtc27 added inline comments to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.
Sun, Dec 20, 10:53 AM · Restricted Project
jrtc27 added a comment to D88392: [M68k] (Patch 6/8) IR Tests.

I see very little by way of testing the return side of the calling convention, and nothing for sret.

Sun, Dec 20, 10:50 AM · Restricted Project
jrtc27 added inline comments to D88390: [M68k] (Patch 4/8) MC layer and object file support.
Sun, Dec 20, 10:46 AM · Restricted Project
jrtc27 added inline comments to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.
Sun, Dec 20, 10:46 AM · Restricted Project
jrtc27 added inline comments to D88386: [MIR][M68K] (Patch 2/8): Changes on Target-independent MIR part.
Sun, Dec 20, 9:49 AM · Restricted Project
jrtc27 added inline comments to D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.
Sun, Dec 20, 9:44 AM · Restricted Project

Fri, Dec 18

jrtc27 accepted D93536: [RISCV] Assume no-op addrspacecasts by default.
Fri, Dec 18, 11:37 AM · Restricted Project

Dec 17 2020

jrtc27 accepted D91460: [AsmParser] make .ascii support spaces as separators.

Looks good to me, let's see what the others think.

Dec 17 2020, 2:50 PM · Restricted Project
jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 17 2020, 2:50 PM · Restricted Project
jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 17 2020, 1:42 PM · Restricted Project

Dec 15 2020

jrtc27 added a comment to D93298: [RISCV] add the MC layer support of Zfinx extension.

Firstly, please generate your diffs with full context (-U with a sufficiently-large number). Secondly, can we avoid having to do a bunch of duplication with some clever use of multiclasses for F/D/Zfh and pseudos? Though maybe it's small enough that the duplication is easier to reason about than an obfuscated abstracted version.

Dec 15 2020, 7:26 AM · Restricted Project, Restricted Project

Dec 12 2020

jrtc27 added inline comments to D92842: [RFC][SelectionDAG] Add Target-Independent Compiler Barrier.
Dec 12 2020, 4:01 PM · Restricted Project

Dec 11 2020

jrtc27 added inline comments to D92105: [RISCV] Add pre-emit pass to make more instructions compressible.
Dec 11 2020, 9:59 AM · Restricted Project
jrtc27 added inline comments to D92105: [RISCV] Add pre-emit pass to make more instructions compressible.
Dec 11 2020, 9:57 AM · Restricted Project

Dec 10 2020

jrtc27 added inline comments to D92842: [RFC][SelectionDAG] Add Target-Independent Compiler Barrier.
Dec 10 2020, 11:07 AM · Restricted Project
jrtc27 added inline comments to D92842: [RFC][SelectionDAG] Add Target-Independent Compiler Barrier.
Dec 10 2020, 10:49 AM · Restricted Project
jrtc27 added inline comments to D93013: [RISCV] Define vadd/vsub/vrsub intrinsics and lower to V instructions..
Dec 10 2020, 10:03 AM · Restricted Project

Dec 8 2020

jrtc27 added a comment to D52050: [Driver] Fix architecture triplets and search paths for Linux x32.

i.e. can we not just support both approaches and prefer x86_64-linux-gnux32 if it exists?

Dec 8 2020, 2:24 PM · Restricted Project
jrtc27 added a comment to D52050: [Driver] Fix architecture triplets and search paths for Linux x32.

I've been able to check what Ubuntu 20.10 offers in terms of x32 support. Its kernel supports x32 binaries, it provides x32 versions of core system libraries in separate packages (e.g. libc6-x32, libx32stdc++6, libx32z1), and it provides a compiler that targets x32 by default (gcc-x86-64-linux-gnux32).

I did that, too. In fact, Ubuntu is identical to Debian in this regard as both the Ubuntu and the Debian gcc packages are maintained by the same maintainer (Matthias Klose whom I also happen to know personally) who first uploads these packages to Debian unstable, then syncs to Ubuntu.

However:

These Ubuntu packages do not use the Debian/Ubuntu multiarch approach: the packages are completely independent of the corresponding x64 and i386 versions with separate names, and nothing in Ubuntu installs any libraries into any /lib/x86_64-linux-gnux32-like path. They are intended to allow x32 cross compilation on an Ubuntu system, not intended to act as a basis for running an x32 Ubuntu system. This appears to be very different from Debian's x32 support. That said, cross-compiled binaries do run on Ubuntu and it should be possible to build an x32-native LLVM with the Ubuntu-provided toolchain.

Well, Debian has both as - as I already mentioned - the gcc packages in Debian and Ubuntu are the same. The only difference is that Ubuntu does not provide an x32 port so there is no possibility to install Ubuntu x32 packages in the commonly known MultiArch manner.

Since MultiArch and the -cross packages are somewhat redundant, I'm not so sure yet which approach to address this issue would be best. I will talk to Matthias regarding this.

Dec 8 2020, 2:23 PM · Restricted Project
jrtc27 added a comment to D92841: [RISCV][NFC] Regenerate RISCV CodeGen tests.

@jrtc27 it sounds like adding @plt into all the tests won't break anything, and will catch if future changes default to non-plt calls, so this change is an improvement?

Dec 8 2020, 1:06 PM · Restricted Project
jrtc27 added a comment to D92841: [RISCV][NFC] Regenerate RISCV CodeGen tests.

The spills look right to me. The question we're not sure of is the appearance of the @plt suffixes on all the external calls.

Dec 8 2020, 7:00 AM · Restricted Project
jrtc27 added a comment to D92832: [compiler-rt] Allow atomic_test to pass on x86_64.

Note that this shows there is currently another bug in x86_64 clang (ignoring OS, since with manual -march=athlon64 on macOS you can also get the non-macOS behaviour). Whether or not libatomic is used is part of the ABI, as libatomic may (and likely will for 16B atomics on your standard Linux distro) use hash tables of locks to emulate atomic operations, which will not correctly synchronise with the inlined hardware atomics. See https://godbolt.org/z/fqfvev for evidence of the ABI breakage.

Dec 8 2020, 4:24 AM · Restricted Project

Dec 7 2020

jrtc27 accepted D91270: [Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct.
Dec 7 2020, 12:08 PM · Restricted Project
jrtc27 added inline comments to D92715: [Clang][RISCV] Define RISC-V V builtin types.
Dec 7 2020, 3:33 AM · Restricted Project

Dec 4 2020

jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 4 2020, 3:22 PM · Restricted Project
jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 4 2020, 3:09 PM · Restricted Project
jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 4 2020, 2:35 PM · Restricted Project

Dec 3 2020

jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 3 2020, 5:34 PM · Restricted Project
jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 3 2020, 4:40 PM · Restricted Project
jrtc27 accepted D92538: [RISCV] Merge FMV_H_X_RV32/FMV_H_X_RV64 into a single opcode. Same with FMV_X_ANYEXTH_RV32/RV64.

Ah, I hadn't realised the F comment got lost in 741b04b0b7912611a8a5b7e74462e87b8930a116, I was looking at our fork which is currently still based on 11. This looks good to me now.

Dec 3 2020, 10:56 AM · Restricted Project
jrtc27 added inline comments to D92538: [RISCV] Merge FMV_H_X_RV32/FMV_H_X_RV64 into a single opcode. Same with FMV_X_ANYEXTH_RV32/RV64.
Dec 3 2020, 9:34 AM · Restricted Project

Dec 2 2020

jrtc27 accepted D90738: [RISCV] Support Zfh half-precision floating-point extension..

Yes that looks like everything now; thanks!

Dec 2 2020, 10:05 AM · Restricted Project
jrtc27 added a comment to D52050: [Driver] Fix architecture triplets and search paths for Linux x32.

Hmm, I was pretty sure that autoconf can deal with x32 inside an x32 chroot.

Most autoconf-using software won't be dealing with a copy of config.guess that was last updated in 2011. Updating that to a more recent version should also work. :)

Dec 2 2020, 9:17 AM · Restricted Project
jrtc27 added a comment to D92479: [RISCV] remove redundant instruction when eliminate frame index.

It's also wrong if the ADDI has FrameReg as its destination. A neater approach might be to always use the destination register (if there is one, using general instruction query information) as the destination for the ADD, keeping the scratch register for the movImm call. Then you'd only need special logic to work out whether you need to keep the (modified) existing instruction.

Dec 2 2020, 7:06 AM · Restricted Project
jrtc27 added a comment to D92479: [RISCV] remove redundant instruction when eliminate frame index.

This is a bit ugly. Surely a simple peephole optimisation should be able to fix this and any other cases more generally? But also please give this a better title and commit message.

Dec 2 2020, 6:56 AM · Restricted Project

Dec 1 2020

jrtc27 added inline comments to D90738: [RISCV] Support Zfh half-precision floating-point extension..
Dec 1 2020, 7:28 PM · Restricted Project
jrtc27 accepted D92293: [RISCVAsmParser] Allow a SymbolRef operand to be a complex expression.
Dec 1 2020, 3:22 PM · Restricted Project
jrtc27 added a comment to D90738: [RISCV] Support Zfh half-precision floating-point extension..

I thought I'd gone through it thoroughly last time but apparently not :( oh well hopefully the last set of nits.

Dec 1 2020, 12:54 AM · Restricted Project

Nov 30 2020

jrtc27 added a comment to D91270: [Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct.

Seems good other than additional comments regarding code clarity.

Nov 30 2020, 3:45 PM · Restricted Project
jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Nov 30 2020, 3:00 PM · Restricted Project
jrtc27 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Nov 30 2020, 3:00 PM · Restricted Project
jrtc27 added a comment to D71124: [RISCV] support clang driver to select cpu.

@khchen ,
how should I obtain available (v)CPU features?
Any sample code, that should work on RISC-V?

Nov 30 2020, 11:52 AM · Restricted Project, Restricted Project
jrtc27 added inline comments to D92293: [RISCVAsmParser] Allow a SymbolRef operand to be a complex expression.
Nov 30 2020, 10:34 AM · Restricted Project
jrtc27 accepted D90738: [RISCV] Support Zfh half-precision floating-point extension..

Couple of minor changes but otherwise LGTM.

Nov 30 2020, 9:45 AM · Restricted Project

Nov 29 2020

jrtc27 added inline comments to D92293: [RISCVAsmParser] Allow a SymbolRef operand to be a complex expression.
Nov 29 2020, 10:56 PM · Restricted Project

Nov 28 2020

jrtc27 added a comment to D92269: [TableGen] Eliminate the 'code' type.

Yes, we could create !codeconcat instead, along with !codeinterleave, !codeeq, etc. We could also just extend the existing bang operators to work on the code type. I thought it was cleaner just to get rid of the distinction.

Nov 28 2020, 4:51 PM · Restricted Project, Restricted Project, Restricted Project
jrtc27 added a comment to D92269: [TableGen] Eliminate the 'code' type.

Losing types and permitting nonsensical operators like !subst on code seems a bit sad. Why not just define a !codeconcat, like how we have a separate !listconcat? I agree that the TypeOf_xxx stuff is really ugly and so having _greater_ expressivity like !codeconcat would go a long way to help that. Also, cast<code>(str) would be the natural way to express the cases where you still really do want to construct code as a string without needing TypeOf_xxx.

Nov 28 2020, 2:23 PM · Restricted Project, Restricted Project, Restricted Project

Nov 26 2020

jrtc27 added a comment to D92097: [RISCV] Basic jump table lowering.

This seems fine to me now other than the outstanding discussion about code size.

Nov 26 2020, 8:55 AM · Restricted Project
jrtc27 added a comment to D92097: [RISCV] Basic jump table lowering.

You still appear to have the old patch that doesn't change addDisp.

Nov 26 2020, 8:45 AM · Restricted Project
jrtc27 added inline comments to D83059: [RISCV] Use Generated Instruction Uncompresser.
Nov 26 2020, 8:42 AM · Restricted Project
jrtc27 added inline comments to D92097: [RISCV] Basic jump table lowering.
Nov 26 2020, 7:26 AM · Restricted Project
jrtc27 added inline comments to D92097: [RISCV] Basic jump table lowering.
Nov 26 2020, 7:19 AM · Restricted Project
jrtc27 added inline comments to D92097: [RISCV] Basic jump table lowering.
Nov 26 2020, 7:17 AM · Restricted Project
jrtc27 added inline comments to D92097: [RISCV] Basic jump table lowering.
Nov 26 2020, 5:09 AM · Restricted Project
jrtc27 added inline comments to D92097: [RISCV] Basic jump table lowering.
Nov 26 2020, 5:01 AM · Restricted Project
jrtc27 requested changes to D92097: [RISCV] Basic jump table lowering.
Nov 26 2020, 4:57 AM · Restricted Project

Nov 25 2020

jrtc27 requested changes to D90738: [RISCV] Support Zfh half-precision floating-point extension..
Nov 25 2020, 4:07 PM · Restricted Project
jrtc27 added inline comments to D92105: [RISCV] Add pre-emit pass to make more instructions compressible.
Nov 25 2020, 8:08 AM · Restricted Project

Nov 24 2020

jrtc27 added inline comments to D91314: [RISCV] ELF attribute for Zfh extension..
Nov 24 2020, 3:19 AM · Restricted Project
jrtc27 added inline comments to D90738: [RISCV] Support Zfh half-precision floating-point extension..
Nov 24 2020, 2:06 AM · Restricted Project

Nov 23 2020

jrtc27 added a comment to D91428: Add support for multiple program address spaces.

Currently P0-P1 is valid and results in the program address space being 1, but this patch changes the semantics of that. How sure are you that nothing will break? I do not like the idea of reusing existing valid syntax to mean something else; if you want to introduce some notion of secondary program address spaces then the syntax should be different.

Nov 23 2020, 12:21 PM · Restricted Project, Restricted Project
jrtc27 added a comment to D90738: [RISCV] Support Zfh half-precision floating-point extension..

My intuition is that there should be tests for the various instructions that are only hard-float ABIs, and tests specifically for all the different calling conventions that use a very limited set of instructions.

Nov 23 2020, 9:21 AM · Restricted Project
jrtc27 added a comment to D91717: [RISCV][compiler-rt] Add support for save-restore.

It seems a bit excessive to me to coalesce the entry points into bundles of 4. Do you have any particular benchmarking data or reasoning that supports choosing that threshold?
Also, shouldn't this implementation include CFI directives?

I used bundles of 4 just to follow the behaviour I saw in libgcc, and the grouping of 2 for rv64 seemed a bit too fine-grained. I'm not sure what the original justification for the coalescing into groups of 2/4 was in libgcc.

I'll update to account for other suggested changes and see if I can find any benchmarks which show the tradeoff for the grouping threshold

Nov 23 2020, 6:34 AM · Restricted Project