Page MenuHomePhabricator

grimar (George Rimar)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 21 2015, 12:36 AM (174 w, 4 h)

Recent Activity

Today

grimar accepted D56910: [llvm-readelf]Revert --dyn-symbols behaviour to make it GNU compatible, and add new --hash-symbols switch for old behaviour.

Patch looks reasonable to me too.

Mon, Jan 21, 12:06 AM

Yesterday

grimar added a comment to D56920: [PPC64] Sort .toc sections accessed with small code model relocs close to the .got.

! In D56920#1364896, @MaskRay wrote:
Both ld.bfd and gold have the similar "small_toc_reloc" notation and binutils-gdb/gold/power.cc does similar TOC sorting (ld.bfd has a much more complicated rule).
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=dff021e14a9c72380311c15a90c1a646b179b987;f=gold/powerpc.cc#l8376

Sun, Jan 20, 11:29 PM
grimar accepted D56955: COFF, ELF: Adjust ICF hash computation to account for self relocations..

I think this is OK. Please wait for Rui's opinion.

Sun, Jan 20, 1:16 AM
grimar added a comment to D56920: [PPC64] Sort .toc sections accessed with small code model relocs close to the .got.

I am not PPC expert, but approach itself seems probably OK to me. At least I do not have good ideas on how to do what you want much simpler/better atm.
Maybe other reviewers will have something.

Sun, Jan 20, 12:58 AM

Sat, Jan 19

grimar added a comment to D56927: Disable PIC/PIE for MSP430 target.

Can it be tested with a test case?

Sat, Jan 19, 10:15 AM
grimar added a comment to D56925: Do not use frame pointer by default for MSP430.

Testcase?

Sat, Jan 19, 10:14 AM
grimar added a comment to D56325: Sort symbols in .bss by size..

Another data point: Sorting the data section for Chrome on Android, data section dirty pages went down from 6308KB to 6280KB (arm32).

Or 0.5%..

0.5% may not sound a lot, but that's already a helpful amount for some systems, where you have to constantly make tradeoffs that sacrifices some functionalities to free up memory.

Sat, Jan 19, 9:55 AM

Fri, Jan 18

grimar accepted D56855: Add ___Z demangling to new common demangle function.

LGTM.

Fri, Jan 18, 4:59 AM
grimar committed rL351547: [llvm-objdump] - Dump the archive headers when -all-headers is specified..
[llvm-objdump] - Dump the archive headers when -all-headers is specified.
Fri, Jan 18, 4:06 AM
grimar closed D56780: [llvm-objdump] - Dump the archive headers when -all-headers is specified.
Fri, Jan 18, 4:06 AM
grimar committed rL351545: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm….
[llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm…
Fri, Jan 18, 3:38 AM
grimar closed D56842: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm-objdump.cpp.
Fri, Jan 18, 3:38 AM
grimar added inline comments to D56855: Add ___Z demangling to new common demangle function.
Fri, Jan 18, 3:19 AM
grimar committed rL351542: [llvm-objdump] - Show aliases in -help..
[llvm-objdump] - Show aliases in -help.
Fri, Jan 18, 2:45 AM
grimar closed D56853: [llvm-objdump] - Show aliases in -help..
Fri, Jan 18, 2:45 AM
grimar added inline comments to D56855: Add ___Z demangling to new common demangle function.
Fri, Jan 18, 2:36 AM
grimar accepted D56855: Add ___Z demangling to new common demangle function.
Fri, Jan 18, 2:33 AM
grimar added inline comments to D56855: Add ___Z demangling to new common demangle function.
Fri, Jan 18, 2:30 AM
grimar added a comment to D56855: Add ___Z demangling to new common demangle function.

minor nit:

Fri, Jan 18, 2:28 AM
grimar added a comment to D56325: Sort symbols in .bss by size..

Another data point: Sorting the data section for Chrome on Android, data section dirty pages went down from 6308KB to 6280KB (arm32).

Fri, Jan 18, 2:21 AM
grimar added inline comments to D56842: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm-objdump.cpp.
Fri, Jan 18, 1:46 AM
grimar added a comment to D56722: [lld] [ELF] Support for warn-once option.

Without warn-once option on this example lld prints 20 error messages (-O2 optimization while making object file), with warn-once option it prints just one error message, so if we have similar code in many places this option might be useful.

Fri, Jan 18, 1:01 AM
grimar added a comment to D56853: [llvm-objdump] - Show aliases in -help..

LGTM

I think this is a good change, and I agree with you that we don't need to test each alias.

Speaking of the --help message, I found there are other weird stuff there. The options in the --help message are categorized as "Color Options", "General options" and "Generic Options". There is only one option under the "Color Options" category, which is --color. Most options are under "General Options". --help and --version are under "Generic Options". Frankly, these categories doesn't make sense at all to me. (what's the difference between "Generic" and "General"?)

Maybe we should remove the categories and display all options just asciibetically.

Fri, Jan 18, 12:42 AM

Thu, Jan 17

grimar created D56853: [llvm-objdump] - Show aliases in -help..
Thu, Jan 17, 7:04 AM
grimar accepted D56721: Move llvm-objdump demangle function into demangler library.

This is probably fine.

Thu, Jan 17, 6:45 AM
grimar added inline comments to D56780: [llvm-objdump] - Dump the archive headers when -all-headers is specified.
Thu, Jan 17, 6:43 AM
grimar added inline comments to D56780: [llvm-objdump] - Dump the archive headers when -all-headers is specified.
Thu, Jan 17, 6:20 AM
grimar added inline comments to D56780: [llvm-objdump] - Dump the archive headers when -all-headers is specified.
Thu, Jan 17, 6:11 AM
grimar updated the diff for D56780: [llvm-objdump] - Dump the archive headers when -all-headers is specified.

I added a requested test showing how -all-headers now dumps the archive headers.
(and removed the old test since the new one covers the added functionality).

Thu, Jan 17, 5:48 AM
grimar accepted D56791: [llvm-readobj][ELF]Add demangling support.

Thanks for the answers, James. LGTM.

Thu, Jan 17, 5:16 AM
grimar added inline comments to D56842: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm-objdump.cpp.
Thu, Jan 17, 5:02 AM
grimar updated the summary of D56842: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm-objdump.cpp.
Thu, Jan 17, 3:34 AM
grimar added a comment to D56842: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm-objdump.cpp.

This patch only moves the code and renames the few methods. No other changes were performed.

Thu, Jan 17, 3:33 AM
grimar added a child revision for D56721: Move llvm-objdump demangle function into demangler library: D56842: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm-objdump.cpp.
Thu, Jan 17, 3:31 AM
grimar created D56842: [llvm-objdump] - Move getRelocationValueString and dependenices out of the llvm-objdump.cpp.
Thu, Jan 17, 3:31 AM
grimar accepted D56721: Move llvm-objdump demangle function into demangler library.

LGTM.

Thu, Jan 17, 3:01 AM
grimar committed rL351418: [llvm-objdump] - Fix comment. NFC..
[llvm-objdump] - Fix comment. NFC.
Thu, Jan 17, 1:18 AM
grimar committed rL351417: [llvm-objdump] - Simplify the getRelocationValueString. NFCI..
[llvm-objdump] - Simplify the getRelocationValueString. NFCI.
Thu, Jan 17, 1:18 AM
grimar closed D56778: [llvm-objdump] - Simplify the getRelocationValueString. NFCI..
Thu, Jan 17, 1:18 AM

Wed, Jan 16

grimar added inline comments to D56791: [llvm-readobj][ELF]Add demangling support.
Wed, Jan 16, 11:53 PM
grimar added a comment to D56780: [llvm-objdump] - Dump the archive headers when -all-headers is specified.

The code change looks good to me, but I'd like a bit more testing, if it's okay. I assume that the output is something like the following for each archive member in turn:

member1.o:
<archive header>
<file header>
<section headers>
...
member2.o:
<archive header>
...

but I don't see anything showing that we're printing things that way (instead of printing all archive headers, then all file headers etc): we should have a test for that.

Wed, Jan 16, 7:51 AM
grimar added inline comments to D56779: [ELF][X86_64] Fix corrupted LD -> LE optimization for TLS without PLT.
Wed, Jan 16, 7:46 AM
grimar added a comment to D56779: [ELF][X86_64] Fix corrupted LD -> LE optimization for TLS without PLT.

I think it is good. My comments are below (please wait for Rui's ones though, he might have different opinions I guess).

Wed, Jan 16, 6:30 AM
grimar created D56780: [llvm-objdump] - Dump the archive headers when -all-headers is specified.
Wed, Jan 16, 5:49 AM
grimar created D56778: [llvm-objdump] - Simplify the getRelocationValueString. NFCI..
Wed, Jan 16, 5:08 AM
grimar added a comment to D56721: Move llvm-objdump demangle function into demangler library.

OK, I have no more comments.

Wed, Jan 16, 4:16 AM
grimar added inline comments to D56721: Move llvm-objdump demangle function into demangler library.
Wed, Jan 16, 3:37 AM
grimar added inline comments to D56721: Move llvm-objdump demangle function into demangler library.
Wed, Jan 16, 3:19 AM
grimar committed rLLDB351313: [lldb] - Fix crash when listing the history with the key up..
[lldb] - Fix crash when listing the history with the key up.
Wed, Jan 16, 1:31 AM
grimar committed rL351313: [lldb] - Fix crash when listing the history with the key up..
[lldb] - Fix crash when listing the history with the key up.
Wed, Jan 16, 1:31 AM
grimar closed D56014: [lldb] - Fix crash when listing the history with the key up..
Wed, Jan 16, 1:31 AM
grimar added a comment to D56014: [lldb] - Fix crash when listing the history with the key up..

Although I don't want to condone checking things in without a test case, given that the fix is obvious and that testing it is non-trivial, I think it's fine to check this in as is.

Wed, Jan 16, 1:31 AM
grimar added inline comments to D56721: Move llvm-objdump demangle function into demangler library.
Wed, Jan 16, 1:12 AM

Tue, Jan 15

grimar committed rL351192: [llvm-obdump] - Fix the help lines for -stop-address and -z..
[llvm-obdump] - Fix the help lines for -stop-address and -z.
Tue, Jan 15, 6:07 AM
grimar committed rL351171: [llvm-objdump] - Cleanup the code. NFCI..
[llvm-objdump] - Cleanup the code. NFCI.
Tue, Jan 15, 1:25 AM
grimar closed D56637: [llvm-objdump] - Cleanup the code. NFCI..
Tue, Jan 15, 1:24 AM
grimar accepted D56623: Do not emit a corrupt symbol table entry for .rela_iplt_{start,end}..

LGTM.

Tue, Jan 15, 12:59 AM
grimar added a comment to D56060: ELF: BLOCK Keyword linker script support.

Given the following external comment, can this be closed?
https://github.com/ClangBuiltLinux/linux/issues/253#issuecomment-450460750

Tue, Jan 15, 12:57 AM
grimar added a comment to D56666: [LLD][ELF][AArch64] Add missing PLT relocations to isStaticLinkTimeConstant.

Thanks, Peter!

Tue, Jan 15, 12:29 AM

Mon, Jan 14

grimar abandoned D55995: [lldb] - Fix compilation with MSVS 2015 update 3.

Yep, MSVS2017 works fine here for me too. Thanks, Pavel! I am abandoning it.

Mon, Jan 14, 7:23 AM
grimar added inline comments to D56663: [MSP430] Improve support of 'interrupt' attribute.
Mon, Jan 14, 6:55 AM
grimar requested review of D56014: [lldb] - Fix crash when listing the history with the key up..
Mon, Jan 14, 5:24 AM
grimar added a comment to D56014: [lldb] - Fix crash when listing the history with the key up..

I ended up with the following test case:

Mon, Jan 14, 5:24 AM
grimar added a comment to D56637: [llvm-objdump] - Cleanup the code. NFCI..

LGTM.

Mon, Jan 14, 3:31 AM
grimar updated the diff for D56637: [llvm-objdump] - Cleanup the code. NFCI..
  • Addressed review comments.
Mon, Jan 14, 3:15 AM
grimar added inline comments to D56637: [llvm-objdump] - Cleanup the code. NFCI..
Mon, Jan 14, 3:00 AM
grimar added inline comments to D56637: [llvm-objdump] - Cleanup the code. NFCI..
Mon, Jan 14, 2:42 AM

Sat, Jan 12

grimar created D56637: [llvm-objdump] - Cleanup the code. NFCI..
Sat, Jan 12, 7:00 AM
grimar committed rL351006: [llvm-objdump] - Change the output for --all-headers..
[llvm-objdump] - Change the output for --all-headers.
Sat, Jan 12, 4:21 AM
grimar closed D56588: [llvm-objdump] - Change the output for --all-headers..
Sat, Jan 12, 4:21 AM
grimar added inline comments to D56623: Do not emit a corrupt symbol table entry for .rela_iplt_{start,end}..
Sat, Jan 12, 3:18 AM

Fri, Jan 11

grimar added inline comments to D56588: [llvm-objdump] - Change the output for --all-headers..
Fri, Jan 11, 5:07 AM
grimar created D56588: [llvm-objdump] - Change the output for --all-headers..
Fri, Jan 11, 4:15 AM
grimar planned changes to D56014: [lldb] - Fix crash when listing the history with the key up..

I guess it should be possible to trigger this from dotest tests via pexpect. I know we try to avoid it, but given that this is specifically testing terminal interaction, using it here seems appropriate.

Fri, Jan 11, 1:04 AM

Thu, Jan 10

grimar added a comment to D56325: Sort symbols in .bss by size..

I don't think we need a detailed benchmark for other targets, as how programs use .bss (and in general other parts of data sections) doesn't depend too much on targets.

Thu, Jan 10, 11:39 AM
grimar added a comment to D56535: Support MSP430.

How mature is this implementation of MSP support? I'd like to add a note to the release notes for the next major release.

Thu, Jan 10, 11:27 AM
grimar added a comment to D56535: Support MSP430.

And I believe you didn't really have to rush to submit; this is a good patch, so just relax and wait for a little for others to review it as well.

Thu, Jan 10, 10:35 AM
grimar committed rLLD350842: [LLD][ELF] - Fix tests after r350840..
[LLD][ELF] - Fix tests after r350840.
Thu, Jan 10, 8:30 AM
grimar committed rL350842: [LLD][ELF] - Fix tests after r350840..
[LLD][ELF] - Fix tests after r350840.
Thu, Jan 10, 8:29 AM
grimar committed rL350840: [llvm-objdump] - Do not include reserved undefined symbol in -t output..
[llvm-objdump] - Do not include reserved undefined symbol in -t output.
Thu, Jan 10, 8:28 AM
grimar closed D56076: [llvm-objdump] - Do not include reserved undefined symbol in -t output..
Thu, Jan 10, 8:28 AM
grimar added a comment to D56076: [llvm-objdump] - Do not include reserved undefined symbol in -t output..

Thanks, James!

Thu, Jan 10, 8:27 AM
grimar committed rLLD350833: [LLD][ELF] - A follow up for r350819 ("Support MSP430") : add a test case….
[LLD][ELF] - A follow up for r350819 ("Support MSP430") : add a test case…
Thu, Jan 10, 7:40 AM
grimar committed rL350833: [LLD][ELF] - A follow up for r350819 ("Support MSP430") : add a test case….
[LLD][ELF] - A follow up for r350819 ("Support MSP430") : add a test case…
Thu, Jan 10, 7:40 AM
grimar added a comment to D56535: Support MSP430.

Oops, I missed a line, sorry for that. Re-uploaded.

r350819. Thanks!

It seems like the test/ELF/msp430.s file got lost while committing.

Thu, Jan 10, 7:39 AM
grimar planned changes to D55995: [lldb] - Fix compilation with MSVS 2015 update 3.

I'll try to switch to MSVS 2017 and recheck this.

Thu, Jan 10, 7:35 AM
grimar added a comment to D55995: [lldb] - Fix compilation with MSVS 2015 update 3.

BTW, I've been compiling lldb with VS2017 and haven't run into this issue. Also, with the imminent release of VS2019, I expect someone will soon start a thread proposing to drop VS2015, so you might be better of switching to a newer compiler. (But I don't have a problem with checking this in while we still officially support this compiler.)

Thu, Jan 10, 7:34 AM
grimar committed rLLD350824: [LLD][ELF] - Fix the test cases after r350823..
[LLD][ELF] - Fix the test cases after r350823.
Thu, Jan 10, 7:02 AM
grimar committed rL350824: [LLD][ELF] - Fix the test cases after r350823..
[LLD][ELF] - Fix the test cases after r350823.
Thu, Jan 10, 7:02 AM
grimar committed rL350823: [llvm-objdump] - Implement -z/--disassemble-zeroes..
[llvm-objdump] - Implement -z/--disassemble-zeroes.
Thu, Jan 10, 6:59 AM
grimar closed D56083: [llvm-objdump] - Implement -z/--disassemble-zeroes.
Thu, Jan 10, 6:59 AM
grimar committed rLLD350819: [LLD][ELF] - Support MSP430..
[LLD][ELF] - Support MSP430.
Thu, Jan 10, 5:49 AM
grimar committed rL350819: [LLD][ELF] - Support MSP430..
[LLD][ELF] - Support MSP430.
Thu, Jan 10, 5:48 AM
grimar closed D56535: Support MSP430.
Thu, Jan 10, 5:48 AM
grimar added a comment to D56535: Support MSP430.

Oops, I missed a line, sorry for that. Re-uploaded.

Thu, Jan 10, 5:48 AM
grimar added a comment to D56535: Support MSP430.

@grimar Thank you for the review. Could you please commit this change?

Thu, Jan 10, 5:21 AM
grimar accepted D56535: Support MSP430.

LGTM.

Thu, Jan 10, 4:35 AM
grimar added a comment to D40147: [MIPS] Handle cross-mode (regular <-> microMIPS) jumps.

I am pretty sure we can land this because having a patch on a review for about a year is always a terrible thing. My comments are inline.

Thu, Jan 10, 3:50 AM · lld
grimar added inline comments to D56535: Support MSP430.
Thu, Jan 10, 3:20 AM
grimar added a comment to D56535: Support MSP430.

Overall looks good to me. Comments are inlined.

Thu, Jan 10, 3:18 AM