Page MenuHomePhabricator

ruiu (Rui Ueyama)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 18 2013, 12:16 AM (305 w, 2 d)

Recent Activity

Yesterday

ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I asked about the rationale behind the 2-PLT scheme to H. J. Lu who proposed the design. I was not convinced one of the reasons he presented, which is that the 2-PLT scheme provides compatibility with old systems, because clearly the 2-PLT scheme breaks compatibility by changing the semantics of the existing .plt section and adding the second PLT. The other reason was more convincing, which is, it was designed with performance in mind, and it was an effort to reduce the size of the hot code path. With the 2-PLT scheme, the second PLT is in theory hot and the first PLT is relatively cold, as the first PLT is used only when a symbol is resolved lazily. That being said, there was not evidence or benchmark results supporting the claim, so I cannot really buy that argument.

Fri, Feb 22, 5:04 PM · Restricted Project, lld
ruiu committed rG5945ad5c430d: Split a long line to avoid annoying horizontal scrolling on a browser. (authored by ruiu).
Split a long line to avoid annoying horizontal scrolling on a browser.
Fri, Feb 22, 4:27 PM
ruiu committed rLLD354707: Split a long line to avoid annoying horizontal scrolling on a browser..
Split a long line to avoid annoying horizontal scrolling on a browser.
Fri, Feb 22, 4:27 PM
ruiu committed rL354707: Split a long line to avoid annoying horizontal scrolling on a browser..
Split a long line to avoid annoying horizontal scrolling on a browser.
Fri, Feb 22, 4:26 PM
ruiu committed rG81c0880c99a4: s/method/function/g since function is the correct name in C++. (authored by ruiu).
s/method/function/g since function is the correct name in C++.
Fri, Feb 22, 4:03 PM
ruiu committed rGc94dad9d974b: Remove a function from header and move the implementation to a .cpp file. NFC. (authored by ruiu).
Remove a function from header and move the implementation to a .cpp file. NFC.
Fri, Feb 22, 4:03 PM
ruiu committed rLLD354704: s/method/function/g since function is the correct name in C++..
s/method/function/g since function is the correct name in C++.
Fri, Feb 22, 4:03 PM
ruiu committed rLLD354703: Remove a function from header and move the implementation to a .cpp file. NFC..
Remove a function from header and move the implementation to a .cpp file. NFC.
Fri, Feb 22, 4:03 PM
ruiu committed rL354704: s/method/function/g since function is the correct name in C++..
s/method/function/g since function is the correct name in C++.
Fri, Feb 22, 4:03 PM
ruiu committed rL354703: Remove a function from header and move the implementation to a .cpp file. NFC..
Remove a function from header and move the implementation to a .cpp file. NFC.
Fri, Feb 22, 4:02 PM
ruiu accepted D58540: [LLD][ELF][ARM] Accept and ignore -p and -no-pipeline-knowledge.

Perhaps we should remove the usage of these flags from the Linux kernel even after this patch has landed because they are just dead flags, but this patch per se looks good to me.

Fri, Feb 22, 2:31 PM
ruiu added a comment to D58484: [DO NOT SUBMIT] Add -vs-diagnostics..

For me, the code should be, above all, reliable. You can trust it only in that case; otherwise, you never know, when and how it is going to be broken and produce an unexpected result.

I would suggest a simple test for both approaches. What if a developer adds a new diagnostic message?

With D58300 it will probably just work as expected in both modes out-of-the-box. You even don't need to check that it is properly formatted, because most of the formatting is performed in one place, uniformly.

With this patch (D58484) the developer might not be aware of the existence of the --vs-diagnostics switch. Thus, recognizing patterns will not probably be updated and the message generation will be broken for an unpredictable time until someone comes across that new message in Visual Studio.

Fri, Feb 22, 2:28 PM · Restricted Project
ruiu accepted D49366: [LLD][COFF] Add support for /FUNCTIONPADMIN command-line option.

LGTM

Fri, Feb 22, 12:11 PM · Restricted Project, lld
ruiu added inline comments to D49366: [LLD][COFF] Add support for /FUNCTIONPADMIN command-line option.
Fri, Feb 22, 11:13 AM · Restricted Project, lld

Thu, Feb 21

ruiu committed rG04661e1084cc: Update `ld.lld --version` string for monorepo. (authored by ruiu).
Update `ld.lld --version` string for monorepo.
Thu, Feb 21, 10:37 AM
ruiu committed rL354605: Update `ld.lld --version` string for monorepo..
Update `ld.lld --version` string for monorepo.
Thu, Feb 21, 10:37 AM
ruiu committed rLLD354605: Update `ld.lld --version` string for monorepo..
Update `ld.lld --version` string for monorepo.
Thu, Feb 21, 10:37 AM
ruiu closed D58411: Update `ld.lld --version` string for monorepo..
Thu, Feb 21, 10:36 AM · Restricted Project
ruiu accepted D58510: [yaml2obj]Allow explicit symbol indexes in relocations and emit error for bad names.

LGTM

Thu, Feb 21, 10:33 AM · Restricted Project
ruiu accepted D58508: [ELF][test]Remove unnecessary empty symbol references in yaml/add missing symbols for relocs.

LGTM

Thu, Feb 21, 10:29 AM · Restricted Project

Wed, Feb 20

ruiu accepted D58489: ELF: Remove dead code. NFCI..

LGTM

Wed, Feb 20, 9:24 PM · Restricted Project
ruiu updated the summary of D58484: [DO NOT SUBMIT] Add -vs-diagnostics..
Wed, Feb 20, 5:46 PM · Restricted Project
ruiu created D58484: [DO NOT SUBMIT] Add -vs-diagnostics..
Wed, Feb 20, 5:42 PM · Restricted Project
ruiu accepted D58482: [WebAssembly] Remove redundant code added in rL354538. NFC..

LGTM

Wed, Feb 20, 5:26 PM · Restricted Project
ruiu accepted D57909: [WebAssembly] Don't generate invalid modules when function signatures mismatch.

LGTM

Wed, Feb 20, 2:38 PM · Restricted Project
ruiu added inline comments to D49366: [LLD][COFF] Add support for /FUNCTIONPADMIN command-line option.
Wed, Feb 20, 2:32 PM · Restricted Project, lld
ruiu added inline comments to D58411: Update `ld.lld --version` string for monorepo..
Wed, Feb 20, 10:47 AM · Restricted Project
ruiu updated the diff for D58411: Update `ld.lld --version` string for monorepo..
  • address review comments
Wed, Feb 20, 10:47 AM · Restricted Project
ruiu added inline comments to D49366: [LLD][COFF] Add support for /FUNCTIONPADMIN command-line option.
Wed, Feb 20, 10:01 AM · Restricted Project, lld

Tue, Feb 19

ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I think SPLT has better compatibility compared to change the original PLT entry. Given the platform is old, and use the old dynamic linker, the new library that build with SPLT can still be linked by dynamic linker. I am not sure if it still works if we change the struct of PLT entry.

Tue, Feb 19, 6:31 PM · Restricted Project, lld
ruiu accepted D58422: ELF: Remove field for .interp in InStruct. NFC..

LGTM

Tue, Feb 19, 6:23 PM · Restricted Project
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Hi ruiu, I asked its designer. This is really specified in x86-64 psABI SPEC, Please refer https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-draft.pdf 13.2 DynamicLinking

Tue, Feb 19, 4:05 PM · Restricted Project, lld
ruiu committed rG3ae672623401: Sort enum members so that arch-dependent members are at the right place. NFC. (authored by ruiu).
Sort enum members so that arch-dependent members are at the right place. NFC.
Tue, Feb 19, 4:03 PM
ruiu committed rL354405: Sort enum members so that arch-dependent members are at the right place. NFC..
Sort enum members so that arch-dependent members are at the right place. NFC.
Tue, Feb 19, 4:03 PM
ruiu committed rLLD354405: Sort enum members so that arch-dependent members are at the right place. NFC..
Sort enum members so that arch-dependent members are at the right place. NFC.
Tue, Feb 19, 4:03 PM
ruiu accepted D58255: [lld-link] preserve @llvm.used symbols in LTO.

LGTM

Tue, Feb 19, 3:35 PM · Restricted Project
ruiu updated the diff for D58411: Update `ld.lld --version` string for monorepo..
  • Simplify
Tue, Feb 19, 3:18 PM · Restricted Project
ruiu updated the diff for D58411: Update `ld.lld --version` string for monorepo..
  • fix typo
Tue, Feb 19, 3:12 PM · Restricted Project
ruiu created D58411: Update `ld.lld --version` string for monorepo..
Tue, Feb 19, 3:09 PM · Restricted Project
ruiu committed rG659f2752a01b: Move MinGW-specific code out of LinkerDriver::link. NFC. (authored by ruiu).
Move MinGW-specific code out of LinkerDriver::link. NFC.
Tue, Feb 19, 2:06 PM
ruiu committed rLLD354391: Move MinGW-specific code out of LinkerDriver::link. NFC..
Move MinGW-specific code out of LinkerDriver::link. NFC.
Tue, Feb 19, 2:06 PM
ruiu committed rL354391: Move MinGW-specific code out of LinkerDriver::link. NFC..
Move MinGW-specific code out of LinkerDriver::link. NFC.
Tue, Feb 19, 2:06 PM
ruiu closed D58395: Move MinGW-specific code out of LinkerDriver::link. NFC..
Tue, Feb 19, 2:06 PM · Restricted Project
ruiu accepted D58380: [LLD] [COFF] Add -exclude-all-symbols for MinGW.

LGTM

Tue, Feb 19, 12:51 PM · Restricted Project
ruiu added inline comments to D57909: [WebAssembly] Don't generate invalid modules when function signatures mismatch.
Tue, Feb 19, 11:31 AM · Restricted Project
ruiu added inline comments to D58380: [LLD] [COFF] Add -exclude-all-symbols for MinGW.
Tue, Feb 19, 11:21 AM · Restricted Project
ruiu created D58395: Move MinGW-specific code out of LinkerDriver::link. NFC..
Tue, Feb 19, 11:05 AM · Restricted Project
ruiu added inline comments to D58380: [LLD] [COFF] Add -exclude-all-symbols for MinGW.
Tue, Feb 19, 10:49 AM · Restricted Project

Fri, Feb 15

ruiu added inline comments to D57940: Refactor RelocVisitor and fix computation of SHT_RELA-typed relocation entries.
Fri, Feb 15, 4:22 PM · Restricted Project
ruiu added inline comments to D58271: [ELF] --gdb-index: split off GdbSymbol::CuVector and add a separate CuVectors.
Fri, Feb 15, 3:32 PM · Restricted Project
ruiu committed rG9efdd7ac5e91: [PPC64] Preserve LocalEntry when linking (authored by ruiu).
[PPC64] Preserve LocalEntry when linking
Fri, Feb 15, 3:11 PM
ruiu committed rLLD354184: [PPC64] Preserve LocalEntry when linking.
[PPC64] Preserve LocalEntry when linking
Fri, Feb 15, 3:11 PM
ruiu committed rL354184: [PPC64] Preserve LocalEntry when linking.
[PPC64] Preserve LocalEntry when linking
Fri, Feb 15, 3:10 PM
ruiu closed D56782: [PPC64] Preserve LocalEntry when linking.
Fri, Feb 15, 3:10 PM · Restricted Project
ruiu added a comment to D58300: [lld] Enable Visual Studio compatible output.

I think we need to be very careful how to implement the feature. Currently it is very easy to understand what lld prints out as an error message and how to construct an error message. Basically, what you see in the source code is what you get at runtime. This patch added an abstraction layer between the current code and stderr, and now it is not easy to figure out what is the actual output and how to construct a desired error message.

Fri, Feb 15, 3:00 PM · lld
ruiu accepted D58301: lld-link: Mention comdat selection work in the 9.0.0 wip release notes.

LGTM

Fri, Feb 15, 12:56 PM · Restricted Project
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I really don't think we should introduce a new PLT (.splt) if the reason of adding it is to keep the compatibility of the existing .plt format. Second PLT is too complicated and would become a technical debt.

Fri, Feb 15, 11:19 AM · Restricted Project, lld

Thu, Feb 14

ruiu added inline comments to D58255: [lld-link] preserve @llvm.used symbols in LTO.
Thu, Feb 14, 6:09 PM · Restricted Project
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

1 Yes, we will disable the feature if not all the input object files contain the .note.gnu.property section, because the CET feature will supervise all the indirect jumps in the program. If one input file do not contain the CET info (flags which is set in .note.gnu.property section's GNU_PROPERTY_X86_FEATURE_1_AND property ), it means it can‘t be check by the CET hardware, so we need to disable the CET feature. This is important to link the non-CET library.

Thu, Feb 14, 5:46 PM · Restricted Project, lld
ruiu accepted D58265: ELF: Fix typo in --build-id option description.

LGTM

Thu, Feb 14, 5:08 PM · Restricted Project
ruiu accepted D58246: [lld] Fix elf::unlinkAsync detached thread.

LGTM

Thu, Feb 14, 3:00 PM · Restricted Project
ruiu added inline comments to D58246: [lld] Fix elf::unlinkAsync detached thread.
Thu, Feb 14, 1:55 PM · Restricted Project
ruiu added inline comments to D58246: [lld] Fix elf::unlinkAsync detached thread.
Thu, Feb 14, 12:41 PM · Restricted Project
ruiu committed rG72c3b1ed1d40: Move a function from .h to .cpp and use a shorter name. NFC. (authored by ruiu).
Move a function from .h to .cpp and use a shorter name. NFC.
Thu, Feb 14, 11:34 AM
ruiu committed rLLD354054: Move a function from .h to .cpp and use a shorter name. NFC..
Move a function from .h to .cpp and use a shorter name. NFC.
Thu, Feb 14, 11:34 AM
ruiu committed rL354054: Move a function from .h to .cpp and use a shorter name. NFC..
Move a function from .h to .cpp and use a shorter name. NFC.
Thu, Feb 14, 11:34 AM
ruiu created D58245: Remove no-op code..
Thu, Feb 14, 11:28 AM · Restricted Project
ruiu committed rG980fb790c179: Remove a comparator from header and instead use lambdas for simplicity. NFC. (authored by ruiu).
Remove a comparator from header and instead use lambdas for simplicity. NFC.
Thu, Feb 14, 11:22 AM
ruiu committed rLLD354052: Remove a comparator from header and instead use lambdas for simplicity. NFC..
Remove a comparator from header and instead use lambdas for simplicity. NFC.
Thu, Feb 14, 11:21 AM
ruiu committed rL354052: Remove a comparator from header and instead use lambdas for simplicity. NFC..
Remove a comparator from header and instead use lambdas for simplicity. NFC.
Thu, Feb 14, 11:21 AM
ruiu committed rGf69bbbbdd2fc: Add a comment. NFC. (authored by ruiu).
Add a comment. NFC.
Thu, Feb 14, 10:52 AM
ruiu committed rL354049: Add a comment. NFC..
Add a comment. NFC.
Thu, Feb 14, 10:51 AM
ruiu committed rLLD354049: Add a comment. NFC..
Add a comment. NFC.
Thu, Feb 14, 10:51 AM
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Can I ask a few random questions to understand the design of the patch?

Thu, Feb 14, 10:29 AM · Restricted Project, lld
ruiu committed rGb8b81e9b43c8: Improve error message for unknown relocations. (authored by ruiu).
Improve error message for unknown relocations.
Thu, Feb 14, 10:03 AM
ruiu committed rLLD354040: Improve error message for unknown relocations..
Improve error message for unknown relocations.
Thu, Feb 14, 10:02 AM
ruiu committed rL354040: Improve error message for unknown relocations..
Improve error message for unknown relocations.
Thu, Feb 14, 10:02 AM
ruiu accepted D56782: [PPC64] Preserve LocalEntry when linking.

LGTM

Thu, Feb 14, 8:55 AM · Restricted Project

Wed, Feb 13

ruiu accepted D58180: lld/coff: Simplify error message for comdat selection mismatches.

LGTM

Wed, Feb 13, 4:19 PM · Restricted Project
ruiu added a comment to D58047: [LLD][ELF][ARM] Synthesise missing .ARM.exidx sections..

If we define a synthetic ARM.exidx section which consumes all .ARM.exidx sections and replace them with itself, we can perhaps remove SHF_ORDER and other features that we add for .ARM.exidx sections. If there's no other use of the features, I'd like to remove them.

Wed, Feb 13, 3:25 PM
ruiu added a comment to D58047: [LLD][ELF][ARM] Synthesise missing .ARM.exidx sections..

We used to handle .ARM.exidx sections as regular sections with a sentinel section that is synthesized and added to the end. We later added code to merge contiguous .ARM.exidx sections, and now this patch is adding sentinels at various points. At this point, maybe it is easier to handle .ARM.exidx as a single synthetic section just like MergeInputSection? That synthetic section removes all input section whose name is .ARM.exidx from the input section list and add itself to the input section. That design gives you more flexibility than the current design, as the intermediate data representation doesn't have to be an InputSection.

Wed, Feb 13, 3:13 PM
ruiu accepted D57810: MC/ELF: Allow targets to set ABI version.

LGTM

Wed, Feb 13, 2:27 PM · Restricted Project
ruiu accepted D57811: AMDGPU: Set ABI version to 1 for code object v3.

LGTM

Wed, Feb 13, 2:25 PM · Restricted Project
ruiu added inline comments to D58047: [LLD][ELF][ARM] Synthesise missing .ARM.exidx sections..
Wed, Feb 13, 1:48 PM
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

This is a fairly large patch that introduces a completely new feature to lld. Please allow me time to review this patch carefully at high level; I wouldn't like to review the code in detail until high level concerns or questions are addressed. Next time, please consider sending an RFC to express your intent to implement a feature before finishing up your patch. Completing a proof-of-concept is fine, but further development should be done incrementally in my opinion. It is not an uncommon situation in which we have to request a significant rework if a large feature is implemented all at once in a single patch. In addition to that, splitting a patch into a number of incremental patches makes code review much easier.

Wed, Feb 13, 1:43 PM · Restricted Project, lld
ruiu added a comment to D58205: [ELF] Support GNU compressed sections.

I personally think that they should update their toolchain so that they can handle the standardized compressed debug section instead of a GNU-extension, or they should disable tests that depends on the GNU extension by examining if -compressed-debug-section=gnu is supported before running the tests. I particularly dislike the fact that the GNU extension uses .z as a prefix for a compressed section, which honestly seems ugly.

Wed, Feb 13, 1:25 PM · Restricted Project
ruiu added inline comments to D58180: lld/coff: Simplify error message for comdat selection mismatches.
Wed, Feb 13, 1:08 PM · Restricted Project
ruiu added a comment to D58132: lld/elf: When demangling symbols, surround demangled symbol with quotes and include mangled symbol as well.

Boolean flags usually have the form /foo[:no] in MSVC linker, so I'd propose /demangle and /demangle:no for lld/coff.

Wed, Feb 13, 12:51 PM · Restricted Project
ruiu accepted D57371: ELF: Allow GOT relocs pointing to non-preemptable ifunc to resolve to an IRELATIVE where possible..

Even though I believe what this patch does is correct, it seems really complicated to me, but that's not a fault of this patch. This part of lld code is mind-boggling. At this point, it is perhaps easier to rewrite it entirely than incrementally improving the code quality of this file. I don't think I would do this soon, but that's probably what we should keep in mind.

Wed, Feb 13, 11:32 AM · Restricted Project
ruiu added a comment to D58005: Recover elf32-bigmips and elf32-powerpc support in LLD.

Submitted this patch and filed a merge request as https://bugs.llvm.org/show_bug.cgi?id=40721.

Wed, Feb 13, 10:54 AM · Restricted Project
ruiu committed rG4134143cf55d: Recover elf32-bigmips and elf32-powerpc support in LLD (authored by ruiu).
Recover elf32-bigmips and elf32-powerpc support in LLD
Wed, Feb 13, 10:51 AM
ruiu committed rLLD353968: Recover elf32-bigmips and elf32-powerpc support in LLD.
Recover elf32-bigmips and elf32-powerpc support in LLD
Wed, Feb 13, 10:51 AM
ruiu committed rL353968: Recover elf32-bigmips and elf32-powerpc support in LLD.
Recover elf32-bigmips and elf32-powerpc support in LLD
Wed, Feb 13, 10:50 AM
ruiu closed D58005: Recover elf32-bigmips and elf32-powerpc support in LLD.
Wed, Feb 13, 10:50 AM · Restricted Project
ruiu committed rG265e8e8252a6: Show "Unknown -z option" error message even if --version or --help are given. (authored by ruiu).
Show "Unknown -z option" error message even if --version or --help are given.
Wed, Feb 13, 10:49 AM
ruiu committed rLLD353967: Show "Unknown -z option" error message even if --version or --help are given..
Show "Unknown -z option" error message even if --version or --help are given.
Wed, Feb 13, 10:48 AM
ruiu committed rL353967: Show "Unknown -z option" error message even if --version or --help are given..
Show "Unknown -z option" error message even if --version or --help are given.
Wed, Feb 13, 10:48 AM
ruiu closed D55446: Show "Unknown -z option" error message early..
Wed, Feb 13, 10:48 AM · Restricted Project

Tue, Feb 12

ruiu added inline comments to D57909: [WebAssembly] Don't generate invalid modules when function signatures mismatch.
Tue, Feb 12, 7:49 PM · Restricted Project
ruiu added a comment to D57811: AMDGPU: Set ABI version to 1 for code object v3.

Just want to know a bit more about background of this change. Are you making an ABI-breaking change, so you are defining a new ABI version number for the AMDGPU target. Is this understanding correct? Is the AMDGPU target migrating to ABI version 2, or will version 1 and 2 co-exist in the future?

Tue, Feb 12, 4:51 PM · Restricted Project