Page MenuHomePhabricator

pcwalton (Patrick Walton)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 12 2016, 3:40 PM (307 w, 4 d)

Recent Activity

Fri, Jun 24

pcwalton added a comment to D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..

I verified that the failing tests don't break locally.

Fri, Jun 24, 5:24 PM · Restricted Project, Restricted Project

Thu, Jun 23

pcwalton added a comment to D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..

The test failures in libomp don't look related to me.

Thu, Jun 23, 4:56 PM · Restricted Project, Restricted Project

Fri, Jun 17

pcwalton updated the summary of D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..
Fri, Jun 17, 5:57 PM · Restricted Project, Restricted Project
pcwalton added a comment to D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..

This should be ready for review now.

Fri, Jun 17, 5:56 PM · Restricted Project, Restricted Project
pcwalton updated the diff for D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..

So this was a fun one. The previous version of this patch accidentally caused a
pre-existing bug to surface and break Chromium. In certain circumstances
including the Chromium test case, available_externally symbols would cause
invalid lookups in the SymSize table. Before this patch, the lookups were
done with the [] operator, which would succeed but cause an unspecified value
to be emitted in the .debug_aranges table. But this patch moves those lookups
to be done by dereferencing the result of the .find() method, which causes
asserts if the key is not found.

Fri, Jun 17, 5:54 PM · Restricted Project, Restricted Project

Thu, Jun 16

pcwalton added a comment to D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..

Minimal reproduction:

Thu, Jun 16, 11:12 AM · Restricted Project, Restricted Project

Wed, Jun 8

pcwalton updated the diff for D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..

Updated the diff to address comments.

Wed, Jun 8, 11:50 AM · Restricted Project, Restricted Project

Tue, Jun 7

pcwalton added inline comments to D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..
Tue, Jun 7, 2:44 PM · Restricted Project, Restricted Project

Jun 2 2022

pcwalton added a comment to D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..

Test failures look unrelated?

Jun 2 2022, 10:38 AM · Restricted Project, Restricted Project

Jun 1 2022

pcwalton added a reviewer for D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic).: dblaikie.
Jun 1 2022, 3:48 PM · Restricted Project, Restricted Project
pcwalton added a comment to D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

Abandoned in favor of D126835.

Jun 1 2022, 3:47 PM · Restricted Project, Restricted Project
pcwalton requested review of D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic)..
Jun 1 2022, 3:45 PM · Restricted Project, Restricted Project

May 31 2022

pcwalton added a comment to D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

I see. I don't have commit access unfortunately.

May 31 2022, 2:12 AM · Restricted Project, Restricted Project
pcwalton added a comment to D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

Sorry for the delay. I was looking at this on Friday but didn't get around to finishing it. Feel free to revert in the meantime.

May 31 2022, 1:36 AM · Restricted Project, Restricted Project

May 25 2022

pcwalton abandoned D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..

Closing in favor of D126257.

May 25 2022, 1:17 PM · Restricted Project, Restricted Project
pcwalton added a comment to D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

Oh, what I mean is that our choices in the case in which zero-sized global symbols are forbidden at the IR level would be (1) don't emit source-level zero-sized symbols into LLVM IR (or don't emit DWARF metadata for them), or (2) round all zero-sized symbols up to one byte. If we pick (1), I don't think they would show up in the debug info, which could be confusing for programmers. If we pick (2), then certain abstractions which are zero-cost today in Rust become non-zero-cost.

May 25 2022, 1:03 PM · Restricted Project, Restricted Project

May 24 2022

pcwalton added a comment to D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

Wouldn't that mean that those zero-sized data symbols won't show up in the debugger anymore? That might be confusing for Rust users, who do use zero-sized globals reasonably commonly (to attach methods to).

May 24 2022, 5:13 PM · Restricted Project, Restricted Project
pcwalton added a comment to D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

How's this?

May 24 2022, 4:35 PM · Restricted Project, Restricted Project
pcwalton updated the diff for D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

Emit a constant 1 instead of a more complicated MCExpr when emitting symbols of length 1.

May 24 2022, 4:34 PM · Restricted Project, Restricted Project

May 23 2022

pcwalton added inline comments to D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..
May 23 2022, 4:35 PM · Restricted Project, Restricted Project
pcwalton added a comment to D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..

@dblaikie Here's a new version of the patch that takes the alternate approach you suggested of rounding lengths up to 1 byte. Feel free to take either diff and I'll close the other.

May 23 2022, 4:34 PM · Restricted Project, Restricted Project
pcwalton added a reviewer for D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`.: dblaikie.
May 23 2022, 4:33 PM · Restricted Project, Restricted Project
pcwalton requested review of D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`..
May 23 2022, 4:32 PM · Restricted Project, Restricted Project
pcwalton added inline comments to D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..
May 23 2022, 4:22 PM · Restricted Project, Restricted Project
pcwalton added inline comments to D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..
May 23 2022, 10:49 AM · Restricted Project, Restricted Project

May 20 2022

pcwalton added a comment to D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..

I like the idea of moving in the direction of forbidding zero-sized functions (pretty sure Rust never emits these) but supporting zero-sized globals.

May 20 2022, 1:40 PM · Restricted Project, Restricted Project
pcwalton updated the diff for D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..

Added a test.

May 20 2022, 1:38 PM · Restricted Project, Restricted Project

May 19 2022

pcwalton added a comment to D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..

That being said, forbidding global zero-sized symbols might work for us, because I think those are pretty rare. They do appear in practice sometimes, but not commonly.

May 19 2022, 4:14 PM · Restricted Project, Restricted Project
pcwalton added a comment to D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..

Well, named zero-sized values are a first-class feature of Rust. You can write code like this:

May 19 2022, 4:06 PM · Restricted Project, Restricted Project
pcwalton added a reviewer for D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`.: dblaikie.
May 19 2022, 1:07 PM · Restricted Project, Restricted Project
pcwalton requested review of D126010: Make sure the AsmPrinter doesn't emit any zero-sized symbols to `.debug_aranges`..
May 19 2022, 1:06 PM · Restricted Project, Restricted Project

Sep 10 2021

pcwalton added a comment to D109565: Teach SimplifyCFG to fold switches into lookup tables in more cases..

Looks fine to me. Note that to trigger on Debug we also need the frontend changes at https://github.com/rust-lang/rust/pull/88832.

Sep 10 2021, 1:14 PM · Restricted Project

Oct 6 2017

pcwalton created D38648: Make the hardcoded Threshold value in capture tracking configurable via a hidden option.
Oct 6 2017, 3:17 PM

Jun 21 2017

pcwalton updated the diff for D34387: [PATCH 2/2] Implement "probe-stack" on x86.

Addressed review comments.

Jun 21 2017, 8:00 PM
pcwalton updated the diff for D34387: [PATCH 2/2] Implement "probe-stack" on x86.

Addressed comments. Reverted the EAX save refactoring.

Jun 21 2017, 3:14 PM
pcwalton updated the diff for D34387: [PATCH 2/2] Implement "probe-stack" on x86.

Addressed review comments.

Jun 21 2017, 2:13 PM
pcwalton added inline comments to D34387: [PATCH 2/2] Implement "probe-stack" on x86.
Jun 21 2017, 10:48 AM

Jun 20 2017

pcwalton updated the diff for D34387: [PATCH 2/2] Implement "probe-stack" on x86.

Addressed comments.

Jun 20 2017, 6:05 PM
pcwalton added a comment to D34387: [PATCH 2/2] Implement "probe-stack" on x86.

@pcwalton No, both the spill and the "magic" number are necessary. See https://reviews.llvm.org/D9858. ebx is the size of the frame, and 0x1000 is the probe interval to be performed (in other words, page size).

Jun 20 2017, 5:36 PM
pcwalton updated the diff for D34386: [PATCH 1/2] Add a "probe-stack" attribute (revised).

Addressed comments.

Jun 20 2017, 5:09 PM
pcwalton updated the diff for D34387: [PATCH 2/2] Implement "probe-stack" on x86.

Addressed review comments.

Jun 20 2017, 4:45 PM
pcwalton added inline comments to D34387: [PATCH 2/2] Implement "probe-stack" on x86.
Jun 20 2017, 12:27 PM
pcwalton added inline comments to D34387: [PATCH 2/2] Implement "probe-stack" on x86.
Jun 20 2017, 12:24 PM
pcwalton added inline comments to D34387: [PATCH 2/2] Implement "probe-stack" on x86.
Jun 20 2017, 12:10 PM
pcwalton updated the diff for D34387: [PATCH 2/2] Implement "probe-stack" on x86.

Added more context, added missing tests, removed whitespace-only changes.

Jun 20 2017, 10:56 AM
pcwalton updated the diff for D34386: [PATCH 1/2] Add a "probe-stack" attribute (revised).

Split this out correctly, added more context, and added the missing test.

Jun 20 2017, 10:17 AM
pcwalton added inline comments to D34386: [PATCH 1/2] Add a "probe-stack" attribute (revised).
Jun 20 2017, 10:09 AM

Jun 19 2017

pcwalton created D34387: [PATCH 2/2] Implement "probe-stack" on x86.
Jun 19 2017, 11:23 PM
pcwalton created D34386: [PATCH 1/2] Add a "probe-stack" attribute (revised).
Jun 19 2017, 11:20 PM

Aug 22 2016

pcwalton added inline comments to D23470: [memcpyopt] Memcpy-memcpy dependence isn't detected across basic blocks.
Aug 22 2016, 12:27 PM

Aug 12 2016

pcwalton retitled D23470: [memcpyopt] Memcpy-memcpy dependence isn't detected across basic blocks from to [memcpyopt] Memcpy-memcpy dependence isn't detected across basic blocks.
Aug 12 2016, 3:47 PM
pcwalton updated D23469: Make the memory dependence queries in the memcpy optimizer nonlocal.
Aug 12 2016, 3:43 PM
pcwalton retitled D23469: Make the memory dependence queries in the memcpy optimizer nonlocal from to Make the memory dependence queries in the memcpy optimizer nonlocal.
Aug 12 2016, 3:43 PM