Page MenuHomePhabricator

arichardson (Alexander Richardson)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 4 2015, 4:18 PM (171 w, 4 d)

Recent Activity

Thu, Dec 13

arichardson added inline comments to D55483: Introduce the callback attribute and emit !callback metadata.
Thu, Dec 13, 1:35 AM

Wed, Dec 12

arichardson added a comment to D42790: [ELF] Check that Elf_Rela addends are always written with -r.

Is this patch still needs to be applied?

Wed, Dec 12, 10:03 AM

Mon, Dec 10

arichardson added inline comments to D55212: Handle alloc_size attribute on function pointers.
Mon, Dec 10, 12:04 PM
arichardson added a reviewer for D55212: Handle alloc_size attribute on function pointers: arphaman.
Mon, Dec 10, 12:04 PM
arichardson added inline comments to D55212: Handle alloc_size attribute on function pointers.
Mon, Dec 10, 8:29 AM
arichardson updated the diff for D55212: Handle alloc_size attribute on function pointers.

fix typo

Mon, Dec 10, 8:28 AM
arichardson updated the diff for D55212: Handle alloc_size attribute on function pointers.

clang-format
fix a failing test

Mon, Dec 10, 8:27 AM

Thu, Dec 6

arichardson updated the diff for D55212: Handle alloc_size attribute on function pointers.

Add a C++ test for alloc_size

Thu, Dec 6, 12:46 PM
arichardson added inline comments to D55212: Handle alloc_size attribute on function pointers.
Thu, Dec 6, 5:23 AM

Mon, Dec 3

arichardson added a comment to D55212: Handle alloc_size attribute on function pointers.

Thanks for the review! I'll write a C++ test tomorrow.

Mon, Dec 3, 12:23 PM
arichardson updated the diff for D55212: Handle alloc_size attribute on function pointers.
  • address review comments
  • add test that we can assign between function pointers with and without alloc_size attribute.
Mon, Dec 3, 8:07 AM
arichardson added inline comments to D55212: Handle alloc_size attribute on function pointers.
Mon, Dec 3, 8:02 AM
arichardson updated the diff for D55212: Handle alloc_size attribute on function pointers.

Remove RUN: line added for debugging

Mon, Dec 3, 4:51 AM
arichardson created D55212: Handle alloc_size attribute on function pointers.
Mon, Dec 3, 4:49 AM

Wed, Nov 28

arichardson created D55001: [CMake] Fix build with -DLLVM_TOOL_LLVM_{MCA/EXEGESIS}_BUILD=OFF.
Wed, Nov 28, 6:12 AM

Nov 14 2018

arichardson committed rCXX346878: [libcxx] [test] Fix running tests on macOS with python3.
[libcxx] [test] Fix running tests on macOS with python3
Nov 14 2018, 10:02 AM
arichardson committed rL346878: [libcxx] [test] Fix running tests on macOS with python3.
[libcxx] [test] Fix running tests on macOS with python3
Nov 14 2018, 10:01 AM
arichardson closed D54522: Fix running tests on macOS with python3.
Nov 14 2018, 10:01 AM
arichardson updated the diff for D54522: Fix running tests on macOS with python3.

remove debug message

Nov 14 2018, 4:50 AM
arichardson added inline comments to D54522: Fix running tests on macOS with python3.
Nov 14 2018, 4:49 AM
arichardson created D54522: Fix running tests on macOS with python3.
Nov 14 2018, 4:48 AM

Nov 13 2018

arichardson abandoned D38167: [ELF] Ensure that .init/.fini sections are padded with nops instead of traps.

LLD should not attempt to workaround bugs in crt*.o files

Nov 13 2018, 3:03 AM · lld
arichardson updated the diff for D40016: Use ImplicitConversionSequence::setAsIdentityConversion(QualType). NFC.

rebased on latest HEAD

Nov 13 2018, 3:01 AM
arichardson committed rL346751: Fix .cfi_restore with register numbers > 64.
Fix .cfi_restore with register numbers > 64
Nov 13 2018, 2:57 AM
arichardson closed D54420: Fix .cfi_restore with register numbers > 64.
Nov 13 2018, 2:57 AM
arichardson committed rL346750: Fix modules build of AVRAsmParser.cpp.
Fix modules build of AVRAsmParser.cpp
Nov 13 2018, 2:57 AM
arichardson closed D53425: Fix modules build of AVRAsmParser.cpp.
Nov 13 2018, 2:57 AM

Nov 12 2018

arichardson added a comment to D54424: [DWARF] Do not use PRIx32 for printing uint64_t values.

Any chance of a test case?

I hope the following test case starts to pass on mips buildbot:
http://lab.llvm.org:8011/builders/llvm-mips-linux/builds/3739/steps/test-llvm/logs/FAIL%3A%20LLVM%3A%3Adebug_addr.ll

Could this also be tested cross-platform (on any platform that's building the mips backend)? (ie: a version of this test that 'REQUIRES: ' the mips target and specifically targets it)

I cannot figure out how to do that. In fact we test various libc. The following code (compiled by gcc and clang) works fine on x86_64 in both 32/64-bit executables but shows incorrect output on mips 32-bit.

uint64_t V = 1;
printf("0x%8.8" PRIx32 "\n", V);

Nov 12 2018, 1:01 PM
arichardson created D54420: Fix .cfi_restore with register numbers > 64.
Nov 12 2018, 3:29 AM
arichardson added a comment to rL346520: Use the correct address space when emitting the ctor function list.

We have also made quite a few of these changes in https://github.com/CTSRD-CHERI/clang.
We place both functions and globals in AS200 (eventually I would like to move functions to a separate AS but that's quite difficult right now) so we get lots of errors due to implicit address space 0.
In fact I made changes to completely remove all uses of PointerType::getUnqual() in clang and most of them in LLVM. I might make sense for you to have a look at our changes too. I would like to upstream all of these changes but I am currently too busy to clean up the patches. Since program and alloca AS are identical for us there might be some cases where we use them interchangeably.

Nov 12 2018, 1:50 AM

Nov 7 2018

arichardson added a comment to D54200: [PPC64] Use INT32_MIN instead of std::numeric_limits<int32_t>::min().

I agree with James that numeric_limits is clearer. I've run into bugs before because there was a 0/f missing when trying to use max/min value integers.

Nov 7 2018, 3:51 AM

Nov 1 2018

arichardson added a comment to D53425: Fix modules build of AVRAsmParser.cpp.

ping?

Nov 1 2018, 4:41 AM

Oct 23 2018

arichardson accepted D53566: Fix warning: extra ‘;’ [-Wpedantic].

Ah yes looks like I made a copy paste error when changing the lambda to a function and it compiled fine for me so I didn't notice.

Oct 23 2018, 11:07 AM

Oct 20 2018

arichardson committed rLLD344842: Add an addAbsolute static function to Writer.cpp.
Add an addAbsolute static function to Writer.cpp
Oct 20 2018, 4:17 AM
arichardson committed rL344842: Add an addAbsolute static function to Writer.cpp.
Add an addAbsolute static function to Writer.cpp
Oct 20 2018, 4:15 AM
arichardson closed D53393: Add a addAbsolute static function to Writer.cpp.
Oct 20 2018, 4:14 AM

Oct 19 2018

arichardson added a comment to D53393: Add a addAbsolute static function to Writer.cpp.

Thank you very much for the detailed explanation.

Oct 19 2018, 2:39 AM
arichardson created D53425: Fix modules build of AVRAsmParser.cpp.
Oct 19 2018, 2:33 AM

Oct 18 2018

arichardson added a comment to rL344305: Remove SymbolTable::addAbsolute()..

Yes, this is totally fine. I do not expect any kind of API stability and I think removing code that is not used is a good thing.
I have submitted D53393 as a possible solution that in my opinion increases readability and allows reducing differences to upstream.
If you don't think my patch is acceptable I can also revert this commit in our fork but I would very much like to keep our diff as small as possible to make it easier to upstream any changes.

Oct 18 2018, 6:41 AM
arichardson created D53393: Add a addAbsolute static function to Writer.cpp.
Oct 18 2018, 6:04 AM
arichardson added a comment to rL344305: Remove SymbolTable::addAbsolute()..

We were using addAbsolute() outside of this function in our CHERI fork. I can certainly change those uses to Symtab->addDefined(Name, STV_HIDDEN, STT_NOTYPE, 0, 0, STB_GLOBAL, nullptr, nullptr) but I find the addAbsolute() makes the code more easy to understand.
I also find that this change makes addReservedSymbols() less readable.

Oct 18 2018, 5:56 AM

Sep 7 2018

arichardson added inline comments to D50560: [LLD] Enable Visual Studio compatible diagnostics..
Sep 7 2018, 6:29 AM · lld

Aug 23 2018

arichardson committed rC340522: Update avr attributes test for output change in r340519.
Update avr attributes test for output change in r340519
Aug 23 2018, 3:22 AM
arichardson committed rL340522: Update avr attributes test for output change in r340519.
Update avr attributes test for output change in r340519
Aug 23 2018, 3:22 AM
arichardson committed rL340519: Allow creating llvm::Function in non-zero address spaces.
Allow creating llvm::Function in non-zero address spaces
Aug 23 2018, 2:26 AM
arichardson closed D47541: Allow creating llvm::Function in non-zero address spaces.
Aug 23 2018, 2:26 AM

Jul 24 2018

arichardson updated the diff for D47541: Allow creating llvm::Function in non-zero address spaces.

Add parseOptionalProgrammAddrSpace() as suggested
Update tests

Jul 24 2018, 7:56 AM
arichardson committed rL337824: Stop wrapping __has_include in another macro.
Stop wrapping __has_include in another macro
Jul 24 2018, 5:41 AM
arichardson committed rCXX337824: Stop wrapping __has_include in another macro.
Stop wrapping __has_include in another macro
Jul 24 2018, 5:41 AM
arichardson closed D49067: Stop wrapping __has_include in another macro.
Jul 24 2018, 5:41 AM
arichardson closed D49067: Stop wrapping __has_include in another macro.
Jul 24 2018, 5:41 AM

Jul 23 2018

arichardson added a comment to D49067: Stop wrapping __has_include in another macro.

Where are the special lexing rules specified?

Jul 23 2018, 3:37 PM
arichardson added a comment to D49091: Warn about usage of __has_include/__has_include_next in macro expansions.

Ping?

Jul 23 2018, 3:05 PM
arichardson added a comment to D49067: Stop wrapping __has_include in another macro.

Ping?

Jul 23 2018, 3:03 PM

Jul 13 2018

arichardson added a comment to D49084: FileCheck: Add support for variable expressions.

I case people are interested I have also implemented something similar in our fork of llvm. I chose [[@EXPR var + 0x42]] as the syntax. I also added some functions such as [[@EXPR hex(VAR)]].
Variables can either by referenced by name or if they conflict with one of the builtin functions using ${x} syntax: [[@EXPR toupper(hex(${hex}))]]

Simple functions are also on my wish-list! The main ones we care about are converting to and from hex numbers, and being able to do additions and subtractions. But I think just the basic integer and hex reading and additions would be a good start.

Jul 13 2018, 2:06 AM

Jul 12 2018

arichardson added a comment to D47541: Allow creating llvm::Function in non-zero address spaces.

This seems to work quite nice for me now! Nice!

I added a few inline nit comments. Apart from that it looks good to me.
Although, I don't know much about the bitcode reader/writer. Anyone else who can help out reviewing that part?

PS. I still have a few worries about how well this plays together with instrprof, we have had some hacks in that area earlier. Methods like appendToGlobalArray (in ModuleUtils.cpp) still uses PointerType::getUnqual(FnTy) to strip off the addrspace from function pointers (and afaik the size of a pointer can be different in different adress spaces).
Although, I don't think we can solve all problems right away. This patch at least gives us the power to set addrspace on functions. And that should help out if I for example want to reproduce instrprof problems on trunk (using an "experimental target" with non-zero program addrspace). So I think we can leave that for later.

Jul 12 2018, 2:38 PM
arichardson added a comment to D49084: FileCheck: Add support for variable expressions.

I case people are interested I have also implemented something similar in our fork of llvm. I chose [[@EXPR var + 0x42]] as the syntax. I also added some functions such as [[@EXPR hex(VAR)]].
Variables can either by referenced by name or if they conflict with one of the builtin functions using ${x} syntax: [[@EXPR toupper(hex(${hex}))]]
Some examples of what it can do can be seen here: https://github.com/CTSRD-CHERI/llvm/blob/master/test/FileCheck/expressions.txt

Jul 12 2018, 2:33 PM

Jul 9 2018

arichardson updated the diff for D47541: Allow creating llvm::Function in non-zero address spaces.

Remove unrelated change and don't use llvm.phx.aload()

Jul 9 2018, 11:15 AM
arichardson created D49091: Warn about usage of __has_include/__has_include_next in macro expansions.
Jul 9 2018, 10:53 AM
arichardson updated the diff for D47541: Allow creating llvm::Function in non-zero address spaces.

Chose option 3) to handle forward-declarations.
I tried updating the address space later but that caused lots of problems.
Instead the call/invoke instruction can now accept an optional addrspace(N)
qualifier to indicate the address space of the called function.

Jul 9 2018, 8:36 AM
arichardson created D49067: Stop wrapping __has_include in another macro.
Jul 9 2018, 4:10 AM
arichardson added a comment to D45383: Limit types of builtins that can be redeclared..

This broke the build of FreeBSD for me due to the declaration of __builtin_return_address(unsigned int) in https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Base.h#L1281:

Jul 9 2018, 1:53 AM · Restricted Project

Jul 8 2018

arichardson added inline comments to D47541: Allow creating llvm::Function in non-zero address spaces.
Jul 8 2018, 1:17 PM

Jul 4 2018

arichardson abandoned D41821: [MIPS] Disassemble the 0xefefefef trap padding used by LLD.
Jul 4 2018, 11:01 AM

Jul 2 2018

arichardson added a comment to D48803: Place the BlockAddress type in the program address space.

I don't know much about the BlockAddress concept. The LangRef says things like "always has an i8* type" and "this may be passed around as an opaque pointer sized value". But I guess it would be weird if the size doesn't match the size of pointers in the program address space, so the patch makes sense to me.

I assume that this can't be reproduced for any in-tree target?
If you can't find an in-tree reproducer, then maybe you can describe the problem a little bit more instead. Such as which assert you hit, and maybe a small stack trace. That might help when trying to motivate this patch in the future.

Yes I'm not sure I can make a test for this with any of the existing targets. I'll see if I can get something with AVR since that sets program address space to 1.

Jul 2 2018, 9:46 AM
arichardson added a comment to D47541: Allow creating llvm::Function in non-zero address spaces.

Ping? I've applied this to our fork and it has improved inlining and helps us avoid bugs due to using the wrong address space. I would very much like to have this upstream.

Jul 2 2018, 9:44 AM

Jun 30 2018

arichardson created D48803: Place the BlockAddress type in the program address space.
Jun 30 2018, 6:26 AM

Jun 25 2018

arichardson committed rL335495: Use Triple::isMIPS() instead of enumerating all Triples. NFC.
Use Triple::isMIPS() instead of enumerating all Triples. NFC
Jun 25 2018, 9:55 AM
arichardson committed rC335495: Use Triple::isMIPS() instead of enumerating all Triples. NFC.
Use Triple::isMIPS() instead of enumerating all Triples. NFC
Jun 25 2018, 9:54 AM
arichardson closed D48549: Use Triple::isMIPS() instead of enumerating all Triples. NFC.
Jun 25 2018, 9:54 AM
arichardson committed rL335493: Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC.
Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC
Jun 25 2018, 9:54 AM
arichardson closed D48548: Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC.
Jun 25 2018, 9:54 AM
arichardson updated the diff for D48549: Use Triple::isMIPS() instead of enumerating all Triples. NFC.

Ran clang-format on the diff

Jun 25 2018, 8:11 AM
arichardson added inline comments to D48548: Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC.
Jun 25 2018, 8:11 AM
arichardson updated the diff for D48548: Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC.

Remove old comment

Jun 25 2018, 8:11 AM
arichardson added a child revision for D48548: Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC: D48549: Use Triple::isMIPS() instead of enumerating all Triples. NFC.
Jun 25 2018, 7:15 AM
arichardson added a parent revision for D48549: Use Triple::isMIPS() instead of enumerating all Triples. NFC: D48548: Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC.
Jun 25 2018, 7:15 AM
arichardson created D48549: Use Triple::isMIPS() instead of enumerating all Triples. NFC.
Jun 25 2018, 7:10 AM
arichardson created D48548: Add Triple::isMIPS()/isMIPS32()/isMIPS64(). NFC.
Jun 25 2018, 7:09 AM

Jun 21 2018

arichardson added a reviewer for D47541: Allow creating llvm::Function in non-zero address spaces: bjope.

It would be really good if something like this could land soon as I'm currently in the process of doing a merge with upstream LLVM in our fork.
I just ran into the problem that always_inline wasn't working at -O0 for us due to functions being in AS0 but the calls in AS200 and the alwaysInliner only handles direct calls without an addressspacecast. This would be fixed if we could have functions in AS200.

Jun 21 2018, 2:20 AM
arichardson added a comment to D37054: Require address space to be specified when creating functions (2/3).

I've extracted a patch without the Module* -> Module& changes in D47541. It would be really good if something like this could land soon since I just ran into the problem that always_inline wasn't working at -O0 for us due to functions being in AS0 but the calls in AS200 and the alwaysInliner only handles direct calls without an addressspacecast.

Jun 21 2018, 2:18 AM

Jun 12 2018

arichardson committed rLLD334483: [ELF][MIPS] Fix TLS GOT entries for local symbols in shared libraries.
[ELF][MIPS] Fix TLS GOT entries for local symbols in shared libraries
Jun 12 2018, 1:05 AM
arichardson committed rL334483: [ELF][MIPS] Fix TLS GOT entries for local symbols in shared libraries.
[ELF][MIPS] Fix TLS GOT entries for local symbols in shared libraries
Jun 12 2018, 1:05 AM
arichardson closed D48002: [ELF][MIPS] Fix TLS GOT entries for local symbols in shared libraries.
Jun 12 2018, 1:05 AM

Jun 11 2018

arichardson created D48002: [ELF][MIPS] Fix TLS GOT entries for local symbols in shared libraries.
Jun 11 2018, 1:48 AM
arichardson added a comment to D47627: [ASTContext] Make getAddrSpaceQualType replace address spaces..

Well, the documentation mismatch is worth fixing even if the code isn't. But I think at best your use-case calls for weakening the assertion to be that any existing address space isn't *different*, yeah.

Separately, I'm not sure that's really the right representation for a Harvard architecture (which is what I assume you're trying to extend Clang to support); I think you should probably just teach the compiler that function pointers are different.

Jun 11 2018, 1:08 AM

Jun 10 2018

arichardson accepted D31528: [ELF][MIPS] Multi-GOT implementation.
Jun 10 2018, 11:15 AM · lld

May 30 2018

arichardson created D47541: Allow creating llvm::Function in non-zero address spaces.
May 30 2018, 9:35 AM

May 23 2018

arichardson added a comment to D31287: [mips] Fix atomic operations at O0, v3.

LGTM. Also looks like it should be easy to adjust our CHERI ll/sc instructions to use the same mechanism when I do the next upstream merge.

May 23 2018, 1:28 AM
arichardson accepted D31528: [ELF][MIPS] Multi-GOT implementation.

As I have mentioned before, having this patch is the difference between LLD being completely unusable for our purposes (linking FreeBSD MIPS n64) and being able to replace BFD.

May 23 2018, 1:22 AM · lld

May 16 2018

arichardson committed rL332440: Escape ]]> in xunit xml output.
Escape ]]> in xunit xml output
May 16 2018, 2:04 AM
arichardson closed D46886: Escape ]]> in xunit xml output.
May 16 2018, 2:04 AM
arichardson committed rL332439: Emit a left-shift instead of a power-of-two multiply for jump-tables.
Emit a left-shift instead of a power-of-two multiply for jump-tables
May 16 2018, 2:02 AM
arichardson closed D45760: Emit a left-shift instead of a power-of-two multiply for jump-tables.
May 16 2018, 2:02 AM

May 15 2018

arichardson added a comment to D46886: Escape ]]> in xunit xml output.

Is this an issue you are hitting? I noticed we spend a lot of time in string replace while writing the xunit files when the output is large (for instance if every test in the test suite fails). I was hoping the number of replaces we do could go down.

Besides that LGTM.

May 15 2018, 10:04 AM
arichardson created D46886: Escape ]]> in xunit xml output.
May 15 2018, 9:02 AM
arichardson updated the diff for D45760: Emit a left-shift instead of a power-of-two multiply for jump-tables.

Address review comment

May 15 2018, 8:26 AM

May 7 2018

arichardson accepted D24867: Request init/fini array on FreeBSD 12 and later.

I recently made this change for the CHERI clang fork and as far as I can tell everything works fine.

May 7 2018, 2:57 PM

Apr 20 2018

arichardson updated the diff for D45760: Emit a left-shift instead of a power-of-two multiply for jump-tables.

Perform the optimization in LegalizeDAG instead. Turns out this also fixes MSP430

Apr 20 2018, 3:12 AM
arichardson abandoned D43475: [llvm-objcopy] Implement --only-keep-debug.

This approach is wrong and I currently don't have time to work on a correct solution.

Apr 20 2018, 1:05 AM