Page MenuHomePhabricator

mhjacobson (Matt Jacobson)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 27 2021, 12:05 AM (25 w, 6 d)

Recent Activity

Sun, Jan 9

mhjacobson added a comment to D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space.

Happy new year! If this change is acceptable, could someone please merge it for me? (And if not, could someone point out what needs more work?) Thanks for your help!

Sun, Jan 9, 11:00 PM · Restricted Project

Dec 2 2021

mhjacobson added a comment to D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space.

Well, I'm hoping it doesn't prove to be too much work. 🙂 With this fix and D112049, most things are working quite well, though I'm sure I'll uncover more interesting things as I go.

Dec 2 2021, 8:44 PM · Restricted Project

Nov 30 2021

mhjacobson added a reviewer for D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space: ahatanak.
Nov 30 2021, 5:27 PM · Restricted Project
mhjacobson added a comment to D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space.

Ping. Is there anyone more suited to reviewing this patch?

Nov 30 2021, 5:27 PM · Restricted Project

Oct 19 2021

mhjacobson requested review of D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space.
Oct 19 2021, 5:15 PM · Restricted Project
mhjacobson added a comment to D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long.

The test is failing for a different reason (that I'd forgotten I worked around locally). I'll look into fixing that first. Sorry for noise.

Oct 19 2021, 12:43 PM · Restricted Project

Oct 18 2021

mhjacobson requested review of D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long.
Oct 18 2021, 7:28 PM · Restricted Project

Sep 21 2021

mhjacobson added inline comments to D95664: [AVR] Fix the eliminateCallFramePseudos method so that it always expands STDWSPQRr and STDSPQRr.
Sep 21 2021, 11:13 AM · Restricted Project
mhjacobson added a comment to D95664: [AVR] Fix the eliminateCallFramePseudos method so that it always expands STDWSPQRr and STDSPQRr.

I just realized I've been reading "frame pointer elimination" as "frame index elimination" but this change still doesn't make sense to me. Why do you want to treat these stores as frame instructions when they don't modify the stack pointer?

Sep 21 2021, 11:05 AM · Restricted Project

Sep 4 2021

mhjacobson accepted D109270: [NFC] Run clang-format on llvm/lib/Trget/AVR/.
Sep 4 2021, 7:02 AM · Restricted Project

Aug 12 2021

mhjacobson accepted D107853: [NFC][AVR] Remove unused isMachineVerifierClean().

Do we happen to know what change(s) made this pass work correctly for AVR?

Aug 12 2021, 11:02 AM · Restricted Project

Aug 9 2021

mhjacobson added a comment to D107684: [NFC][AVR][clang] Add test for D107672.

--sysroot %S/Inputs/basic_avr_tree/usr/lib seems incorrect.

sysroot is usually a directory which contains usr/lib and lib/. somewhere/usr/lib is not usually used as a sysroot.

Aug 9 2021, 9:00 PM · Restricted Project
mhjacobson updated the diff for D107684: [NFC][AVR][clang] Add test for D107672.

Rebase atop recent changes; use separate check lines and string substitution for SYSROOT.

Aug 9 2021, 8:50 PM · Restricted Project
mhjacobson accepted D107797: [Driver][test] Improve avr-toolchain.c.
Aug 9 2021, 8:10 PM · Restricted Project
mhjacobson added a comment to D107682: [AVR][clang] Improve search for avr-libc installation path.

For other examples of using four ..s, look in Gnu.cpp, where there are several places where getParentLibPath() is appended with /../. AddMultilibPaths(), PushPPaths().

getParentLibPath(), in turn, is GCCInstallPath/../../../

Thanks for your help. I have updated my patch. Now it should work for both your platform and mine. It search GCCRoot.getParentLibPath()/avr first, otherwise GCCRoot.getParentLibPath()/../avr.

Please help me check.

Aug 9 2021, 4:07 PM · Restricted Project
mhjacobson added inline comments to D107684: [NFC][AVR][clang] Add test for D107672.
Aug 9 2021, 3:58 PM · Restricted Project

Aug 7 2021

mhjacobson added a comment to D107684: [NFC][AVR][clang] Add test for D107672.

We need not add another basic_avr_tree_opt_local, since you added /avr to possible avr-libc pathes, you can test your change by specifying --sysroot %S/Inputs/basic_avr_tree/usr/lib/ .

Oh, good idea.

Sorry, only spefifying --sysroot %S/Inputs/basic_avr_tree/usr/lib/ does not work, since the search for avr-ld also relies on --sysroot.

Aug 7 2021, 2:35 AM · Restricted Project
mhjacobson updated the diff for D107684: [NFC][AVR][clang] Add test for D107672.

Simplify.

Aug 7 2021, 2:32 AM · Restricted Project
mhjacobson added a comment to D107682: [AVR][clang] Improve search for avr-libc installation path.

For other examples of using four ..s, look in Gnu.cpp, where there are several places where getParentLibPath() is appended with /../. AddMultilibPaths(), PushPPaths().

Aug 7 2021, 2:11 AM · Restricted Project
mhjacobson added a comment to D107684: [NFC][AVR][clang] Add test for D107672.

We need not add another basic_avr_tree_opt_local, since you added /avr to possible avr-libc pathes, you can test your change by specifying --sysroot %S/Inputs/basic_avr_tree/usr/lib/ .

Aug 7 2021, 1:44 AM · Restricted Project
mhjacobson added a comment to D107682: [AVR][clang] Improve search for avr-libc installation path.

@mhjacobson Could you please check if my modification also works as expected on your platform ?

Aug 7 2021, 1:43 AM · Restricted Project
mhjacobson added inline comments to D107682: [AVR][clang] Improve search for avr-libc installation path.
Aug 7 2021, 12:08 AM · Restricted Project

Aug 6 2021

mhjacobson requested review of D107684: [NFC][AVR][clang] Add test for D107672.
Aug 6 2021, 9:05 PM · Restricted Project
mhjacobson added a comment to D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr.

@benshi001 This was prematurely committed. It has no test and has apparent unaddressed issue. (I revamped many parts in the Linux/Gnu search paths so am quite confident about my observation.)

Aug 6 2021, 8:22 PM · Restricted Project
mhjacobson added a comment to D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr.

At first glance, I think you're right: avr-gcc does expect avr-libc to be relative to itself. That seems slightly weird to me (since avr-libc is technically separate from GCC), but OK.

Aug 6 2021, 8:12 PM · Restricted Project
mhjacobson added a comment to D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr.

I know nearly nothing about AVR, but from avr-gcc a.c '-###' I doubt this is the GCC behavior. GCC's library path is relative to the GCC installation. /usr/lib/gcc/avr/5.4.0/../../../avr/lib

There is a difference between sysroot inferred path and GCC installation inferred path. https://maskray.me/blog/2021-03-28-compiler-driver-and-cross-compilation

Aug 6 2021, 7:35 PM · Restricted Project
mhjacobson added a comment to D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr.

I'm also tempted to remove /usr/lib/avr, because these paths make no sense to me: $SYSROOT/usr/lib/avr/lib and $SYSROOT/usr/lib/avr/include.

Aug 6 2021, 3:40 PM · Restricted Project
mhjacobson added a comment to D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr.

I'm also tempted to remove /usr/lib/avr, because these paths make no sense to me: $SYSROOT/usr/lib/avr/lib and $SYSROOT/usr/lib/avr/include.

Aug 6 2021, 3:37 PM · Restricted Project
mhjacobson requested review of D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr.
Aug 6 2021, 3:34 PM · Restricted Project

Aug 5 2021

mhjacobson added a comment to D107610: [AVR][clang] Pass '-fno-use-init-array' to cc1.

Thanks!

Aug 5 2021, 7:30 PM · Restricted Project
mhjacobson requested review of D107610: [AVR][clang] Pass '-fno-use-init-array' to cc1.
Aug 5 2021, 4:05 PM · Restricted Project

Aug 4 2021

mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Thanks for your help, @benshi001 and @MaskRay!

Aug 4 2021, 7:49 PM · Restricted Project
mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Appeased the linter.

Aug 4 2021, 12:27 AM · Restricted Project
mhjacobson added inline comments to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.
Aug 4 2021, 12:23 AM · Restricted Project

Aug 3 2021

mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Move emission of __do_copy_data and __do_clear_bss into AVRAsmPrinter. (Leave AVRTargetStreamer as base class of AVRELFStreamer.)

Aug 3 2021, 11:38 PM · Restricted Project
mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

@MaskRay, do you have an opinion on whether I should move the other undefined-symbols definitions into AVRAsmPrinter.cpp to match?

Aug 3 2021, 10:24 PM · Restricted Project
mhjacobson added inline comments to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.
Aug 3 2021, 10:04 PM · Restricted Project
mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Removed incorrect use of setExternal().

Aug 3 2021, 10:03 PM · Restricted Project
mhjacobson added inline comments to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.
Aug 3 2021, 9:11 PM · Restricted Project
mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Defining __do_global_ctors looks like ld's work (in GNU ld), not the assembler's.

Aug 3 2021, 8:57 PM · Restricted Project

Aug 1 2021

mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Appeasing linter.

Aug 1 2021, 10:14 PM · Restricted Project
mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Just brainstorming. Another option might be to move the code in AVRTargetStreamer::finish() into a new override AVRAsmPrinter::doFinalization(). That would make it so that both bits of .globl code are in AVRAsmPrinter.

Aug 1 2021, 10:08 PM · Restricted Project
mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Just brainstorming. Another option might be to move the code in AVRTargetStreamer::finish() into a new override AVRAsmPrinter::doFinalization(). That would make it so that both bits of .globl code are in AVRAsmPrinter.

Aug 1 2021, 9:21 PM · Restricted Project
mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Sure, I'm open to that.

Aug 1 2021, 7:08 PM · Restricted Project

Jul 31 2021

mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Conform to source formatting convention.

Jul 31 2021, 8:14 PM · Restricted Project

Jul 30 2021

mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

You're right, AVR does have a section (.ctors) for the *table* of constructors. But there has to be some code that actually calls the functions in that table. That's what __do_global_ctors does. When present, it gets linked into the boot path (after things like __do_copy_data and __do_clear_bss) to do its work. But if nothing requests it, it won't be linked, and the constructor table will just sit around, never to be consulted.

Jul 30 2021, 11:45 PM · Restricted Project
mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Add assembly comment.

Jul 30 2021, 11:05 PM · Restricted Project
mhjacobson added inline comments to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.
Jul 30 2021, 10:51 PM · Restricted Project
mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Fix the ordering of the directives in the test, so that it actually passes! :)

Jul 30 2021, 10:40 PM · Restricted Project
mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Argh, the test fails now, because the ordering is wrong. Fixing it now.

Jul 30 2021, 10:34 PM · Restricted Project
mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Fixed alphabetization of includes.

Jul 30 2021, 9:42 PM · Restricted Project
mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Updated test with update_llc_test_checks.py. Note that update_llc_test_checks.py doesn't actually support AVR at the moment! I had to apply this fix first:

Jul 30 2021, 9:24 PM · Restricted Project
mhjacobson updated the diff for D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

Added a test.

Jul 30 2021, 1:30 PM · Restricted Project

Jul 29 2021

mhjacobson added a comment to D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.

I roughly followed the example of how __do_copy_data and __do_clear_bss are linked, in AVRTargetStreamer::finish().

Jul 29 2021, 11:53 PM · Restricted Project
mhjacobson requested review of D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`.
Jul 29 2021, 11:50 PM · Restricted Project
mhjacobson added a comment to D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain.

Thanks, Ben. Yes, would you mind to land the patch for me? Name and e-mail are

Jul 29 2021, 9:50 AM · Restricted Project

Jul 27 2021

mhjacobson updated the diff for D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain.

Explicitly added --start-group and --end-group to the expectation in the test.

Jul 27 2021, 9:39 PM · Restricted Project
mhjacobson added inline comments to D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain.
Jul 27 2021, 9:26 PM · Restricted Project
mhjacobson updated the diff for D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain.

Updated the diff to add more context.

Jul 27 2021, 9:26 PM · Restricted Project
mhjacobson added a reviewer for D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain: benshi001.
Jul 27 2021, 2:24 PM · Restricted Project
mhjacobson updated the diff for D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain.

Modified regression test Driver/avr-ld.c to account for the new arguments.

Jul 27 2021, 2:22 PM · Restricted Project
mhjacobson requested review of D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain.
Jul 27 2021, 2:10 AM · Restricted Project