Page MenuHomePhabricator

echristo (Eric Christopher)
User

Projects

User Details

User Since
Oct 15 2012, 2:12 PM (344 w, 17 h)

Recent Activity

Thu, May 16

echristo accepted D61990: [X86] Ignore "short" even harder in Intel ASM..

Be nice to split the predicate change from the short change. No need to re-review though, they're both fine.

Thu, May 16, 4:03 PM · Restricted Project

Wed, May 15

echristo committed rG29ff0f25196a: Fix typo in comment of CSAction -> Action. (authored by echristo).
Fix typo in comment of CSAction -> Action.
Wed, May 15, 6:07 PM
echristo committed rL360834: Fix typo in comment of CSAction -> Action..
Fix typo in comment of CSAction -> Action.
Wed, May 15, 6:05 PM

Tue, May 14

echristo committed rG030b17db665a: Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI… (authored by echristo).
Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI…
Tue, May 14, 12:39 PM
echristo added a reverting change for rGbdb21337e6e1: Change -gz and -Wa,--compress-debug-sections to use gABI compression…: rG030b17db665a: Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI….
Tue, May 14, 12:39 PM
echristo committed rL360703: Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI….
Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI…
Tue, May 14, 12:39 PM
echristo committed rC360703: Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI….
Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI…
Tue, May 14, 12:38 PM
echristo added inline comments to D61846: [DAGCombiner] Fix invalid alias analysis..
Tue, May 14, 11:50 AM · Restricted Project

Fri, May 10

echristo committed rGa2d876c95a88: Remove an unnecessary header from SROA.h. (authored by echristo).
Remove an unnecessary header from SROA.h.
Fri, May 10, 12:59 AM
echristo committed rL360410: Remove an unnecessary header from SROA.h..
Remove an unnecessary header from SROA.h.
Fri, May 10, 12:59 AM
echristo added a comment to D61737: [lldb] add -ex CLI option as alias to --one-line.

I would rather not clutter up the lldb command driver's options with gdb command flags. That seems like it will make lldb harder to figure out and reduce our freedom to choose reasonable short names for lldb driver options.

It was reasonable to add lldb aliases for the gdb commands that you use tens to hundreds of times in a give debugging session - those get wired into your hands... But I don't think the same consideration holds for command line options...

If we feel the need to add a driver gdb compatibility mode like this, I like Rafael's suggestion of:

lldb --gdb <everything after this is handled by our gdb emulation parser>

Fri, May 10, 12:45 AM · Restricted Project

Thu, May 9

echristo accepted D61762: [NFC][compiler-rt][builtins] Tidy and match comments for floating point operations.
Thu, May 9, 3:27 PM · Restricted Project, Restricted Project

Wed, May 8

echristo committed rGc93f56d39e62: Temporarily Revert "[DebugInfo] Terminate more location-list ranges at the end… (authored by echristo).
Temporarily Revert "[DebugInfo] Terminate more location-list ranges at the end…
Wed, May 8, 4:54 PM
echristo committed rL360301: Temporarily Revert "[DebugInfo] Terminate more location-list ranges at the end….
Temporarily Revert "[DebugInfo] Terminate more location-list ranges at the end…
Wed, May 8, 4:51 PM

Tue, May 7

echristo closed D61546: Stop the DAG combiner from combining vector stores greater than preferred vector width....

This happened here:

Tue, May 7, 12:33 PM · Restricted Project
echristo committed rG472722173440: Make sure that the DAG combiner doesn't merge stores that we explicitly asked… (authored by echristo).
Make sure that the DAG combiner doesn't merge stores that we explicitly asked…
Tue, May 7, 12:24 PM
echristo committed rL360183: Make sure that the DAG combiner doesn't merge stores that we explicitly.
Make sure that the DAG combiner doesn't merge stores that we explicitly
Tue, May 7, 12:23 PM

Mon, May 6

echristo updated the diff for D61546: Stop the DAG combiner from combining vector stores greater than preferred vector width....

Update comments. Have testcase work for multiple sizes and multiple preferred vector sizes.

Mon, May 6, 10:18 PM · Restricted Project
echristo committed rGe2b7332d2d9f: Fix typo in risc-v register aliases. (authored by echristo).
Fix typo in risc-v register aliases.
Mon, May 6, 5:45 PM
echristo committed rC360104: Fix typo in risc-v register aliases..
Fix typo in risc-v register aliases.
Mon, May 6, 5:45 PM
echristo committed rL360104: Fix typo in risc-v register aliases..
Fix typo in risc-v register aliases.
Mon, May 6, 5:45 PM
echristo closed D61464: [RiscV] Typo in register aliases.
Mon, May 6, 5:44 PM · Restricted Project, Restricted Project
echristo added reviewers for D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++: saugustine, cmatthews.

Adding Sterling and Chris to this to take a look at the new layout :)

Mon, May 6, 3:16 PM · Restricted Project, Restricted Project, Restricted Project

Fri, May 3

echristo added a comment to D61546: Stop the DAG combiner from combining vector stores greater than preferred vector width....

Arguably this could use some more comments and I'll add those as well.

Fri, May 3, 8:17 PM · Restricted Project
echristo created D61546: Stop the DAG combiner from combining vector stores greater than preferred vector width....
Fri, May 3, 8:16 PM · Restricted Project

Thu, May 2

echristo committed rG86e2f169bb7b: Tidy up a comment, fix a typo, remove a comment that's obsolete. (authored by echristo).
Tidy up a comment, fix a typo, remove a comment that's obsolete.
Thu, May 2, 5:14 PM
echristo committed rL359852: Tidy up a comment, fix a typo, remove a comment that's obsolete..
Tidy up a comment, fix a typo, remove a comment that's obsolete.
Thu, May 2, 5:14 PM
echristo accepted D61464: [RiscV] Typo in register aliases.

LGTM. Sorry I didn't notice this earlier.

Thu, May 2, 5:07 PM · Restricted Project, Restricted Project
echristo committed rG88a0f138920d: Typo Functino->Function. (authored by echristo).
Typo Functino->Function.
Thu, May 2, 12:51 PM
echristo committed rL359821: Typo Functino->Function..
Typo Functino->Function.
Thu, May 2, 12:51 PM

Tue, Apr 30

echristo committed rGdb555ab4df0e: Make some comments that were meant to be for public documentation actually… (authored by echristo).
Make some comments that were meant to be for public documentation actually…
Tue, Apr 30, 6:27 PM
echristo committed rL359640: Make some comments that were meant to be for public documentation.
Make some comments that were meant to be for public documentation
Tue, Apr 30, 6:26 PM
echristo committed rG7a76e2b8cd7f: Add an include of Module since we actually access it now and remove the forward… (authored by echristo).
Add an include of Module since we actually access it now and remove the forward…
Tue, Apr 30, 2:54 PM
echristo committed rL359618: Add an include of Module since we actually access it now and remove.
Add an include of Module since we actually access it now and remove
Tue, Apr 30, 2:54 PM
echristo committed rG6435102c03e3: Fix a few -Werror warnings: - Remove a variable only used in an assert - Fix… (authored by echristo).
Fix a few -Werror warnings: - Remove a variable only used in an assert - Fix…
Tue, Apr 30, 2:45 PM
echristo committed rL359617: Fix a few -Werror warnings:.
Fix a few -Werror warnings:
Tue, Apr 30, 2:45 PM

Mon, Apr 29

echristo added a reviewer for D61253: DWARF v5: fix directory index in the line table: tamur.
Mon, Apr 29, 1:47 PM · Restricted Project

Apr 19 2019

echristo accepted D60931: [builtins] Use aliases for function redirects.
Apr 19 2019, 6:47 PM · Restricted Project, Restricted Project
echristo committed rGdfebd84eb32b: Remove the EnableEarlyCSEMemSSA set of options from the legacy and new pass… (authored by echristo).
Remove the EnableEarlyCSEMemSSA set of options from the legacy and new pass…
Apr 19 2019, 3:17 PM
echristo committed rL358789: Remove the EnableEarlyCSEMemSSA set of options from the legacy.
Remove the EnableEarlyCSEMemSSA set of options from the legacy
Apr 19 2019, 3:17 PM
echristo closed D60747: Remove EnableEarlyCSEMemSSA option.
Apr 19 2019, 3:17 PM · Restricted Project
echristo closed D60745: Remove RunSLPAfterLoopVectorization option....

Committed thusly:

Apr 19 2019, 2:46 PM · Restricted Project

Apr 18 2019

echristo added a comment to D53379: GSYM symbolication format.

We have a use case for this other than Breakpad.

Apr 18 2019, 5:13 PM
echristo added a comment to D60858: AMDGPU: Skip debug instructions in assert.

Sure. I'd have probably just checked for the function and maybe an instruction or two rather than the whole thing. It's more fragile that way and harder to understand what you're actually checking for since you mostly just said "doesn't crash" :)

Apr 18 2019, 2:05 PM
echristo added a comment to D60858: AMDGPU: Skip debug instructions in assert.

Looks ok to me. Are you worried about the correctness of the code? That seems to be a lot of matching?

Apr 18 2019, 1:00 PM
echristo added a comment to D60858: AMDGPU: Skip debug instructions in assert.

FWIW they're fine IMO too :)

Apr 18 2019, 3:52 AM

Apr 17 2019

echristo accepted D60487: [llvm] Prevent duplicate files in debug line header in dwarf 5: another attempt.

OK, this code could use some work, but not for you to refactor it more than you already have.

Apr 17 2019, 11:43 PM · Restricted Project
echristo added a comment to D60747: Remove EnableEarlyCSEMemSSA option.

This sounds like people are fine with me doing this...

Apr 17 2019, 11:31 PM · Restricted Project
echristo committed rGeff3b6fe7f69: Elaborate why we have an option on by default for enabling chr. (authored by echristo).
Elaborate why we have an option on by default for enabling chr.
Apr 17 2019, 11:17 PM
echristo committed rL358641: Elaborate why we have an option on by default for enabling chr..
Elaborate why we have an option on by default for enabling chr.
Apr 17 2019, 11:15 PM
echristo accepted D60841: [AsmPrinter] hoist %a output template to base class for ARM+Aarch64.

LGTM.

Apr 17 2019, 3:02 PM · Restricted Project

Apr 16 2019

echristo committed rL358552: Revert "Temporarily Revert "Add basic loop fusion pass."".
Revert "Temporarily Revert "Add basic loop fusion pass.""
Apr 16 2019, 9:55 PM
echristo committed rGe29874eaa04d: Revert "Add basic loop fusion pass." Per request. (authored by echristo).
Revert "Add basic loop fusion pass." Per request.
Apr 16 2019, 9:55 PM
echristo committed rL358553: Revert "Add basic loop fusion pass." Per request..
Revert "Add basic loop fusion pass." Per request.
Apr 16 2019, 9:55 PM
echristo committed rGcee313d288a4: Revert "Temporarily Revert "Add basic loop fusion pass."" (authored by echristo).
Revert "Temporarily Revert "Add basic loop fusion pass.""
Apr 16 2019, 9:55 PM
echristo committed rG0ebbf72a6346: Remove the run-slp-after-loop-vectorization option. (authored by echristo).
Remove the run-slp-after-loop-vectorization option.
Apr 16 2019, 7:28 PM
echristo committed rL358548: Remove the run-slp-after-loop-vectorization option..
Remove the run-slp-after-loop-vectorization option.
Apr 16 2019, 7:24 PM
echristo committed rL358546: Temporarily Revert "Add basic loop fusion pass.".
Temporarily Revert "Add basic loop fusion pass."
Apr 16 2019, 7:21 PM
echristo committed rGa86343512845: Temporarily Revert "Add basic loop fusion pass." As it's causing some bot… (authored by echristo).
Temporarily Revert "Add basic loop fusion pass." As it's causing some bot…
Apr 16 2019, 7:15 PM
echristo added a comment to D60745: Remove RunSLPAfterLoopVectorization option....

Do we need to adjust the new pass manager too?

Apr 16 2019, 5:19 PM · Restricted Project
echristo updated the diff for D60747: Remove EnableEarlyCSEMemSSA option.

Update for the new pass manager as well. :)

Apr 16 2019, 5:07 PM · Restricted Project
echristo added a comment to D60747: Remove EnableEarlyCSEMemSSA option.

I can't find any users of this flag, so I'm in favor of s/EnableEarlyCSEMemSSA/true/ if that simplifies things.

Is this still useful for debugging problems with MemSSA and GVNHoist?

If we just have to tweak this one place, IMO it's sufficiently straightforward to just manually flip between true/false in code. I've personally never used it to debug anything, though I can only speak for myself.

That was pretty much my thought. As far as the new pass manager - the flag isn't being used anywhere else other than this location :)

Apr 16 2019, 4:43 PM · Restricted Project
echristo accepted D60803: [AsmPrinter] defer %c to base class for ARM, PPC, and Hexagon. NFC.
Apr 16 2019, 4:34 PM · Restricted Project
echristo added a comment to D60747: Remove EnableEarlyCSEMemSSA option.

I can't find any users of this flag, so I'm in favor of s/EnableEarlyCSEMemSSA/true/ if that simplifies things.

Is this still useful for debugging problems with MemSSA and GVNHoist?

If we just have to tweak this one place, IMO it's sufficiently straightforward to just manually flip between true/false in code. I've personally never used it to debug anything, though I can only speak for myself.

Apr 16 2019, 4:07 PM · Restricted Project

Apr 15 2019

echristo created D60747: Remove EnableEarlyCSEMemSSA option.
Apr 15 2019, 7:21 PM · Restricted Project
echristo created D60745: Remove RunSLPAfterLoopVectorization option....
Apr 15 2019, 6:59 PM · Restricted Project
echristo committed rG3ad162bbebc4: Remove some more unused headers from MachineFunction.h and friends. (authored by echristo).
Remove some more unused headers from MachineFunction.h and friends.
Apr 15 2019, 6:06 PM
echristo committed rL358468: Remove some more unused headers from MachineFunction.h and friends..
Remove some more unused headers from MachineFunction.h and friends.
Apr 15 2019, 6:04 PM
echristo closed D60741: Remove some more unused headers from MachineFunction.h and friends..
Apr 15 2019, 6:04 PM · Restricted Project
echristo added a comment to D60741: Remove some more unused headers from MachineFunction.h and friends..

Sounds good to me.
How'd you test that the MachineMemOperand.h changes were OK? Looks like it doesn't have an implementation file, so I'm not sure it's included first in any file - which means it's harder to tell if removing headers from it is valid/keeps it standalone (since its inclusion context might be allowing it to get away without including necessary headers)

Apr 15 2019, 5:33 PM · Restricted Project
echristo created D60741: Remove some more unused headers from MachineFunction.h and friends..
Apr 15 2019, 5:23 PM · Restricted Project
echristo added a reviewer for D60738: [MSP430AsmPrinter] Refactor special cases out of printOperand. NFC: krisb.

Kristina has been in here the most, adding.

Apr 15 2019, 4:54 PM · Restricted Project
echristo accepted D60727: [NVPTXAsmPrinter] clean up dead code. NFC.

LGTM then.

Apr 15 2019, 1:05 PM · Restricted Project
echristo added a reviewer for D60727: [NVPTXAsmPrinter] clean up dead code. NFC: tra.

Nick: Can you do some archaeology on the original patch and find out if there was supposed to be something supported here?

Apr 15 2019, 10:50 AM · Restricted Project

Apr 12 2019

echristo committed rGb41448771911: Move getNumFrameInfos and getDwarfFrameInfos out of line and remove the MCDwarf. (authored by echristo).
Move getNumFrameInfos and getDwarfFrameInfos out of line and remove the MCDwarf.
Apr 12 2019, 12:44 AM
echristo committed rL358264: Move getNumFrameInfos and getDwarfFrameInfos out of line and remove.
Move getNumFrameInfos and getDwarfFrameInfos out of line and remove
Apr 12 2019, 12:44 AM
echristo committed rG6b06c6a5ef9f: Add explicit dependencies on MCSection.h and MCDwarf.h to the .cpp files rather… (authored by echristo).
Add explicit dependencies on MCSection.h and MCDwarf.h to the .cpp files rather…
Apr 12 2019, 12:44 AM
echristo committed rL358263: Add explicit dependencies on MCSection.h and MCDwarf.h to the .cpp.
Add explicit dependencies on MCSection.h and MCDwarf.h to the .cpp
Apr 12 2019, 12:38 AM
echristo committed rGb6926bdcff6d: Revert "[PowerPC] Add initialization for some ppc passes" (authored by echristo).
Revert "[PowerPC] Add initialization for some ppc passes"
Apr 12 2019, 12:16 AM
echristo added a reverting change for rG6f8f98ce8de7: [PowerPC] Add initialization for some ppc passes: rGb6926bdcff6d: Revert "[PowerPC] Add initialization for some ppc passes".
Apr 12 2019, 12:16 AM
echristo committed rL358260: Revert "[PowerPC] Add initialization for some ppc passes".
Revert "[PowerPC] Add initialization for some ppc passes"
Apr 12 2019, 12:15 AM

Apr 11 2019

echristo committed rG886a7b3b9cbb: Move addInitialFrameState out of line and remove the MCDwarf.h include. (authored by echristo).
Move addInitialFrameState out of line and remove the MCDwarf.h include.
Apr 11 2019, 11:56 PM
echristo committed rL358258: Move addInitialFrameState out of line and remove the MCDwarf.h include..
Move addInitialFrameState out of line and remove the MCDwarf.h include.
Apr 11 2019, 11:56 PM
echristo committed rG8bbc3039be69: Move addFrameInst out of line and remove the MCDwarf.h include. (authored by echristo).
Move addFrameInst out of line and remove the MCDwarf.h include.
Apr 11 2019, 11:31 PM
echristo committed rL358255: Move addFrameInst out of line and remove the MCDwarf.h include..
Move addFrameInst out of line and remove the MCDwarf.h include.
Apr 11 2019, 11:30 PM
echristo committed rGb6c190da2312: Include what's used in a few cpp files - these were getting transitive includes… (authored by echristo).
Include what's used in a few cpp files - these were getting transitive includes…
Apr 11 2019, 11:16 PM
echristo committed rL358254: Include what's used in a few cpp files - these were getting transitive.
Include what's used in a few cpp files - these were getting transitive
Apr 11 2019, 11:14 PM
echristo committed rG06bfe353febb: Move a couple of optional references to just optional to make the forwarding… (authored by echristo).
Move a couple of optional references to just optional to make the forwarding…
Apr 11 2019, 8:50 PM
echristo committed rL358250: Move a couple of optional references to just optional to make the.
Move a couple of optional references to just optional to make the
Apr 11 2019, 8:47 PM
echristo committed rG492cad51a470: Remove a parameter that was being passed around that we had at the local… (authored by echristo).
Remove a parameter that was being passed around that we had at the local…
Apr 11 2019, 6:03 PM
echristo committed rL358244: Remove a parameter that was being passed around that we had at the.
Remove a parameter that was being passed around that we had at the
Apr 11 2019, 6:03 PM
echristo accepted D60577: [X86AsmPrinter] refactor static functions into private methods. NFC.

I have zero context on this change specifically, but "we should prefer functions in a private namespace" seems to be the exact opposite of the advice in https://llvm.org/docs/CodingStandards.html#static

While the commit comment references possibly moving things to a private namespace (which I agree is in contradiction to the referenced page), the actual code moves these to being private member functions. For functions that are taking *this, or propagating a *this in all usage, it seems fairly appropriate to me. Updating the commit message to not disagree with the official guidance would be helpful.

Yup, the change itself makes perfect sense, just that part of the commit message stood out :)

Apr 11 2019, 3:29 PM · Restricted Project

Apr 10 2019

echristo accepted D60526: [X86AsmPrinter] refactor to limit use of Modifier. NFC.

LGTM. Thanks!

Apr 10 2019, 11:54 AM · Restricted Project

Apr 9 2019

echristo accepted D60488: [AsmPrinter] refactor to remove remove AsmVariant. NFC.

One inline comment for a future cleanup, otherwise LGTM and thanks!

Apr 9 2019, 3:13 PM · Restricted Project
echristo committed rG202c9b99e00b: Remove the unit at a time option Removes the code from opt and the pass manager… (authored by echristo).
Remove the unit at a time option Removes the code from opt and the pass manager…
Apr 9 2019, 11:31 AM
echristo committed rL358024: Remove the unit at a time option.
Remove the unit at a time option
Apr 9 2019, 11:31 AM

Apr 8 2019

echristo committed rG88c70ec68e41: Include omitted word in comment. (authored by echristo).
Include omitted word in comment.
Apr 8 2019, 11:35 PM
echristo committed rL357967: Include omitted word in comment..
Include omitted word in comment.
Apr 8 2019, 11:34 PM
echristo closed D60430: Add a reduced copy of the llvm .gitignore as a start for the monorepo .gitignore.

This went here:

Apr 8 2019, 6:19 PM · Restricted Project
echristo committed rG6f75a8f5d067: Add a reduced copy of the llvm .gitignore as a start for the monorepo . (authored by echristo).
Add a reduced copy of the llvm .gitignore as a start for the monorepo .
Apr 8 2019, 5:52 PM