theraven (David Chisnall)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 19 2012, 2:56 AM (295 w, 2 d)

Recent Activity

Tue, Jul 10

theraven added a comment to D10480: Implement shared_mutex re: N4508.

I missed this when it went in and coming across the code now I'm quite surprised that it did. Why is shared_mutex not implemented as a wrapper around rwlocks (pthreads and Windows both provide this abstraction)? The current implementation looks a lot less efficient than the system version on any operating system that I'm familiar with and has the added effect that native_handle isn't possible to implement, meaning that these can't easily interoperate with C APIs either.

Tue, Jul 10, 3:45 AM

May 26 2018

theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 26 2018, 4:25 AM
theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 26 2018, 4:19 AM

May 22 2018

theraven committed rL332966: [objc-gnustep2] Use unsigned char to avoid potential UB in isalnum..
[objc-gnustep2] Use unsigned char to avoid potential UB in isalnum.
May 22 2018, 3:17 AM
theraven committed rL332963: Revert "Revert r332955 "GNUstep Objective-C ABI version 2"".
Revert "Revert r332955 "GNUstep Objective-C ABI version 2""
May 22 2018, 3:17 AM
theraven committed rL332965: [objc-gnu] Fix test..
[objc-gnu] Fix test.
May 22 2018, 3:17 AM
theraven committed rL332964: [objc-gnustep2] Use isalnum instead of a less efficient and nonportable….
[objc-gnustep2] Use isalnum instead of a less efficient and nonportable…
May 22 2018, 3:17 AM
theraven committed rC332965: [objc-gnu] Fix test..
[objc-gnu] Fix test.
May 22 2018, 3:17 AM
theraven committed rC332966: [objc-gnustep2] Use unsigned char to avoid potential UB in isalnum..
[objc-gnustep2] Use unsigned char to avoid potential UB in isalnum.
May 22 2018, 3:17 AM
theraven committed rC332964: [objc-gnustep2] Use isalnum instead of a less efficient and nonportable….
[objc-gnustep2] Use isalnum instead of a less efficient and nonportable…
May 22 2018, 3:17 AM
theraven committed rC332963: Revert "Revert r332955 "GNUstep Objective-C ABI version 2"".
Revert "Revert r332955 "GNUstep Objective-C ABI version 2""
May 22 2018, 3:17 AM
theraven added inline comments to rC332950: GNUstep Objective-C ABI version 2.
May 22 2018, 1:50 AM
theraven added inline comments to rC332950: GNUstep Objective-C ABI version 2.
May 22 2018, 1:38 AM
theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 22 2018, 1:31 AM
theraven added inline comments to rC332950: GNUstep Objective-C ABI version 2.
May 22 2018, 12:27 AM
theraven committed rL332955: Add cctype include..
Add cctype include.
May 22 2018, 12:26 AM
theraven committed rC332955: Add cctype include..
Add cctype include.
May 22 2018, 12:26 AM

May 21 2018

theraven committed rL332950: GNUstep Objective-C ABI version 2.
GNUstep Objective-C ABI version 2
May 21 2018, 11:13 PM
theraven committed rC332950: GNUstep Objective-C ABI version 2.
GNUstep Objective-C ABI version 2
May 21 2018, 11:13 PM
theraven closed D46052: GNUstep Objective-C ABI version 2.
May 21 2018, 11:13 PM

May 20 2018

theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.

Address review comments, including adding a size field for ivars.

May 20 2018, 11:02 AM
theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 20 2018, 11:01 AM

May 1 2018

theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 1 2018, 1:57 AM

Apr 28 2018

theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
Apr 28 2018, 4:41 AM
theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.
Apr 28 2018, 4:41 AM

Apr 26 2018

theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.

Fix the symbol names that I thought I'd done previously.

Apr 26 2018, 4:20 AM
theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.
  • Fixed non-use of isOptional (in all three places where it should have been used)
  • Reject -fobjc-runtime=gnustep-2.x in the driver for non-ELF targets
  • Fix a bug where the wrong variable was assigned in protocol generation, which triggered an assertion failure in one of the runtime's tests.
  • Rename protocol reference variables to prevent accidental conflicts with protocol references (this was already done for other indirection variables, but I'd missed protocols)
Apr 26 2018, 1:27 AM
theraven added a comment to D46052: GNUstep Objective-C ABI version 2.

Are you asking for a code review or a design review of the ABI?

Apr 26 2018, 1:17 AM

Apr 25 2018

theraven created D46052: GNUstep Objective-C ABI version 2.
Apr 25 2018, 5:17 AM

Apr 11 2018

theraven committed rL329882: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
ObjCGNU: Fix empty v3 protocols being emitted two fields short
Apr 11 2018, 11:51 PM
theraven committed rC329882: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
ObjCGNU: Fix empty v3 protocols being emitted two fields short
Apr 11 2018, 11:51 PM
theraven closed D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
Apr 11 2018, 11:51 PM · Restricted Project

Apr 8 2018

theraven added a comment to D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.

I think that we emit empty method lists so that the GCC runtime doesn't choke on them, though there's no reason why we couldn't with the GNUstep ABI. It would therefore be nice if the test failed if we did change to emitting null so that we could update the test at that point and check that it's actually doing the right thing.

Apr 8 2018, 9:57 AM · Restricted Project

Apr 7 2018

theraven added a comment to D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.

Isn't it better to test for the correct structure existing in the IR?

Apr 7 2018, 10:45 AM · Restricted Project

Apr 5 2018

theraven accepted D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
Apr 5 2018, 1:51 AM · Restricted Project
theraven added a comment to D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.

Looks correct to me. Protocols are only referenced by pointer, so it doesn't matter for old versions of the runtime if these fields are present.

Apr 5 2018, 1:45 AM · Restricted Project

Mar 17 2018

theraven added a comment to D44580: Sema: allow comparison between blocks & block-compatible objc types.

We seem to be missing tests for the assignment part of this patch.

Mar 17 2018, 1:58 AM · Restricted Project

Feb 8 2018

theraven accepted D37052: Add default address space for functions to the data layout (1/3).

Looks fine to me.

Feb 8 2018, 3:21 AM

Feb 7 2018

theraven added inline comments to D42123: Derive GEP index type from Data Layout.
Feb 7 2018, 2:54 AM

Feb 6 2018

theraven accepted D42123: Derive GEP index type from Data Layout.

Two very small nits (which I'd be happy to see fixed after commit, but might be easier to fix first), but otherwise it looks like a significantly cleaned-up version of what we have.

Feb 6 2018, 2:31 AM

Feb 1 2018

theraven added inline comments to D42123: Derive GEP index type from Data Layout.
Feb 1 2018, 3:49 AM

Jan 25 2018

theraven added a comment to D42123: Derive GEP index type from Data Layout.

I want to deprecate SCEVs for pointers if the index size is not equal to pointer size.
What do you think?

Jan 25 2018, 3:36 AM

Jan 24 2018

theraven committed rL323311: [Doc] Guideline on adding exception handling support for a target.
[Doc] Guideline on adding exception handling support for a target
Jan 24 2018, 1:54 AM
theraven closed D42178: [Doc] Guideline on adding exception handling support for a target.
Jan 24 2018, 1:54 AM

Jan 23 2018

theraven added a comment to D42178: [Doc] Guideline on adding exception handling support for a target.

I appear to have lost my credentials for LLVM svn. I've asked Chris to reset them, but someone else should feel free to commit this before then if they want to.

Jan 23 2018, 6:50 AM

Jan 22 2018

theraven added a comment to D42123: Derive GEP index type from Data Layout.

I don't like this patch as is, for several reasons.

  1. It's adding a hack that assumes that the offset should be the width of the widest integer operation. This is probably true in most cases (it is for us), but if we're going to introduce the idea that an address offset is distinct from the size of the pointer then we should do it properly and add that to the TargetInfo string explicitly (defaulting to the same size, if not specified).

So you propose to extend Data Layout string and add index size to it, right? It was one of options that Hal suggested. Ok.

Jan 22 2018, 6:18 AM
theraven accepted D42178: [Doc] Guideline on adding exception handling support for a target.

I'm inclined to recommend that you commit this. It's not perfect, but it's a *lot* better than what was there before, and hopefully having something there will encourage other people to improve it.

Jan 22 2018, 5:28 AM

Jan 19 2018

theraven requested changes to D42123: Derive GEP index type from Data Layout.

I don't like this patch as is, for several reasons.

Jan 19 2018, 4:20 AM

Jan 17 2018

theraven added a comment to D42178: [Doc] Guideline on adding exception handling support for a target.

Thank you very much for making a start on this!

Jan 17 2018, 6:50 AM

Oct 30 2017

theraven added a comment to D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size.

This makes things worse for us. On CHERI, [u]intptr_t is a (typedef for a) built-in type that can hold a capability. Having unw_word_t be uintptr_t

For understanding, I guess you meant "Having unw_word_t be uint64_t" here? Because othewise, that's exactly the change I'm doing - currently it's uint64_t while I'm proposing making it uintptr_t - that you're saying is easier to work with?

Oct 30 2017, 7:56 AM
theraven added a comment to D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size.

This makes things worse for us. On CHERI, [u]intptr_t is a (typedef for a) built-in type that can hold a capability. Having unw_word_t be uintptr_t made LLVM's libunwind considerably easier to port than the HP unwinder would have been, because uintptr_t is a type that is able to hold either any integer-register type or a pointer, whereas neither uint32_t nor uint64_t is. This will be true for any architecture that supports any kind of fat-pointer representation.

Oct 30 2017, 6:01 AM

Sep 21 2017

theraven added a comment to D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes..

Note: My n64 changes (and, by the look of it, the o32 changes) only supported the soft-float ABI and so need a check for defined(__mips_soft_float), and should still #error if we're not saving the floating point registers. We're currently using soft float for CHERI, I wasn't able to test a hard-float version (though it should be fairly trivial to add).

Sep 21 2017, 1:07 AM

Sep 7 2017

theraven added inline comments to D23566: [RISCV 8/10] Add support for all RV32I instructions.
Sep 7 2017, 12:53 AM

Sep 6 2017

theraven added a comment to D23566: [RISCV 8/10] Add support for all RV32I instructions.

I think that it's probably about time to move the RISC-V back end code to post-commit review.

Sep 6 2017, 2:48 AM

Aug 24 2017

theraven added inline comments to D37052: Add default address space for functions to the data layout (1/3).
Aug 24 2017, 9:21 AM

Aug 21 2017

theraven added inline comments to D36916: Associate functions with address spaces.
Aug 21 2017, 12:54 AM

Aug 10 2017

theraven added a comment to D31951: TableGen support for parameterized register class information.

Looking forward to this landing, as we currently have some horrible #ifdefs to support different register sizes. We also currently have a *really* ugly hack that allows intrinsics that take iPTR to map to multiple types. I wonder if the same infrastructure could be used to allow iPTR to map to multiple types depending on address space?

Aug 10 2017, 3:23 AM

Aug 9 2017

theraven added inline comments to D29933: [RISCV 11/n] Initial codegen support for ALU operations.
Aug 9 2017, 4:33 AM

May 15 2017

theraven added a comment to D31924: SROA: Allow eliminating addrspacecasted allocas.

For us, speculation isn't a problem. ptrtoint is not guaranteed to give stable results in all run-time environments (i.e. if we enable a copying GC), but it doesn't break the memory safety guarantees. inttoptr only works in some execution environments (and will result in a null where it wouldn't work), and it's up to the C programmer to ensure that they don't use it when it wouldn't be sensible and other front ends won't emit it at all. Code works as expected, as long as optimisers don't try to add them.

May 15 2017, 11:38 AM
theraven added a comment to D31924: SROA: Allow eliminating addrspacecasted allocas.

There was some discussion about non-integral address spaces at EuroLLVM. The current restriction is too great, as not allowing ptrtoint and inttoptr makes it impossible to support C-like languages. We discussed refining the definition to be that optimisers should not introduce inttoptr or ptrtoint, but that they are allowed to be inserted by the front end in places where they are valid in the context of the source language.

May 15 2017, 1:32 AM

Apr 21 2017

theraven added inline comments to D31924: SROA: Allow eliminating addrspacecasted allocas.
Apr 21 2017, 1:11 AM

Mar 30 2017

theraven added a comment to D27143: Fix detection of backtrace() availability on FreeBSD.

It's probably worth having Joerg look at this, as I believe a similar fix is needed on NetBSD.

Mar 30 2017, 3:20 AM

Feb 16 2017

theraven added a comment to D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC.

No, it's a bug in clang. Clang does not reject other functions that are used to implement builtins (if it did, compiler-rt would be a lot more difficult to build).

Feb 16 2017, 6:02 AM
theraven added a comment to D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC.

This code is working around something that's probably a clang bug. It would be better to fix the clang bug than add more complex workarounds.

Feb 16 2017, 2:19 AM

Feb 1 2017

theraven added a comment to D2808: Initial MIPS IV support. Separate out the is-MIPS64-ISA checks from is-64-bit-MIPS tests..

I think most of this was committed. I remember fixing a few merge conflicts where upstream had the wrong check, at least...

Feb 1 2017, 8:45 AM
theraven added a comment to D23568: [RISCV 10/10] Add common fixups and relocations.

@theraven @jyknight What's the state of this change?

Feb 1 2017, 6:52 AM

Oct 18 2016

theraven added a comment to D25431: [libcxx] Convert Solaris support library to C++ to fix -std=c++11 build .

Looks like a much better change to me.

Oct 18 2016, 7:15 AM

Oct 10 2016

theraven added a comment to D25431: [libcxx] Convert Solaris support library to C++ to fix -std=c++11 build .

It sounds as if the bug here is either that the .c files are being treated as c++, or that we're sticking -std=c++{whatever} in CFLAGS as well as CXXFLAGS. This is probably the bug that should be fixed.

Oct 10 2016, 6:06 AM

Oct 8 2016

theraven added inline comments to D23566: [RISCV 8/10] Add support for all RV32I instructions.
Oct 8 2016, 3:03 AM

Sep 27 2016

theraven resigned from D24954: [Driver] Disable OpenSUSE rules for OpenSUSE/SLES 10 and older.
Sep 27 2016, 2:38 AM

Sep 14 2016

theraven added a comment to D23561: [RISCV 4/10] Add basic RISCV{InstrFormats,InstrInfo,RegisterInfo,}.td.
In D23561#542222, @asb wrote:

On a somewhat longer-term note:

We have the exact same situation with HVX: the vector registers can be 64 or 128-byte long, depending on the processor mode. The actual encodings are identical between the two modes, so it's possible to have a single binary that would work in both modes. We have every HVX instruction defined twice, and separate register classes for both types of registers. This is a pain and a mess, and I have been planning to get rid of that for quite some time now.

Since this type of situation now appears in several targets, I hope that this will be enough of a rationale to develop a proper support for this issue, namely register class with a non-constant register size/alignment. This should be fairly simple, actually, and I can develop a prototype for review, hopefully in a few days.

Sep 14 2016, 8:03 AM

Aug 26 2016

theraven accepted D23567: [RISCV 9/10] Add support for disassembly.
Aug 26 2016, 7:14 AM
theraven added a comment to D23566: [RISCV 8/10] Add support for all RV32I instructions.

Comments inline.

Aug 26 2016, 6:59 AM
theraven accepted D23564: [RISCV 7/10] Add RISCVInstPrinter and basic MC assembler tests.

LGTM.

Aug 26 2016, 6:47 AM
theraven added inline comments to D23562: [RISCV 5/10] Add bare-bones RISC-V MCTargetDesc.
Aug 26 2016, 6:42 AM
theraven added a comment to D23561: [RISCV 4/10] Add basic RISCV{InstrFormats,InstrInfo,RegisterInfo,}.td.

Comments inline.

Aug 26 2016, 6:15 AM
theraven accepted D23558: [RISCV 2/10] Add RISC-V ELF defines.

I've not checked the relocations against the ABI doc, but assuming that they're correct it all looks good to me.

Aug 26 2016, 6:03 AM
theraven accepted D23557: [RISCV 1/10] Recognise riscv32 and riscv64 in triple parsing code.
Aug 26 2016, 6:00 AM
theraven added a comment to D23560: [RISCV 3/10] Add stub backend.

A couple of small nits inline.

Aug 26 2016, 5:58 AM

Jul 6 2016

theraven added a comment to D21329: Rename and rework `_LIBCPP_TRIVIAL_PAIR_COPY_CTOR`. Move FreeBSD configuration in-tree..

Looks fine to me, though I wonder if we want to move to the new ABI for FreeBSD11 and use the old one for <=10.

Jul 6 2016, 6:24 AM

Apr 16 2016

theraven added a comment to D11781: Refactored pthread usage in libcxx.

For us (ARM), a threads porting layer is important on several RTOSes (where a full-blown pthreads implementations is not available). I will see if I can publish one of those porting layer implementations, but perhaps a windows porting layer is more interesting to the community, in which case I'll look into hacking one up (unless of course, if @espositofulvio has already got one).

Apr 16 2016, 1:24 AM

Apr 4 2016

theraven added a comment to rL264966: [X86] Enable call frame optimization ("mov to push") not only for optsize….

5% overall on macrobenchmarks - adding a pipeline stall on every function entry and exit adds up a lot. Good to see that it doesn't happen on modern hardware, but I'd still be interested in seeing what impact it has on AMD, Centaur, or older Intel hardware.

Apr 4 2016, 10:09 PM
theraven added a comment to rL264966: [X86] Enable call frame optimization ("mov to push") not only for optsize….

According to the latest version of the Intel 64 and IA-32 Architectures Optimization Reference Manual, my concerns are not valid for the latest microarchitectures as the Stack Pointer Tracker eliminates the implicit dependencies. I am not sure that this applies to AMD / Centaur microarchitectures and how recent the addition of this to Intel chips is. I'd like to see some benchmarks, as this had a 5-10% performance penalty last time I looked at it.

Apr 4 2016, 9:17 AM
theraven added a comment to rL264966: [X86] Enable call frame optimization ("mov to push") not only for optsize….

What hardware was this change benchmarked on? The last time I looked at this was a few years ago, but back then the increased implicit hazards on %esp introduced noticeable performance regressions. Have recent CPUs optimised the microcode for this? The Intel optimisation guide used to explicitly say not to do this, but I've not checked recently.

Apr 4 2016, 4:29 AM

Mar 2 2016

theraven updated subscribers of D17270: Support arbitrary addrspace pointers in masked load/store intrinsics.
Mar 2 2016, 11:26 AM

Feb 19 2016

theraven added inline comments to D17413: [IR] Extend cmpxchg to allow pointer type operands.
Feb 19 2016, 4:16 AM

Nov 29 2015

theraven added inline comments to D7241: Partially fix memcpy / memset / memmove lowering in SelectionDAG construction if address space != 0..
Nov 29 2015, 8:44 AM

Oct 27 2015

theraven added a comment to D12678: [mips] Reserve address spaces 1-255 for software use..

We can maintain a patch locally, but please keep it in mind for future changes.

Oct 27 2015, 9:22 AM
theraven added a comment to D12678: [mips] Reserve address spaces 1-255 for software use..

I only just spotted this. Given that you know that we're using AS 200 for a hardware AS, it would have been nice to have been cc'd on this as a heads up.

Oct 27 2015, 2:51 AM

Sep 22 2015

theraven added a comment to D13051: Add placeholder __libcpp_relaxed_store() for when atomic builtins are not available..

Looks fine (though this probably won't actually work correctly on ARMv4).

Sep 22 2015, 4:34 AM

Aug 11 2015

theraven added inline comments to D11781: Refactored pthread usage in libcxx.
Aug 11 2015, 12:58 AM

Aug 10 2015

theraven added a comment to D11781: Refactored pthread usage in libcxx.

It would be nice to use rwlocks, but that's an ABI-breaking change. We'd need to bump the .so version and other annoyances. Definitely something I'd be in favour of seeing hidden behind some #ifdefs (and enabled for FreeBSD 11 and probably for the libc++ in packages), but it's beyond the scope of this set of changes.

Aug 10 2015, 9:50 AM

Aug 8 2015

theraven added a comment to D11781: Refactored pthread usage in libcxx.

This version looks much better (well, the previous one is cleaner, but this one looks like it shouldn't break the ABI - did you test this? It looks like it should work, but I've not actually tried it).

Aug 8 2015, 12:52 PM

Aug 7 2015

theraven added inline comments to D11781: Refactored pthread usage in libcxx.
Aug 7 2015, 1:38 AM

Aug 6 2015

theraven added inline comments to D11781: Refactored pthread usage in libcxx.
Aug 6 2015, 8:23 AM
theraven added inline comments to D11781: Refactored pthread usage in libcxx.
Aug 6 2015, 6:37 AM

Jun 4 2015

theraven added a comment to D10187: PR5172: Fix for a bug in pragma redefine_extname implementation..

Should be fine then, though I'm not sure if the code for the C11 atomic helpers is compiled by default (it needs to go in a shared lib to work).

Jun 4 2015, 10:37 AM
theraven added a comment to D10187: PR5172: Fix for a bug in pragma redefine_extname implementation..

I'm also not completely sure of the semantics - I just made it work well enough for the Solaris headers that libc++ needed to work. The changes look fine to me though.

Jun 4 2015, 5:15 AM

May 7 2015

theraven added a comment to D6096: Protection against stack-based memory corruption errors using SafeStack: compiler-rt runtime support library.

A couple of high-level comments:

May 7 2015, 2:24 AM

Mar 20 2015

theraven accepted D8082: Sema: Accept pointers to any address space for builtin functions.

Looks good to me! Thanks for working on this.

Mar 20 2015, 12:01 AM