Page MenuHomePhabricator

jrtc27 (Jessica Clarke)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Fri, Apr 9

jrtc27 added a comment to D99605: [TableGen] [Docs] Add lldb-tblgen to command guide; add 4 stubs for xxx-tblgen commands.

I pushed this patch.

Fri, Apr 9, 7:10 AM · Restricted Project

Thu, Apr 8

jrtc27 added a comment to D99605: [TableGen] [Docs] Add lldb-tblgen to command guide; add 4 stubs for xxx-tblgen commands.

Do you think it's necessary to have the change of document title reviewed in Phabricator?

Thu, Apr 8, 2:18 PM · Restricted Project
jrtc27 added inline comments to D99605: [TableGen] [Docs] Add lldb-tblgen to command guide; add 4 stubs for xxx-tblgen commands.
Thu, Apr 8, 2:15 PM · Restricted Project
jrtc27 added inline comments to D99605: [TableGen] [Docs] Add lldb-tblgen to command guide; add 4 stubs for xxx-tblgen commands.
Thu, Apr 8, 2:12 PM · Restricted Project
jrtc27 added inline comments to D99605: [TableGen] [Docs] Add lldb-tblgen to command guide; add 4 stubs for xxx-tblgen commands.
Thu, Apr 8, 1:46 PM · Restricted Project
jrtc27 added inline comments to D99605: [TableGen] [Docs] Add lldb-tblgen to command guide; add 4 stubs for xxx-tblgen commands.
Thu, Apr 8, 1:43 PM · Restricted Project
jrtc27 added inline comments to D99605: [TableGen] [Docs] Add lldb-tblgen to command guide; add 4 stubs for xxx-tblgen commands.
Thu, Apr 8, 1:43 PM · Restricted Project
jrtc27 accepted D100083: [RISCV] Add InstAlias for Zbb Zbp and Zbs extension.
Thu, Apr 8, 11:14 AM · Restricted Project

Tue, Apr 6

jrtc27 added inline comments to D99988: [compiler-rt] Add platform detection support for x32.
Tue, Apr 6, 1:23 PM · Restricted Project

Fri, Apr 2

jrtc27 committed rG1bd4986e7cdc: [Sema] Fix Windows build after b001d574d7d9 (authored by jrtc27).
[Sema] Fix Windows build after b001d574d7d9
Fri, Apr 2, 12:29 PM
jrtc27 added a comment to D99009: [RISCV] [1/2] Add intrinsic for Zbr extension.

Looks like this doesn't build on windows: http://45.33.8.238/win/36271/step_4.txt

Please take a look and revert for now if it takes a while to fix.

Fri, Apr 2, 12:29 PM · Restricted Project, Restricted Project
jrtc27 added a comment to D99009: [RISCV] [1/2] Add intrinsic for Zbr extension.

Thanks, and good point re ordering

Fri, Apr 2, 11:20 AM · Restricted Project, Restricted Project
jrtc27 added inline comments to D99009: [RISCV] [1/2] Add intrinsic for Zbr extension.
Fri, Apr 2, 11:08 AM · Restricted Project, Restricted Project

Wed, Mar 31

jrtc27 added a comment to D98936: [RISCV] DAG nodes and pseudo instructions for CSR access.
In D98936#2661228, @asb wrote:

Are there ever any cases where you _wouldn't_ want a CSR-specific pseudo in order to have control over the scheduling of it specifically? This feels a bit like a middle-ground that's the worst of both worlds to me.

Hi @jrtc27 - could you please elaborate a little on the concern that the approach in this path might be the worst of both worlds? I'm not sure I fully follow. Thanks.

If you want to have different scheduling for different CSRs, do you not need per-CSR pseudos in order to express that? This diff is ostensibly to allow for that in future, but is at a much coarser read/write/swap granularity, so doesn't really get you much over and above just scheduling CSRRW itself as a whole, just adds more complexity for little gain. IMO any CSRs we need scheduling info for should just get their own dedicated read/write/swap pseudos as and when they're needed.

The patch D99083 demonstrates the solution for FP state/control resisters. Every register and every access get separate pseudos, each of which can have their own scheduling properties.

That looks like the kind of thing I'm imagining. So can we remove the Read/Write/Swap_CSR defs from this diff and just keep the Read/Write/SwapSysReg classes for use with such pseudos?

No problem. I just thought that putting Read/Write/Swap_CSR defs (which are general purpose definitions) into a patch specific for FP support is not quite logical. But if this is OK, I'll move them.

Wed, Mar 31, 7:58 AM · Restricted Project
jrtc27 added a comment to D98936: [RISCV] DAG nodes and pseudo instructions for CSR access.
In D98936#2661228, @asb wrote:

Are there ever any cases where you _wouldn't_ want a CSR-specific pseudo in order to have control over the scheduling of it specifically? This feels a bit like a middle-ground that's the worst of both worlds to me.

Hi @jrtc27 - could you please elaborate a little on the concern that the approach in this path might be the worst of both worlds? I'm not sure I fully follow. Thanks.

If you want to have different scheduling for different CSRs, do you not need per-CSR pseudos in order to express that? This diff is ostensibly to allow for that in future, but is at a much coarser read/write/swap granularity, so doesn't really get you much over and above just scheduling CSRRW itself as a whole, just adds more complexity for little gain. IMO any CSRs we need scheduling info for should just get their own dedicated read/write/swap pseudos as and when they're needed.

The patch D99083 demonstrates the solution for FP state/control resisters. Every register and every access get separate pseudos, each of which can have their own scheduling properties.

Wed, Mar 31, 7:49 AM · Restricted Project
jrtc27 added a comment to D98936: [RISCV] DAG nodes and pseudo instructions for CSR access.
In D98936#2661228, @asb wrote:

Are there ever any cases where you _wouldn't_ want a CSR-specific pseudo in order to have control over the scheduling of it specifically? This feels a bit like a middle-ground that's the worst of both worlds to me.

Hi @jrtc27 - could you please elaborate a little on the concern that the approach in this path might be the worst of both worlds? I'm not sure I fully follow. Thanks.

Wed, Mar 31, 5:54 AM · Restricted Project

Sun, Mar 28

jrtc27 added a comment to D99468: [TableGen] Emit more helpful error messages on empty type set.

The fact that this is under NDEBUG at all seems rather dubious

Sun, Mar 28, 8:15 AM · Restricted Project

Wed, Mar 24

jrtc27 added inline comments to D99151: [RISCV][Clang] Add RVV vleff intrinsic functions..
Wed, Mar 24, 10:06 AM · Restricted Project
jrtc27 added inline comments to D99151: [RISCV][Clang] Add RVV vleff intrinsic functions..
Wed, Mar 24, 10:06 AM · Restricted Project
jrtc27 added inline comments to D99151: [RISCV][Clang] Add RVV vleff intrinsic functions..
Wed, Mar 24, 6:22 AM · Restricted Project

Tue, Mar 23

jrtc27 added inline comments to D99040: [RISCV] Add scheduler classes for the Zba and Zbb extensions..
Tue, Mar 23, 1:54 PM · Restricted Project
jrtc27 added inline comments to D99158: [RISCV][WIP] Implement intrinsics for P extension.
Tue, Mar 23, 7:05 AM · Restricted Project, Restricted Project
jrtc27 added inline comments to D95588: [RISCV] Implement the MC layer support of P extension.
Tue, Mar 23, 6:57 AM · Restricted Project

Mon, Mar 22

jrtc27 added a comment to D99082: [RISCV][NFC] Fix RVV intrinsic tests..

Most likely because you're adding assembly tests in Clang, which won't work if the backend isn't present (and needs a REQUIRES line). Assembly tests in Clang are generally bad practice and best avoided for that reason. If you do want them it's probably best to split them out into their own tests so people can still run the frontend tests regardless.

Mon, Mar 22, 7:21 PM · Restricted Project
jrtc27 added inline comments to D99009: [RISCV] [1/2] Add intrinsic for Zbr extension.
Mon, Mar 22, 6:16 PM · Restricted Project, Restricted Project
jrtc27 added a comment to D98936: [RISCV] DAG nodes and pseudo instructions for CSR access.

Are there ever any cases where you _wouldn't_ want a CSR-specific pseudo in order to have control over the scheduling of it specifically? This feels a bit like a middle-ground that's the worst of both worlds to me.

Mon, Mar 22, 11:02 AM · Restricted Project

Sat, Mar 20

jrtc27 accepted D99026: [RISCV] Add isel pattern to optimize (mul (and X, 0xffffffff), (and Y, 0xffffffff)) on RV64.
Sat, Mar 20, 1:01 PM · Restricted Project
jrtc27 added a comment to D99026: [RISCV] Add isel pattern to optimize (mul (and X, 0xffffffff), (and Y, 0xffffffff)) on RV64.

I think the final line of your commit message is the wrong way round?

Maybe it was poorly written. How does it look now?

Sat, Mar 20, 1:00 PM · Restricted Project
jrtc27 added a comment to D99026: [RISCV] Add isel pattern to optimize (mul (and X, 0xffffffff), (and Y, 0xffffffff)) on RV64.

I think the final line of your commit message is the wrong way round?

Sat, Mar 20, 12:45 PM · Restricted Project
jrtc27 added a comment to D99009: [RISCV] [1/2] Add intrinsic for Zbr extension.

Please fix the style issues and update the patch with full context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface, or use arc)

Sat, Mar 20, 3:29 AM · Restricted Project, Restricted Project

Fri, Mar 19

jrtc27 added a comment to D98929: Add missing cases in RISCVMCExpr::getVariantKindName.

I think these are just for debug output? Though maybe they appear in MIR too?

Fri, Mar 19, 9:29 AM · Restricted Project
jrtc27 added a comment to D98936: [RISCV] DAG nodes and pseudo instructions for CSR access.

CSR addresses are uimm12s not simm12s

Fri, Mar 19, 4:46 AM · Restricted Project

Thu, Mar 18

jrtc27 added a comment to D98881: [RISCV] Fix mcount name.

This should really be for all OSes on RISC-V, which I think means copying it N times? :/

Thu, Mar 18, 2:19 PM · Restricted Project
jrtc27 added a comment to D98113: [Driver] Also search FilePaths for compiler-rt before falling back.

I just looked at this again and I don't have the full context in my mind right now but won't the test just exercise the BareMetal toolchain and not your changes?

Thu, Mar 18, 9:00 AM · Restricted Project

Wed, Mar 17

jrtc27 added inline comments to D98821: [RISCV] Improve 64-bit integer materialization for some cases..
Wed, Mar 17, 3:12 PM · Restricted Project
jrtc27 added a comment to D98659: [MachineCopyPropagation] Do more backward copy propagations.

I don't know why it only affects microMIPS (presumably it does its lowering subtly differently from normal MIPS) but those changes look sensible to me

Wed, Mar 17, 7:38 AM · Restricted Project

Mon, Mar 15

jrtc27 added inline comments to D98669: [RISCV] Make empty name symbols SF_FormatSpecific so that llvm-symbolizer ignores them for symbolization.
Mon, Mar 15, 4:39 PM · Restricted Project
jrtc27 accepted D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'.
Mon, Mar 15, 3:44 PM · Restricted Project, Restricted Project
jrtc27 added inline comments to D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'.
Mon, Mar 15, 11:39 AM · Restricted Project, Restricted Project
jrtc27 added inline comments to D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'.
Mon, Mar 15, 11:11 AM · Restricted Project, Restricted Project
jrtc27 added inline comments to D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'.
Mon, Mar 15, 9:45 AM · Restricted Project, Restricted Project
jrtc27 added a comment to D98616: [RISCV] Add inline asm constraint 'v' in Clang for RISC-V 'V'..

GCC use vr for vector register and vm for vector mask register.

How does that even work? Aren't multi character strings a set of options?

Mon, Mar 15, 7:10 AM · Restricted Project
jrtc27 added a comment to D98616: [RISCV] Add inline asm constraint 'v' in Clang for RISC-V 'V'..

GCC use vr for vector register and vm for vector mask register.

Mon, Mar 15, 7:05 AM · Restricted Project

Sun, Mar 14

jrtc27 added a comment to D98616: [RISCV] Add inline asm constraint 'v' in Clang for RISC-V 'V'..

This seems like the obvious choice for the constraint, but it would be good to ensure there's consensus with GCC people, especially since their assembly constraints are intimately tied to their instruction patterns (or, really, the assembly constraints just expose those and we pretend to be GCC) so they have less flexibility than us.

Sun, Mar 14, 7:53 PM · Restricted Project
jrtc27 added inline comments to D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'.
Sun, Mar 14, 6:41 PM · Restricted Project, Restricted Project

Sat, Mar 13

jrtc27 requested changes to D98574: [Sparc] Define the same macros for -mcpu=v9 as GCC on Linux and the BSDs.
Sat, Mar 13, 4:53 PM · Restricted Project

Fri, Mar 12

jrtc27 accepted D98542: [RISCV] Teach normaliseSetCC to canonicalize X > -1 to X >= 0 and X < 1 to 0 >= X..
Fri, Mar 12, 11:43 AM · Restricted Project
jrtc27 added a comment to D98542: [RISCV] Teach normaliseSetCC to canonicalize X > -1 to X >= 0 and X < 1 to 0 >= X..

Improve comment and try to improve function name to encapsulate new behavior.

Fri, Mar 12, 11:42 AM · Restricted Project
jrtc27 added a comment to D98542: [RISCV] Teach normaliseSetCC to canonicalize X > -1 to X >= 0 and X < 1 to 0 >= X..

Hm, this seems like a bit of an abuse of what the function is intended to do, at least with its current name and comment, as it's now not just doing normalisation for mapping to the set of supported instructions but also optimising at the same time.

Fri, Mar 12, 11:33 AM · Restricted Project
jrtc27 accepted D98449: [RISCV] Add isel-patterns to optimize (a < 1) into blez (a <= 0).

This feels like it should be a target-independent DAGCombine?

DAG combine and instcombine canonicalize to constants on the RHS and SETGT, SETLT, SETUGT, and SETULT. So X <= 0 will always be rewritten to X< 1.

Fri, Mar 12, 8:24 AM · Restricted Project
jrtc27 added a comment to D98449: [RISCV] Add isel-patterns to optimize (a < 1) into blez (a <= 0).

This feels like it should be a target-independent DAGCombine?

Fri, Mar 12, 5:33 AM · Restricted Project

Mar 11 2021

jrtc27 added inline comments to D98307: [RISCV] Remap 'generic' CPU to 'generic-rv32' or 'generic-rv64'. Validate 64Bit feature against the triple..
Mar 11 2021, 11:39 AM · Restricted Project
jrtc27 added inline comments to D98307: [RISCV] Remap 'generic' CPU to 'generic-rv32' or 'generic-rv64'. Validate 64Bit feature against the triple..
Mar 11 2021, 11:13 AM · Restricted Project
jrtc27 added inline comments to D98307: [RISCV] Remap 'generic' CPU to 'generic-rv32' or 'generic-rv64'. Validate 64Bit feature against the triple..
Mar 11 2021, 10:26 AM · Restricted Project
jrtc27 added inline comments to D98136: [RISCV][RFC] Initially support the K-extension instructions on the LLVM MC layer.
Mar 11 2021, 4:25 AM · Restricted Project

Mar 10 2021

jrtc27 added inline comments to D98307: [RISCV] Remap 'generic' CPU to 'generic-rv32' or 'generic-rv64'. Validate 64Bit feature against the triple..
Mar 10 2021, 6:02 PM · Restricted Project
jrtc27 added inline comments to D98307: [RISCV] Remap 'generic' CPU to 'generic-rv32' or 'generic-rv64'. Validate 64Bit feature against the triple..
Mar 10 2021, 6:00 PM · Restricted Project
jrtc27 accepted D98379: [RISCV] Add additional checking to tablgen RISCVVEmitter requested in D95016..

Any TableGen backend error that's a real message and not an assertion failure with a backtrace is a win in my books :)

Mar 10 2021, 4:55 PM · Restricted Project
jrtc27 added inline comments to D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics..
Mar 10 2021, 1:38 PM · Restricted Project, Restricted Project
jrtc27 added inline comments to D98205: [builtins] Fix ABI-incompatibility with GCC for floating-point compare.
Mar 10 2021, 11:12 AM · Restricted Project

Mar 9 2021

jrtc27 requested changes to D98236: [RISCV] Add SiFive-VIU75 for llvm.
Mar 9 2021, 7:35 AM · Restricted Project

Mar 8 2021

jrtc27 added a comment to D98113: [Driver] Also search FilePaths for compiler-rt before falling back.

One thing that is currently rather ugly about BareMetal is that it ignores the AddArch argument. Once you have a sysroot, the architecture suffix is rather unnecessary, and given compiler-rt uses LLVM_ENABLE_PER_TARGET_RUNTIME_DIR to decide both whether to add a suffix and whether to add lib/${OS} to the install path, that means you can't nicely point compiler-rt at the sysroot as the install directory (well, unless you do something like make COMPILER_RT_OUTPUT_DIR point at the sysroot not the libdir and set COMPILER_RT_OS_DIR to . (or maybe the empty string also works)).

Agree that setting the compiler-rt installation directory is not ideal. I remember I ended up doing some post installation steps to make sure that libraries were in right folder.

Regarding this patch, currently Baremetal toolchain looks for compiler-rt in a multilib specific directory. Does that not solve the problem of finding compiler-rt for any given arch/abi combination automatically?

Mar 8 2021, 8:32 AM · Restricted Project

Mar 7 2021

jrtc27 added a comment to D88391: [M68k] (Patch 5/8) Target lowering.

@jrtc27 Do you have any more feedback before I accept this and we let the experimental backend be pushed to trunk?

Mar 7 2021, 6:42 PM · Restricted Project
jrtc27 accepted D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.
Mar 7 2021, 11:39 AM · Restricted Project
jrtc27 accepted D88391: [M68k] (Patch 5/8) Target lowering.
Mar 7 2021, 11:39 AM · Restricted Project
jrtc27 added inline comments to D98124: [RISCV] Clean up parsing fence arguments.
Mar 7 2021, 11:35 AM · Restricted Project
jrtc27 added inline comments to D98124: [RISCV] Clean up parsing fence arguments.
Mar 7 2021, 10:28 AM · Restricted Project
jrtc27 added a comment to D98136: [RISCV][RFC] Initially support the K-extension instructions on the LLVM MC layer.

Hm, having SHA-2 acceleration but not SHA-3 in 2021 is a bit surprising to me

Mar 7 2021, 8:00 AM · Restricted Project
jrtc27 added inline comments to D88391: [M68k] (Patch 5/8) Target lowering.
Mar 7 2021, 7:27 AM · Restricted Project
jrtc27 added inline comments to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.
Mar 7 2021, 7:24 AM · Restricted Project

Mar 6 2021

jrtc27 requested review of D98125: [RISCV] Clean up parsing floating point rounding mode.
Mar 6 2021, 12:03 PM · Restricted Project
jrtc27 requested review of D98124: [RISCV] Clean up parsing fence arguments.
Mar 6 2021, 11:47 AM · Restricted Project
jrtc27 added a comment to D98113: [Driver] Also search FilePaths for compiler-rt before falling back.

One thing that is currently rather ugly about BareMetal is that it ignores the AddArch argument. Once you have a sysroot, the architecture suffix is rather unnecessary, and given compiler-rt uses LLVM_ENABLE_PER_TARGET_RUNTIME_DIR to decide both whether to add a suffix and whether to add lib/${OS} to the install path, that means you can't nicely point compiler-rt at the sysroot as the install directory (well, unless you do something like make COMPILER_RT_OUTPUT_DIR point at the sysroot not the libdir and set COMPILER_RT_OS_DIR to . (or maybe the empty string also works)).

Mar 6 2021, 9:25 AM · Restricted Project
jrtc27 updated the diff for D98113: [Driver] Also search FilePaths for compiler-rt before falling back.

... and another accidental change

Mar 6 2021, 9:18 AM · Restricted Project
jrtc27 updated the diff for D98113: [Driver] Also search FilePaths for compiler-rt before falling back.

Dropped unintended change

Mar 6 2021, 9:17 AM · Restricted Project
jrtc27 updated the diff for D98113: [Driver] Also search FilePaths for compiler-rt before falling back.

Added missing file for test

Mar 6 2021, 9:16 AM · Restricted Project
jrtc27 requested review of D98113: [Driver] Also search FilePaths for compiler-rt before falling back.
Mar 6 2021, 7:35 AM · Restricted Project

Mar 5 2021

jrtc27 committed rG5d6e0e474e86: [benchmark] Replace references to M680x0 with M68k (authored by jrtc27).
[benchmark] Replace references to M680x0 with M68k
Mar 5 2021, 5:04 PM
jrtc27 added inline comments to D97916: [Driver][RISCV] Support parsing multi-lib config from GCC..
Mar 5 2021, 4:57 AM · Restricted Project
jrtc27 added a comment to D94163: [RISCV] Set dependency on floating point CSRs, 1/3.
In D94163#2603738, @asb wrote:

We discussed this briefly in the RISC-V call as I noted this patchset has been sat open for some time. One thing that might be helpful is whether you could say a little bit more about the goal for this patchset.

Most floating point instructions set accrued exception bits in fflags register. If an instruction is specified with dynamic rounding mode, it also depends on the content of frm register. So in the following code:

csrwi  frm, a1
fadd.d ft2, ft2, ft3

changing the order of instruction is not allowed, because fadd.d depends on the content of frm, which is changed by the previous instruction. Similarly, the code:

fadd.d ft2, ft2, ft3
csrrs t0, fflags, zero

does not allow to change the order of the instructions, as crsrs reads content of fflags, which is set by the first instruction.

Now nothing prevents the compiler from changing the order of instructions in these examples. Existing instruction definitions are unable to express these dependencies. It does not allow to write programs that use non-default floating point environment, for example, dynamic rounding mode.

The goal of this patchset is to establish means to express such dependencies.

Once this lands, what's the next step?

This is the first and necessary step toward full-fledged implementation of floating point support. In particular it would allow progress in D91242 and D90854. Actually this work was undertaken because the lack of dependencies prevents implementation of llvm.set.rounding (https://reviews.llvm.org/D91242#2400476). The next steps, of course, include support of constrained intrinsics for RSCV.

Is there some relevant RFC,

If RFC can facilitate review of this patchset, I will prepare it.

or equivalent changes being made to other in-tree architectures?

Targets that support full-fledged floating point operations add implicit uses and definitions of FP state and control register(s). For example:
• X86: https://github.com/llvm/llvm-project/blob/bc172e532a89754d47fef1306064a26a4dc0a76b/llvm/lib/Target/X86/X86InstrFPStack.td#L728 (see let Defs = [FPSW] and let Uses = [FPCW]),
• PowerPC: https://github.com/llvm/llvm-project/blob/e7361c8eccb7663146096622549dc03240414157/llvm/lib/Target/PowerPC/PPCInstrInfo.td#L3169 (see Uses = [RM]),
• SystemZ: https://github.com/llvm/llvm-project/blob/9e28b89827a3be4ab602b40c263839665af06b4a/llvm/lib/Target/SystemZ/SystemZInstrFP.td#L434 (see let Uses = [FPC])

We're also still unclear about the advantage of changing codegen to default to a static rounding mode (which might be a surprising change, as all software compiled to date on both GCC and LLVM has used used the dynamic rounding mode by default).

Instructions in assembler without explicit rounding mode specification get dynamic rounding mode as now. Lowering of FP operations like fadd uses static rounding mode RNE, because these operations assume default floating point environment (https://llvm.org/docs/LangRef.html#floating-point-environment). Using static rounding mode has some advantages over assuming frm to have particular value. The code that requires default rounding mode does not require setting rfm in a program where some pieces uses non-default rounding mode. Such code works as designed even if it is called from a region where other rounding mode is set. Such implementation simplifies implementation of things like #pragma STDC FENV_ROUND and make programs more robust.

Mar 5 2021, 4:52 AM · Restricted Project

Mar 4 2021

jrtc27 added inline comments to D97970: [RISCV] Make the hasStdExtM() check in RISCVInstrInfo::getVLENFactoredAmount emit a diagnostic rather than an assert..
Mar 4 2021, 11:51 AM · Restricted Project
jrtc27 updated subscribers of D97961: [Cost]Canonicalize the cost for logical or/and reductions..
Mar 4 2021, 9:14 AM · Restricted Project

Mar 3 2021

jrtc27 accepted D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k.
Mar 3 2021, 6:24 AM · Restricted Project
jrtc27 accepted D88392: [M68k] (Patch 6/8) IR Tests.
Mar 3 2021, 6:23 AM · Restricted Project
jrtc27 added a comment to D88391: [M68k] (Patch 5/8) Target lowering.

Still a lot of instances of bool isFoo variables/arguments that should be bool IsFoo.

Mar 3 2021, 6:22 AM · Restricted Project
jrtc27 accepted D88390: [M68k] (Patch 4/8) MC layer and object file support.
Mar 3 2021, 6:15 AM · Restricted Project
jrtc27 added a comment to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.

Hopefully the last set of comments for this patch

Mar 3 2021, 6:15 AM · Restricted Project
jrtc27 accepted D88386: [MIR][M68K] (Patch 2/8): Changes on Target-independent MIR part.
Mar 3 2021, 6:06 AM · Restricted Project
jrtc27 accepted D88385: [TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions.
Mar 3 2021, 6:06 AM · Restricted Project

Feb 28 2021

jrtc27 added inline comments to D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics..
Feb 28 2021, 10:46 AM · Restricted Project, Restricted Project
jrtc27 added inline comments to D88391: [M68k] (Patch 5/8) Target lowering.
Feb 28 2021, 8:06 AM · Restricted Project
jrtc27 accepted D88393: [cfe][M68k] (Patch 7/8) Basic Clang support.
Feb 28 2021, 7:38 AM · Restricted Project
jrtc27 added inline comments to D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files.
Feb 28 2021, 7:27 AM · Restricted Project

Feb 26 2021

jrtc27 added a comment to D97606: [Clang interpreter] Avoid storing pointers at unaligned locations.

Is there any way I can usefully test this? As far as I can tell there's only a single constexpr test in the tree that uses the new interpreter, and it's pretty trivial?

Feb 26 2021, 7:35 PM · Restricted Project
jrtc27 added a comment to D64146: [Clang Interpreter] Initial patch for the constexpr interpreter.

CodePtr points into the bytecode emitted by the byte code compiler. In some instances, pointers to auxiliary data structures are embedded into the byte code, such as functions or AST nodes which contain information relevant to the execution of the instruction.

Would it help if instead of encoding pointers, the byte code encoded some integers mapped to the original objects?

I've read through the code and have slightly more understanding now. It seems there are several options:

  1. Keep the pointers somewhere on the side and put an integer in the byte code, like you suggest
  1. Pad values in the byte code to their natural alignment in general (and ensure the underlying std::vector<char> gets its storage allocated at an aligned boundary / use a different container), though this can get a little weird as the amount of padding between consecutive arguments varies depending on where you are (unless you force realignment to the max alignment at the start of a new opcode)
  1. Make the byte code be an array of uintptr_t instead of packing it like is done currently, with care needed on ILP32; that can either just use uint64_t instead and we declare CHERI unsupported for 32-bit architectures (which is unlikely to be a problem as you probably want a 64-bit virtual address space if the doubling pointer size, with 64-bit CHERI capabilities on 32-bit VA systems being only for embedded use) or you can split 64-bit integers into two 32-bit integers and treat them as two arguments

1 works but feels ugly. 2 or 3 would be my preference, and mirror how "normal" interpreters work, though those might split the code and data so they can keep the opcodes as, say, 32-bit integers, but the stack full of native word/pointer slots; my inclination is that 3 is the best option as it looks like the simplest. How do you feel about each of those? Is memory overhead from not packing values a concern?

Hm, though I see the "store an ID" pattern is common for dynamic things and this should be quite rare, so maybe that is indeed the right approach, mirroring something like getOrCreateGlobal?

Feb 26 2021, 7:34 PM · Restricted Project, Restricted Project
jrtc27 updated the diff for D97606: [Clang interpreter] Avoid storing pointers at unaligned locations.

Reworked code slightly to make it look nicer after clang-format uglified it

Feb 26 2021, 7:30 PM · Restricted Project
jrtc27 requested review of D97606: [Clang interpreter] Avoid storing pointers at unaligned locations.
Feb 26 2021, 7:27 PM · Restricted Project
jrtc27 added a comment to D64146: [Clang Interpreter] Initial patch for the constexpr interpreter.

CodePtr points into the bytecode emitted by the byte code compiler. In some instances, pointers to auxiliary data structures are embedded into the byte code, such as functions or AST nodes which contain information relevant to the execution of the instruction.

Would it help if instead of encoding pointers, the byte code encoded some integers mapped to the original objects?

I've read through the code and have slightly more understanding now. It seems there are several options:

  1. Keep the pointers somewhere on the side and put an integer in the byte code, like you suggest
  1. Pad values in the byte code to their natural alignment in general (and ensure the underlying std::vector<char> gets its storage allocated at an aligned boundary / use a different container), though this can get a little weird as the amount of padding between consecutive arguments varies depending on where you are (unless you force realignment to the max alignment at the start of a new opcode)
  1. Make the byte code be an array of uintptr_t instead of packing it like is done currently, with care needed on ILP32; that can either just use uint64_t instead and we declare CHERI unsupported for 32-bit architectures (which is unlikely to be a problem as you probably want a 64-bit virtual address space if the doubling pointer size, with 64-bit CHERI capabilities on 32-bit VA systems being only for embedded use) or you can split 64-bit integers into two 32-bit integers and treat them as two arguments

1 works but feels ugly. 2 or 3 would be my preference, and mirror how "normal" interpreters work, though those might split the code and data so they can keep the opcodes as, say, 32-bit integers, but the stack full of native word/pointer slots; my inclination is that 3 is the best option as it looks like the simplest. How do you feel about each of those? Is memory overhead from not packing values a concern?

Feb 26 2021, 5:42 PM · Restricted Project, Restricted Project
jrtc27 committed rG9e0d55024d4e: [clang][NFC] Clean up whitespace in ClangOpcodesEmitter output (authored by jrtc27).
[clang][NFC] Clean up whitespace in ClangOpcodesEmitter output
Feb 26 2021, 5:30 PM
jrtc27 added a comment to D64146: [Clang Interpreter] Initial patch for the constexpr interpreter.

CodePtr points into the bytecode emitted by the byte code compiler. In some instances, pointers to auxiliary data structures are embedded into the byte code, such as functions or AST nodes which contain information relevant to the execution of the instruction.

Would it help if instead of encoding pointers, the byte code encoded some integers mapped to the original objects?

Feb 26 2021, 4:44 PM · Restricted Project, Restricted Project