Page MenuHomePhabricator

ruiu (Rui Ueyama)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 18 2013, 12:16 AM (339 w, 4 d)

Recent Activity

Yesterday

ruiu accepted D69198: [LLD] [COFF] Use the local dwarf code instead of Symbolizer for resolving code locations. NFC..

LGTM

Sun, Oct 20, 8:02 PM · Restricted Project
ruiu accepted D69197: [LLD] Move duplicated dwarf parsing code to the Common library. NFC..

LGTM

Sun, Oct 20, 7:53 PM · Restricted Project

Fri, Oct 18

ruiu added a comment to D68975: [LLD] [COFF] Try to report source locations for duplicate symbols.

Ah, that's true. How about something like "Used if DWARF debug info is used (which is not common)"?

Fri, Oct 18, 12:34 AM · Restricted Project

Thu, Oct 17

ruiu committed rG9a5ad9bd5ad1: Update release notes (authored by ruiu).
Update release notes
Thu, Oct 17, 11:12 PM
ruiu committed rL375206: Update release notes.
Update release notes
Thu, Oct 17, 11:12 PM
ruiu added inline comments to D69043: [RFC] Adding time-trace to LLD?.
Thu, Oct 17, 10:56 PM · Restricted Project, Restricted Project
ruiu added a comment to D69043: [RFC] Adding time-trace to LLD?.

I applied this patch and built clang with ThinLTO. Here is the generated file:

Thu, Oct 17, 9:58 PM · Restricted Project, Restricted Project
ruiu accepted D68975: [LLD] [COFF] Try to report source locations for duplicate symbols.

Code-wise, it's quite a bit of code for dwarf debug info which isn't really used all that much, but meh. I kind of feel like it might possibly make sense to move all the CoffMingw bits into a separate lld/ directory instead of having it in lld/COFF since it's lots of special purpose code. But that's not my call :)

I think some parts of the verbose dwarf code here could be refactored into lld/Common and share it with the ELF linker, but I'd prefer doing that as a separate step after landing this if that's tolerable (as some refactorings easily end up dragging on longer than initially expected). For other mingw related bits, we do have COFF/MinGW.cpp for logic that is easily decoupled from the rest of the COFF logic.

Thu, Oct 17, 7:16 PM · Restricted Project
ruiu added a comment to D69043: [RFC] Adding time-trace to LLD?.

This seems useful. Can I see an example output?

Thu, Oct 17, 3:33 AM · Restricted Project, Restricted Project
ruiu added inline comments to D69087: [llvm-ar] Implement the O modifier: display member offsets inside the archive.
Thu, Oct 17, 2:30 AM · Restricted Project
ruiu accepted D69087: [llvm-ar] Implement the O modifier: display member offsets inside the archive.

LGTM

Thu, Oct 17, 12:30 AM · Restricted Project

Wed, Oct 16

ruiu added a comment to D68975: [LLD] [COFF] Try to report source locations for duplicate symbols.

Nico, are you happy with this change?

Wed, Oct 16, 8:46 PM · Restricted Project
ruiu accepted D68959: [lld][WebAssebmly] Preserve custom import attributes with LTO.

LGTM

Wed, Oct 16, 8:36 PM · Restricted Project
ruiu accepted D68998: [docs][llvm-ar] Update llvm-ar command guide.

LGTM

Wed, Oct 16, 8:27 PM · Restricted Project
ruiu accepted D69074: [lld][test] Fix use of escape character in an lld test on Windows.

LGTM

Wed, Oct 16, 8:09 PM · Restricted Project
ruiu accepted D69073: [lld][WebAssembly] Fix for weak references to data symbols in archives.

LGTM

Wed, Oct 16, 8:09 PM · Restricted Project
ruiu added a comment to D68998: [docs][llvm-ar] Update llvm-ar command guide.

Thank you for doing this!

Wed, Oct 16, 2:09 AM · Restricted Project

Tue, Oct 15

ruiu accepted D68033: [llvm-ar] Make paths case insensitive when on windows.

LGTM

Tue, Oct 15, 9:50 PM · Restricted Project
ruiu added inline comments to D68959: [lld][WebAssebmly] Preserve custom import attributes with LTO.
Tue, Oct 15, 8:41 PM · Restricted Project
ruiu added a reviewer for D68975: [LLD] [COFF] Try to report source locations for duplicate symbols: thakis.

This patch looks good to me, but I'd like to get a second opinion from other devs because this could be a big change in UX. Nico, you recently worked on error messages. What do you think of this change?

Tue, Oct 15, 3:59 AM · Restricted Project
ruiu added inline comments to D68033: [llvm-ar] Make paths case insensitive when on windows.
Tue, Oct 15, 3:46 AM · Restricted Project
ruiu accepted D68875: [lld] Check for branch range overflows..

LGTM

Tue, Oct 15, 3:35 AM · Restricted Project
ruiu added a comment to D68751: [lld][WebAssembly] Where possible handle signature mismatches via an adaptor function.

Do you want to land this or abandon? I think I'm fine either relying on a compiler magic or landing this patch (and remove the compiler hack), but I'm not very happy about landing this and leaving the compiler hack, so I guess you need to make your mind.

Tue, Oct 15, 3:35 AM · Restricted Project
ruiu added inline comments to D68975: [LLD] [COFF] Try to report source locations for duplicate symbols.
Tue, Oct 15, 3:23 AM · Restricted Project
ruiu added a comment to D68959: [lld][WebAssebmly] Preserve custom import attributes with LTO.

It seems that the patch description is too terse. Please explain the bug that this patch is fixing in the commit message.

Tue, Oct 15, 3:11 AM · Restricted Project
ruiu added inline comments to D68975: [LLD] [COFF] Try to report source locations for duplicate symbols.
Tue, Oct 15, 3:00 AM · Restricted Project
ruiu added inline comments to D68965: [WebAssembly] Elide data segments for .bss sections.
Tue, Oct 15, 2:37 AM · Restricted Project
ruiu accepted D68935: [LLD] [COFF] Wrap file location pair<StringRef,int> in Optional<>. NFC..

LGTM

Tue, Oct 15, 2:10 AM · Restricted Project

Thu, Oct 10

ruiu committed rG9adea6e4fae3: Make nullptr check more robust (authored by ruiu).
Make nullptr check more robust
Thu, Oct 10, 5:41 AM
ruiu committed rL374332: Make nullptr check more robust.
Make nullptr check more robust
Thu, Oct 10, 5:41 AM
ruiu added a comment to D68033: [llvm-ar] Make paths case insensitive when on windows.

As jhenderson suggested, could you add a description about this behavior? Looks like llvm-ar doesn't have a manual page, so the second best thing to add a description is probably the help message for --help.

Thu, Oct 10, 5:22 AM · Restricted Project
ruiu accepted D68772: [COFF] Wrap things in namespace lld { namespace coff {.

LGTM

Thu, Oct 10, 3:46 AM · Restricted Project
ruiu added inline comments to D68758: Improve error message for bad SHF_MERGE sections.
Thu, Oct 10, 3:19 AM · Restricted Project
ruiu added inline comments to D68758: Improve error message for bad SHF_MERGE sections.
Thu, Oct 10, 3:01 AM · Restricted Project
ruiu committed rG37bf9bb405ff: Use error instead of fatal to report usage errors (authored by ruiu).
Use error instead of fatal to report usage errors
Thu, Oct 10, 2:51 AM
ruiu closed D68768: Use error instead of fatal to report usage errors.
Thu, Oct 10, 2:51 AM · Restricted Project
ruiu committed rL374297: Use error instead of fatal to report usage errors.
Use error instead of fatal to report usage errors
Thu, Oct 10, 2:50 AM
ruiu added inline comments to D68768: Use error instead of fatal to report usage errors.
Thu, Oct 10, 2:49 AM · Restricted Project
ruiu created D68768: Use error instead of fatal to report usage errors.
Thu, Oct 10, 2:22 AM · Restricted Project
ruiu committed rGd7ead5b58daf: Improve error message for bad SHF_MERGE sections (authored by ruiu).
Improve error message for bad SHF_MERGE sections
Thu, Oct 10, 1:35 AM
ruiu committed rL374290: Improve error message for bad SHF_MERGE sections.
Improve error message for bad SHF_MERGE sections
Thu, Oct 10, 1:34 AM
ruiu closed D68758: Improve error message for bad SHF_MERGE sections.
Thu, Oct 10, 1:34 AM · Restricted Project
ruiu added inline comments to D68758: Improve error message for bad SHF_MERGE sections.
Thu, Oct 10, 12:48 AM · Restricted Project
ruiu updated the diff for D68758: Improve error message for bad SHF_MERGE sections.
  • address review comments
  • merged with a existing test file
Thu, Oct 10, 12:20 AM · Restricted Project
ruiu accepted D68689: [LLD] [MinGW] Look for other library patterns with -l.

Using error instead of fatal is not a high priority matter for the MinGW driver because I believe users are using this driver as a command. But still it is better to use error because it is more like a contract as a library. So, can you make a follow-up change?

Thu, Oct 10, 12:11 AM · Restricted Project

Wed, Oct 9

ruiu accepted D68688: [LLD] [MinGW] Add a testcase for -l:name style library options. NFC..

LGTM

Wed, Oct 9, 11:06 PM · Restricted Project
ruiu accepted D68759: [WebAssembly] Wrap definitions in namespace lld { namespace wasm {. NFC.

LGTM

Wed, Oct 9, 9:44 PM · Restricted Project
ruiu added inline comments to D68751: [lld][WebAssembly] Where possible handle signature mismatches via an adaptor function.
Wed, Oct 9, 9:34 PM · Restricted Project
ruiu created D68758: Improve error message for bad SHF_MERGE sections.
Wed, Oct 9, 9:25 PM · Restricted Project
ruiu added inline comments to D68751: [lld][WebAssembly] Where possible handle signature mismatches via an adaptor function.
Wed, Oct 9, 8:49 PM · Restricted Project
ruiu added a comment to D68751: [lld][WebAssembly] Where possible handle signature mismatches via an adaptor function.

I'm not sure if there's a standard for the wasm object files and how the linker works, but if there's any, could you add a pointer to the document to the comment?

Wed, Oct 9, 8:03 PM · Restricted Project
ruiu accepted D68689: [LLD] [MinGW] Look for other library patterns with -l.

LGTM with this change.

Wed, Oct 9, 7:45 PM · Restricted Project
ruiu accepted D68749: [lld][WebAssembly] Refactor markLive.cpp. NFC.

LGTM

Wed, Oct 9, 7:45 PM · Restricted Project
ruiu committed rG07775b207a95: Use lld-link instead of llvm-dlltool to create an implib (authored by ruiu).
Use lld-link instead of llvm-dlltool to create an implib
Wed, Oct 9, 12:11 AM
ruiu committed rL374142: Use lld-link instead of llvm-dlltool to create an implib.
Use lld-link instead of llvm-dlltool to create an implib
Wed, Oct 9, 12:02 AM
ruiu added inline comments to D68352: [lld] Don't create hints-section if Hint/Name Table is empty.
Wed, Oct 9, 12:02 AM · Restricted Project

Tue, Oct 8

ruiu committed rGc3c5e0fbbf76: [lld] Don't create hints-section if Hint/Name Table is empty (authored by ruiu).
[lld] Don't create hints-section if Hint/Name Table is empty
Tue, Oct 8, 11:53 PM
ruiu committed rL374140: [lld] Don't create hints-section if Hint/Name Table is empty.
[lld] Don't create hints-section if Hint/Name Table is empty
Tue, Oct 8, 11:52 PM
ruiu closed D68352: [lld] Don't create hints-section if Hint/Name Table is empty.
Tue, Oct 8, 11:52 PM · Restricted Project
ruiu added a comment to D68352: [lld] Don't create hints-section if Hint/Name Table is empty.

OK, I'll just apply your change and submit. Thanks!

Tue, Oct 8, 11:52 PM · Restricted Project
ruiu added a comment to D68352: [lld] Don't create hints-section if Hint/Name Table is empty.

Is it OK to drop -debug option from the lld-link command line? I think we don't need that option there.

Tue, Oct 8, 11:34 PM · Restricted Project
ruiu added a comment to D68352: [lld] Don't create hints-section if Hint/Name Table is empty.

Build reproducibility (a property that if you feed the exact same inputs to compilers or linkers you can get the exact same output) is important. Unfrotunately, by default, COFF has a timestamp field that changes virtually every time, but it looks like the difference between you and me is a difference how things are laid out in the file. That is not supposed to vary.

Tue, Oct 8, 10:44 PM · Restricted Project
ruiu added a comment to D68352: [lld] Don't create hints-section if Hint/Name Table is empty.

It looks like the same test fails on Windows with the same output, so I'll fix it to match the actual output and submit. Does this sound OK?

Tue, Oct 8, 10:30 PM · Restricted Project
ruiu added a comment to D68352: [lld] Don't create hints-section if Hint/Name Table is empty.

The test failed on my Linux box with the following message. Is that expected?

Tue, Oct 8, 10:21 PM · Restricted Project
ruiu accepted D68352: [lld] Don't create hints-section if Hint/Name Table is empty.

LGTM

Tue, Oct 8, 7:30 PM · Restricted Project
ruiu committed rGd2e9dd3877e9: Use /dev/null for tests that we do not need outputs (authored by ruiu).
Use /dev/null for tests that we do not need outputs
Tue, Oct 8, 1:06 AM
ruiu committed rG54933667296d: Report error if -export-dynamic is used with -r (authored by ruiu).
Report error if -export-dynamic is used with -r
Tue, Oct 8, 1:06 AM
ruiu committed rL374022: Report error if -export-dynamic is used with -r.
Report error if -export-dynamic is used with -r
Tue, Oct 8, 1:06 AM
ruiu committed rL374023: Use /dev/null for tests that we do not need outputs.
Use /dev/null for tests that we do not need outputs
Tue, Oct 8, 1:05 AM
ruiu closed D68441: Ignore --export-dynamic if --relocatable is given.
Tue, Oct 8, 1:05 AM · Restricted Project
ruiu added inline comments to D68441: Ignore --export-dynamic if --relocatable is given.
Tue, Oct 8, 1:05 AM · Restricted Project
ruiu updated the diff for D68441: Ignore --export-dynamic if --relocatable is given.
  • report error instead of ignoring -export-dynamic
Tue, Oct 8, 12:29 AM · Restricted Project

Mon, Oct 7

ruiu added inline comments to D68062: Propeller lld framework for basicblock sections.
Mon, Oct 7, 11:34 PM · Restricted Project
ruiu added inline comments to D68570: Unify the two CRC implementations.
Mon, Oct 7, 11:02 PM · Restricted Project, Restricted Project, Restricted Project
ruiu added inline comments to D10548: Teach LTOModule to emit linker flags for dllexported symbols, plus interface cleanup..
Mon, Oct 7, 10:09 PM · Restricted Project
ruiu added a comment to D68352: [lld] Don't create hints-section if Hint/Name Table is empty.

So, you are creating an executable that has an import table that has only ordinals. Is my understanding correct? Indeed, that condition is rare and I've thought of that case before when I implemented this part.

Mon, Oct 7, 9:08 PM · Restricted Project
ruiu accepted D68323: [ELF] Wrap things in `namespace lld { namespace elf {`, NFC.

LGTM

Mon, Oct 7, 12:52 AM · Restricted Project
ruiu accepted D68561: [ELF][MIPS] Use lld::elf::{read,write}* instead of llvm::support::endian::{read,write}*.

I think you can de-template some functions now, but that can be done in a follow-up patch.

Mon, Oct 7, 12:52 AM · Restricted Project

Fri, Oct 4

ruiu added a comment to D68033: [llvm-ar] Make paths case insensitive when on windows.

Generally looking good.

Fri, Oct 4, 2:06 AM · Restricted Project
ruiu added a comment to D68017: [LLD] [COFF] Always demangle the __imp_ prefix to __declspec(dllimport).

LGTM but I'd like to know what rnk thinks about this change.

Fri, Oct 4, 1:45 AM · Restricted Project
ruiu added a comment to D68017: [LLD] [COFF] Always demangle the __imp_ prefix to __declspec(dllimport).

I think I prefer not to remove the leading underscore for i386 because as you wrote it makes the output ambiguous.

Fri, Oct 4, 12:56 AM · Restricted Project
ruiu committed rGe4758a5c279a: [MinGW] Add --reproduce option (authored by ruiu).
[MinGW] Add --reproduce option
Fri, Oct 4, 12:29 AM
ruiu committed rG0d53ac809643: Add /reproduce option to lld/COFF (authored by ruiu).
Add /reproduce option to lld/COFF
Fri, Oct 4, 12:29 AM
ruiu committed rG67858244314c: Revert r371729: lld-link: Make /linkrepro: take a filename, not a directory. (authored by ruiu).
Revert r371729: lld-link: Make /linkrepro: take a filename, not a directory.
Fri, Oct 4, 12:29 AM
ruiu committed rL373705: [MinGW] Add --reproduce option.
[MinGW] Add --reproduce option
Fri, Oct 4, 12:29 AM
ruiu closed D68382: [MinGW] Add --reproduce option.
Fri, Oct 4, 12:28 AM · Restricted Project
ruiu committed rL373704: Add /reproduce option to lld/COFF.
Add /reproduce option to lld/COFF
Fri, Oct 4, 12:28 AM
ruiu closed D68381: Add /reproduce option to lld/COFF.
Fri, Oct 4, 12:28 AM · Restricted Project
ruiu committed rL373703: Revert r371729: lld-link: Make /linkrepro: take a filename, not a directory..
Revert r371729: lld-link: Make /linkrepro: take a filename, not a directory.
Fri, Oct 4, 12:28 AM
ruiu closed D68378: Revert r371729: lld-link: Make /linkrepro: take a filename, not a directory..
Fri, Oct 4, 12:28 AM · Restricted Project
ruiu added a comment to D68382: [MinGW] Add --reproduce option.

If you are OK with the other patches, please stamp.

Fri, Oct 4, 12:09 AM · Restricted Project

Thu, Oct 3

ruiu added a comment to D68382: [MinGW] Add --reproduce option.

Yes, please take a look at https://reviews.llvm.org/D68381 and https://reviews.llvm.org/D68378.

Thu, Oct 3, 11:58 PM · Restricted Project
ruiu added a comment to D68323: [ELF] Wrap things in `namespace lld { namespace elf {`, NFC.

How about always using elf::write32 instead of endian::write32<E>?

lld::elf::* are more commonly used in lld. This will end up with a lot of qualified write32 read16 calls. I think we should reduce the number of using namespace llvm::support::endian;.

I mean we can replace all occurrences of endian::write32<E>() with write32(). It looks like lld still compiles after I run the following command to replace them.

sed -i 's/write32<.>/write32/g' lld/ELF/Arch/Mips.cpp

This will make the generated code slightly worse because the implementation tests config->endianness. llvm::support::endian::write16(p, v, config->endianness);

Personally I prefer keeping the templated version, though I don't mind changing it if you are strong about this or @atanasyan is fine with lld::elf::* functions.

Thu, Oct 3, 11:10 PM · Restricted Project
ruiu added a comment to D68323: [ELF] Wrap things in `namespace lld { namespace elf {`, NFC.

How about always using elf::write32 instead of endian::write32<E>?

lld::elf::* are more commonly used in lld. This will end up with a lot of qualified write32 read16 calls. I think we should reduce the number of using namespace llvm::support::endian;.

Thu, Oct 3, 10:48 PM · Restricted Project
ruiu added inline comments to D68062: Propeller lld framework for basicblock sections.
Thu, Oct 3, 10:27 PM · Restricted Project
ruiu added inline comments to D68062: Propeller lld framework for basicblock sections.
Thu, Oct 3, 10:14 PM · Restricted Project
ruiu added a comment to D68323: [ELF] Wrap things in `namespace lld { namespace elf {`, NFC.

How about always using elf::write32 instead of endian::write32<E>?

Thu, Oct 3, 10:07 PM · Restricted Project
ruiu added a comment to D68323: [ELF] Wrap things in `namespace lld { namespace elf {`, NFC.

Yeah, I'm fine with quoting the entire file with the namespaces, but I'm not entirely happy about adding endian:: to read32. Looks like the problem is that we use the same name for two functions. Can you just rename one to avoid the name collision? Does that make the code look simpler?

Thu, Oct 3, 9:50 PM · Restricted Project
ruiu created D68441: Ignore --export-dynamic if --relocatable is given.
Thu, Oct 3, 8:58 PM · Restricted Project
ruiu created D68382: [MinGW] Add --reproduce option.
Thu, Oct 3, 2:32 AM · Restricted Project