ruiu (Rui Ueyama)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 18 2013, 12:16 AM (261 w, 3 d)

Recent Activity

Fri, Apr 20

ruiu accepted D45905: [ELF] Swap argument names: use Old to refer to original symbol and New for incoming one.

LGTM

Fri, Apr 20, 3:50 PM
ruiu added a comment to D45905: [ELF] Swap argument names: use Old to refer to original symbol and New for incoming one.

Don't you have to change the test?

Fri, Apr 20, 3:46 PM
ruiu accepted D45789: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.

LGTM

Fri, Apr 20, 3:08 PM
ruiu committed rLLD330482: Add -z {combreloc,copyreloc,noexecstack,lazy,relro,text}..
Add -z {combreloc,copyreloc,noexecstack,lazy,relro,text}.
Fri, Apr 20, 2:27 PM
ruiu committed rL330482: Add -z {combreloc,copyreloc,noexecstack,lazy,relro,text}..
Add -z {combreloc,copyreloc,noexecstack,lazy,relro,text}.
Fri, Apr 20, 2:27 PM
ruiu closed D45902: Add "-z lazy" and "-z relro"..
Fri, Apr 20, 2:27 PM
ruiu updated the diff for D45902: Add "-z lazy" and "-z relro"..
  • add more flags
  • add tests
Fri, Apr 20, 2:20 PM
ruiu accepted D45801: COFF: Use (name, output characteristics) as a key when grouping input sections into output sections..

No I don't. LGTM

Fri, Apr 20, 2:03 PM
ruiu added inline comments to D45902: Add "-z lazy" and "-z relro"..
Fri, Apr 20, 2:01 PM
ruiu created D45902: Add "-z lazy" and "-z relro"..
Fri, Apr 20, 1:51 PM
ruiu accepted D45892: [PPC64] Fix toc restore nops offset for V2 ABI.

LGTM with these fixes.

Fri, Apr 20, 11:26 AM

Thu, Apr 19

ruiu added inline comments to D45849: [ELF] --warn-backrefs: use the same GroupId for object files in the same --{start,end}-lib.
Thu, Apr 19, 5:53 PM
ruiu added a comment to D45849: [ELF] --warn-backrefs: use the same GroupId for object files in the same --{start,end}-lib.

If that's the case, --start-lib implies --start-group. To handle it, I'd set InputFile::IsInGroup to true for --start-lib just like we are doing for --start-group, so that --start-lib is handled as if --start-group everywhere.

Thu, Apr 19, 4:58 PM
ruiu added a comment to D45849: [ELF] --warn-backrefs: use the same GroupId for object files in the same --{start,end}-lib.

Does --start-lib/--end-lib really implies --start-group/--end-group?

Thu, Apr 19, 4:48 PM
ruiu added a comment to D45833: [PPC64] Add .toc input sections to end of .got output section.

I wonder if you really need to merge .toc into .got. If I understand your description correctly, it doesn't matter whether they are merged or not as long as they are close enough. So I'm wondering if you can just manipulate the section output order so that .toc and .got are output next to each other.

Thu, Apr 19, 4:30 PM
ruiu accepted D45846: [ELF] Increase NextGroupId with --end-group.

LGTM

Thu, Apr 19, 4:24 PM
ruiu added inline comments to D45788: Mitigate relocation overflow.
Thu, Apr 19, 4:13 PM
ruiu added a comment to D45846: [ELF] Increase NextGroupId with --end-group.

Please add a test for the linker script.

Thu, Apr 19, 4:09 PM
ruiu added a comment to D45846: [ELF] Increase NextGroupId with --end-group.

I think you need to fix readGroup in ScriptParser.cpp as well.

Thu, Apr 19, 4:02 PM
ruiu accepted D45830: Fix trap instruction on pp64.

LGTM

Thu, Apr 19, 4:00 PM
ruiu added a comment to D45841: Keep the output text sections with prefixes ".text.hot" , ".text.unlikely", ".text.startup", ".text.exit" separate.

What do you mean by "as if they were listed in --symbol-ordering-file"?

Thu, Apr 19, 3:49 PM
ruiu added a comment to D45841: Keep the output text sections with prefixes ".text.hot" , ".text.unlikely", ".text.startup", ".text.exit" separate.

And this should also slightly improve performance because of less number of page faults and improved locality for hot paths, I guess?

Thu, Apr 19, 3:40 PM
ruiu added a comment to D45789: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.

This is not actually that important, but I think I prefer my original suggestion. At least we shouldn't use assert() to report a broken file.

Thu, Apr 19, 2:55 PM
ruiu added inline comments to D45788: Mitigate relocation overflow.
Thu, Apr 19, 10:23 AM
ruiu added a comment to D45642: [PPC64] V2 abi: Add glink section for lazy symbol resolution..

Yeah, this seems like a good fit, as long as there is no objection to renaming GotPltSection and PltSection for EM_PPC64.

Thu, Apr 19, 10:10 AM
ruiu accepted D45800: COFF: Remove OutputSection::getPermissions() and getCharacteristics()..

LGTM

Thu, Apr 19, 10:06 AM
ruiu accepted D45799: COFF: Rename Chunk::getPermissions to getOutputCharacteristics..

LGTM

Thu, Apr 19, 10:06 AM
ruiu accepted D45802: COFF: Preserve section type when processing /section flag..

LGTM

Thu, Apr 19, 10:02 AM
ruiu accepted D45803: COFF: Merge .bss into .data by default..

LGTM

Thu, Apr 19, 9:58 AM
ruiu accepted D45804: COFF: Merge .xdata into .rdata by default..

LGTM

Thu, Apr 19, 9:57 AM
ruiu added inline comments to D45801: COFF: Use (name, output characteristics) as a key when grouping input sections into output sections..
Thu, Apr 19, 9:55 AM

Wed, Apr 18

ruiu added inline comments to D45789: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.
Wed, Apr 18, 4:57 PM
ruiu added inline comments to D45788: Mitigate relocation overflow.
Wed, Apr 18, 3:51 PM
ruiu accepted D45778: [COFF] Mark images with no exception handlers for /safeseh.

LGTM

Wed, Apr 18, 2:47 PM
ruiu added inline comments to D45778: [COFF] Mark images with no exception handlers for /safeseh.
Wed, Apr 18, 1:39 PM
ruiu added a comment to D45571: [ELF] - Speedup MergeInputSection::splitStrings.

I believe a fast strlen() can be implemented using SSE instructions. And if you are using SSE instructions, your data is loaded to XMM registers. I believe there exists a fast vectorized hash function that works on data on a XMM register. I wonder if we can combine the two to create a single fast function that returns the length of a string as well as its hash value.

Wed, Apr 18, 1:18 PM

Tue, Apr 17

ruiu accepted D45747: COFF: Implement /pdbaltpath flag..

LGTM

Tue, Apr 17, 4:16 PM
ruiu accepted D45467: COFF: Friendlier undefined symbol errors..

LGTM

Tue, Apr 17, 3:32 PM
ruiu added inline comments to D45729: [PPC64] Add offset to local entry point when calling functions without plt.
Tue, Apr 17, 3:29 PM
ruiu accepted D45737: COFF: Merge .idata, .didat and .edata into .rdata by default..

LGTM

Tue, Apr 17, 2:43 PM
ruiu committed rL330216: Rename sys::Process::GetArgumentVector -> sys::windows::GetCommandLineArguments.
Rename sys::Process::GetArgumentVector -> sys::windows::GetCommandLineArguments
Tue, Apr 17, 2:12 PM
ruiu closed D45641: Rename sys::Process::GetArgumentVector -> sys::windows::GetCommandLineArguments.
Tue, Apr 17, 2:12 PM
ruiu updated the diff for D45641: Rename sys::Process::GetArgumentVector -> sys::windows::GetCommandLineArguments.
  • address a review comment
Tue, Apr 17, 10:53 AM

Mon, Apr 16

ruiu added inline comments to D45467: COFF: Friendlier undefined symbol errors..
Mon, Apr 16, 9:21 PM
ruiu accepted D45714: COFF: Make SectionChunk::Relocs field an ArrayRef. NFCI..

LGTM

Mon, Apr 16, 6:53 PM
ruiu added inline comments to D45467: COFF: Friendlier undefined symbol errors..
Mon, Apr 16, 3:18 PM
ruiu added a comment to D45641: Rename sys::Process::GetArgumentVector -> sys::windows::GetCommandLineArguments.

My concern with this change is that once GetArgumentVector no longer exists, any developer who wants to retrieve the arguments as UTF8 on all platforms will need to be aware that there is a windows-specific function needed to do so. By having GetArgumentVector in sys::Process, we have a unified way of retrieving arguments on all platforms and if in the future it was necessary to specialize on say, Android, the interface wouldn't have to change. I dislike the "GetArgumentVector" name because it is rather unclear what it does (and likely the reason why it was not uniformly used), but I do think it should stay in sys::process.

Mon, Apr 16, 1:20 PM

Fri, Apr 13

ruiu created D45641: Rename sys::Process::GetArgumentVector -> sys::windows::GetCommandLineArguments.
Fri, Apr 13, 3:16 PM
ruiu committed rL330067: Use InitLLVM in clang as well..
Use InitLLVM in clang as well.
Fri, Apr 13, 2:01 PM
ruiu committed rC330067: Use InitLLVM in clang as well..
Use InitLLVM in clang as well.
Fri, Apr 13, 2:01 PM
ruiu closed D45634: Use InitLLVM in clang as well..
Fri, Apr 13, 2:01 PM
ruiu created D45634: Use InitLLVM in clang as well..
Fri, Apr 13, 1:46 PM
ruiu committed rLLD330046: Define InitLLVM to do common initialization all at once..
Define InitLLVM to do common initialization all at once.
Fri, Apr 13, 11:30 AM
ruiu committed rL330046: Define InitLLVM to do common initialization all at once..
Define InitLLVM to do common initialization all at once.
Fri, Apr 13, 11:30 AM
ruiu closed D45602: Define InitLLVM to do common initialization all at once..
Fri, Apr 13, 11:29 AM

Thu, Apr 12

ruiu updated the diff for D45602: Define InitLLVM to do common initialization all at once..
  • fix unsafe memory access issue
Thu, Apr 12, 8:12 PM
ruiu added inline comments to D45602: Define InitLLVM to do common initialization all at once..
Thu, Apr 12, 8:07 PM
ruiu updated the diff for D45602: Define InitLLVM to do common initialization all at once..
  • address review comment
Thu, Apr 12, 8:07 PM
ruiu updated the diff for D45602: Define InitLLVM to do common initialization all at once..
  • convert more tools to use InitLLVM
Thu, Apr 12, 7:46 PM
ruiu added inline comments to D45602: Define InitLLVM to do common initialization all at once..
Thu, Apr 12, 5:39 PM
ruiu updated the diff for D45602: Define InitLLVM to do common initialization all at once..
  • set banners to ExitOnError objects
Thu, Apr 12, 5:39 PM
ruiu created D45602: Define InitLLVM to do common initialization all at once..
Thu, Apr 12, 4:34 PM
ruiu accepted D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.

GetArgumentVector is used from a variety of places not all of which share the same code exit behavior (though most do). The same is true for PrintStackTraceOnErrorSignal and PrettyStackTraceProgram. Since all of these are frequently used together (and it's probably an omission of they are not), I agree it would be a great idea to create a simpler pattern than can be used by tools. To properly make the change, we would have to update all the tools to use the new pattern, which is also out of our scope right now. Perhaps we should open a bug/task to be addressed later?

Thu, Apr 12, 3:28 PM
ruiu added inline comments to D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.
Thu, Apr 12, 3:13 PM
ruiu committed rLLD329960: Do not keep shared symbols created from garbage-collected eliminated DSOs..
Do not keep shared symbols created from garbage-collected eliminated DSOs.
Thu, Apr 12, 3:00 PM
ruiu committed rL329960: Do not keep shared symbols created from garbage-collected eliminated DSOs..
Do not keep shared symbols created from garbage-collected eliminated DSOs.
Thu, Apr 12, 3:00 PM
ruiu closed D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..
Thu, Apr 12, 3:00 PM
ruiu added a comment to D45508: Implement --ctors-in-init-array..

I found that this patch isn't correct. Sorry guys. The issue is that we need to reverse the contents after applying relocations. This patch reverses it before applying relocations. I need to fix that. I'll update the patch.

Thu, Apr 12, 2:29 PM
ruiu added a comment to D45118: Linking debug (DWARF) sections from the WebAssembly object files.

I don't think I understand what you are trying to achieve with this patch in the first place, as you didn't explain it anywhere. Can you write a patch description properly so that we can understand the intention of the change and what the change is?

Thu, Apr 12, 2:27 PM
ruiu accepted D45142: [Transforms] Change std::sort to llvm::sort in response to r327219.

This is a mechanical change to replace std::sort with llvm::sort.

Thu, Apr 12, 2:00 PM
ruiu accepted D45139: [ProfileData] Change std::sort to llvm::sort in response to r327219.

LGTM

Thu, Apr 12, 2:00 PM
ruiu accepted D45138: [MC] Change std::sort to llvm::sort in response to r327219.

LGTM

Thu, Apr 12, 2:00 PM
ruiu accepted D45137: [LTO] Change std::sort to llvm::sort in response to r327219.

LGTM

Thu, Apr 12, 1:51 PM
ruiu added a comment to D45571: [ELF] - Speedup MergeInputSection::splitStrings.

What is your operating system and CPU?

Thu, Apr 12, 1:46 PM
ruiu added inline comments to D45571: [ELF] - Speedup MergeInputSection::splitStrings.
Thu, Apr 12, 1:45 PM
ruiu added a comment to D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.

I think I'm convinced. We probably should enable that particular test if Python 3.

Thu, Apr 12, 1:41 PM
ruiu added a comment to D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.

So, I mean I think what this patch is correct, but orthogonal to that, we should disable the failing test on Windows so that we don't think too hard about character conversion round trip issues.

Thu, Apr 12, 11:25 AM
ruiu added a comment to D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.

Thank you for the detailed explanation! That's very helpful. I agree that we shouldn't make a guess on character encoding of command line arguments, instead, we should use Windows APIs to get them in the correct encoding and then convert them to UTF-8. That definitely looks like the right direction.

Thu, Apr 12, 11:22 AM

Wed, Apr 11

ruiu added a comment to D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.

What are you actually trying to fix with the patch? Did you just find that lld/test/ELF/format-binary-non-ascii.s didn't pass on your machine and trying to fix it?

Wed, Apr 11, 9:31 PM
ruiu added a comment to D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.

That website recommends we handle all strings as UTF-8 whether externally or internally, and if you need to handle strings encoded not in UTF-8, you convert them to UTF-8 first. I'd agree that principle.

Wed, Apr 11, 9:30 PM
ruiu added a comment to D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms.

lld/test/ELF/format-binary-non-ascii.s may not run properly on Windows if the expected encoding for the command line option is not UTF-8, as the test file contains a pathname containing a non-ASCII character encoded in UTF-8 encoding. That is not immediately an issue on Windows.

Wed, Apr 11, 7:14 PM
ruiu added inline comments to D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..
Wed, Apr 11, 5:57 PM
ruiu added a comment to D45375: [ELF] - Introduce synthetic file for linker script symbols..

Would never having a null InputFile be sufficient?

Wed, Apr 11, 5:56 PM
ruiu created D45548: Avoid hash table lookup when sorting local symbols..
Wed, Apr 11, 5:52 PM
ruiu added a comment to D45375: [ELF] - Introduce synthetic file for linker script symbols..

The reality is that we already have Synthetic files, we just have a very special representation for them: a null pointer.

Wed, Apr 11, 5:46 PM
ruiu added a comment to D45519: [ELF] - Change the way of sorting local symbols..

Since the order of two files is arbitrary, how about just giving each InputFile a number when it is created ?

Wed, Apr 11, 5:43 PM
ruiu added a comment to D45375: [ELF] - Introduce synthetic file for linker script symbols..

I'm still in doubt if this is overall a win. At least this patch itself doesn't demonstrate the benefit of having an extra type of file, and I doubt if it will be. Can this simplify things significantly? I think I'm not convinced that we want this one.

Wed, Apr 11, 5:34 PM
ruiu added a comment to D45516: [ELF] - Refactor lazy symbol duplicated code..

Isn't it more complicated than before? I understand that it is tempting, but generally speaking, I try to *not* abstract things unless doing it is a clear win. I'm a bit worried the direction of your recent "refactoring" patches as it adds more abstractions for a (in my opinion) marginal benefits. Factoring out common code is not always a win in terms of readability, especially when it involves callback functions and such. Simple, repeated code is sometimes better, and the original code was written with that in mind. I really prefer boring code over clever code that uses higher order functions.

Wed, Apr 11, 5:31 PM
ruiu added a comment to D45519: [ELF] - Change the way of sorting local symbols..

Yeah, but the it looks up the hash table N times where N is the number of local symbols, no?

Wed, Apr 11, 5:26 PM
ruiu added a comment to D45519: [ELF] - Change the way of sorting local symbols..

George, can you try to not use hash table like this in the future? Both this patch and your previous patch use a hash table to store a lot of symbols, which is a violation of the rule I set to achieve a good performance. If it is unavoidable, please explain in the commit message so that we can discuss and examine if it is really unavoidable. Just using a hash table without an explanation isn't good.

Wed, Apr 11, 5:17 PM
ruiu added a comment to D45519: [ELF] - Change the way of sorting local symbols..

I mean just sorting a symbol shouldn't take 0.73% of the total execution time.

Wed, Apr 11, 5:12 PM
ruiu added a comment to D45519: [ELF] - Change the way of sorting local symbols..

Strictly speaking, this is a violation of the rule that we shouldn't use a hash table for symbols other than the Symbol Table. Is there any way to avoid using a new hash table?

Wed, Apr 11, 5:10 PM
ruiu updated the diff for D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..
  • fix a test and confirmed that the test fails without this patch
Wed, Apr 11, 4:55 PM
ruiu updated the diff for D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..
  • replace dangling shared symbols even if --gc-sections is not specified
Wed, Apr 11, 3:32 PM
ruiu added a comment to D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..

I"m not sure if I understand https://bugs.llvm.org/show_bug.cgi?id=28335 correctly, but it sounds like it fixes that bug as well. The intention of this change is, when a DSO is eliminated because of the garbage collection, all traces of the DSO should be removed as if the DSO weren't present in the command line from the beginning.

Wed, Apr 11, 2:50 PM
ruiu created D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..
Wed, Apr 11, 2:30 PM
ruiu updated the diff for D45508: Implement --ctors-in-init-array..
  • address review comments
Wed, Apr 11, 1:05 PM
ruiu abandoned D45533: Implement --ctors-in-init-array..

This change was created by mistake.

Wed, Apr 11, 1:05 PM
ruiu updated the diff for D45533: Implement --ctors-in-init-array..
  • address review comments
Wed, Apr 11, 1:01 PM
ruiu created D45533: Implement --ctors-in-init-array..
Wed, Apr 11, 1:01 PM