Page MenuHomePhabricator

tejohnson (Teresa Johnson)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 27 2015, 11:17 AM (199 w, 4 d)

Recent Activity

Sat, Feb 16

tejohnson added a comment to D54176: [PGO] clang part of change for context-sensitive PGO..

LGTM except for place noted below where I disagree with a change made earlier. Will let @davidxl chime in if he disagrees with me or has any other comments.

Sat, Feb 16, 3:27 PM · Restricted Project
tejohnson added a comment to D54175: [PGO] context sensitive PGO.

LGTM, but wait for @davidxl to do final approval since I'm not sure his last comments addressed.

Sat, Feb 16, 2:57 PM
tejohnson added a comment to D50016: IR: Add entry/exit instrumentation symbols to the libcall list..

Needs a test.

Sat, Feb 16, 7:48 AM · Restricted Project

Thu, Feb 14

tejohnson accepted D58258: [HotColdSplit] Schedule splitting late to fix perf regression.

LGTM

Thu, Feb 14, 6:51 PM · Restricted Project
tejohnson added inline comments to D58258: [HotColdSplit] Schedule splitting late to fix perf regression.
Thu, Feb 14, 4:56 PM · Restricted Project
tejohnson added inline comments to D58258: [HotColdSplit] Schedule splitting late to fix perf regression.
Thu, Feb 14, 4:36 PM · Restricted Project
tejohnson added a comment to D57805: [HotColdSplit] Move splitting after instrumented PGO use.
In D57805#1398718, @vsk wrote:

... I will try moving the splitting to after the ThinLTO backend (post-thinlink) inlining and see what effect there is. Theoretically we should be getting more accurate importing/inlining, it would be good to understand where this is going wrong if not!

I have not yet tried the experiment you've described here. We've noticed that scheduling splitting early causes a regression for certain benchmarks in SPEC even without PGO data applied, however. As the heuristics for splitting are very conservative without PGO, this suggests that splitting before inlining may inadvertently hide important context from the optimizer.

Thu, Feb 14, 4:12 PM · Restricted Project
tejohnson committed rGd0b1f30b32bd: [ThinLTO] Detect partially split modules during the thin link (authored by tejohnson).
[ThinLTO] Detect partially split modules during the thin link
Thu, Feb 14, 1:23 PM
tejohnson committed rL354062: [ThinLTO] Detect partially split modules during the thin link.
[ThinLTO] Detect partially split modules during the thin link
Thu, Feb 14, 1:23 PM
tejohnson closed D57561: [ThinLTO] Detect partially split modules during the thin link.
Thu, Feb 14, 1:23 PM · Restricted Project
tejohnson updated the diff for D57561: [ThinLTO] Detect partially split modules during the thin link.

Address comment

Thu, Feb 14, 1:23 PM · Restricted Project
tejohnson added a comment to D57561: [ThinLTO] Detect partially split modules during the thin link.

ping

Thu, Feb 14, 9:18 AM · Restricted Project
tejohnson abandoned D58064: [ThinLTO] Record in index whether IR used a flattened sample PGO profile.

Decided to take a different approach for now.

Thu, Feb 14, 6:16 AM · Restricted Project
tejohnson committed rGc374a800e7f8: Refine ArgPromotion metadata handling (authored by tejohnson).
Refine ArgPromotion metadata handling
Thu, Feb 14, 6:14 AM
tejohnson committed rL354032: Refine ArgPromotion metadata handling.
Refine ArgPromotion metadata handling
Thu, Feb 14, 6:14 AM
tejohnson closed D58215: Refine ArgPromotion metadata handling.
Thu, Feb 14, 6:14 AM · Restricted Project

Wed, Feb 13

tejohnson updated the diff for D58215: Refine ArgPromotion metadata handling.

Add assert

Wed, Feb 13, 9:30 PM · Restricted Project
tejohnson updated the summary of D58215: Refine ArgPromotion metadata handling.
Wed, Feb 13, 3:53 PM · Restricted Project
tejohnson created D58215: Refine ArgPromotion metadata handling.
Wed, Feb 13, 3:49 PM · Restricted Project

Mon, Feb 11

tejohnson added a comment to D57805: [HotColdSplit] Move splitting after instrumented PGO use.
In D57805#1393416, @vsk wrote:

@tejohnson have you had a chance to evaluate performance with IR-PGO + splitting enabled?

Mon, Feb 11, 12:17 PM · Restricted Project
tejohnson accepted D56203: [IRReader] Expose getLazyIRModule.

The original change predates my involvement with LLVM, but it sounds like the only reason was just cleanup since it wasn't used elsewhere. I don't see any issue with adding this interface (similar to how we have exposed both parseIR and parseIRFile).

Mon, Feb 11, 10:37 AM · Restricted Project
tejohnson added inline comments to D58064: [ThinLTO] Record in index whether IR used a flattened sample PGO profile.
Mon, Feb 11, 10:22 AM · Restricted Project
tejohnson created D58064: [ThinLTO] Record in index whether IR used a flattened sample PGO profile.
Mon, Feb 11, 10:19 AM · Restricted Project

Fri, Feb 8

Herald added a project to D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting: Restricted Project.

Overall it looks ok to me, but I'd like Chandler to comment regarding the preferred way to do this with the new PM, since we don't tend to use booleans there in the PassBuilder to control passes. Is it preferable to instead use a new function attribute instead of boolean flags on the PMs (e.g. the way -fno-inline is handled)?

Fri, Feb 8, 9:34 AM · Restricted Project
tejohnson committed rG3ce8112dad68: ArgumentPromotion should copy all metadata to new Function (authored by tejohnson).
ArgumentPromotion should copy all metadata to new Function
Fri, Feb 8, 9:10 AM
tejohnson committed rL353537: ArgumentPromotion should copy all metadata to new Function.
ArgumentPromotion should copy all metadata to new Function
Fri, Feb 8, 9:10 AM
tejohnson closed D57846: ArgumentPromotion should copy all metadata to new Function.
Fri, Feb 8, 9:10 AM · Restricted Project
tejohnson added a comment to D57561: [ThinLTO] Detect partially split modules during the thin link.

ping

Fri, Feb 8, 6:34 AM · Restricted Project
Herald added a project to D57470: [ThinLTO] Restructor AliasSummary to contain ValueInfo of Aliasee: Restricted Project.

ping

Fri, Feb 8, 6:32 AM · Restricted Project

Thu, Feb 7

tejohnson committed rGc36c10ddfb3d: [HotColdSplit] With PGO add profile entry metadata to split cold function (authored by tejohnson).
[HotColdSplit] With PGO add profile entry metadata to split cold function
Thu, Feb 7, 9:51 AM
tejohnson committed rL353434: [HotColdSplit] With PGO add profile entry metadata to split cold function.
[HotColdSplit] With PGO add profile entry metadata to split cold function
Thu, Feb 7, 9:50 AM
tejohnson closed D57900: [HotColdSplit] With PGO add profile entry metadata to split cold function.
Thu, Feb 7, 9:50 AM · Restricted Project
tejohnson created D57900: [HotColdSplit] With PGO add profile entry metadata to split cold function.
Thu, Feb 7, 9:15 AM · Restricted Project

Wed, Feb 6

tejohnson created D57846: ArgumentPromotion should copy all metadata to new Function.
Wed, Feb 6, 2:02 PM · Restricted Project

Tue, Feb 5

tejohnson committed rG716abbeb4382: [HotColdSplit] Move splitting after instrumented PGO use (authored by tejohnson).
[HotColdSplit] Move splitting after instrumented PGO use
Tue, Feb 5, 8:30 PM
tejohnson committed rL353270: [HotColdSplit] Move splitting after instrumented PGO use.
[HotColdSplit] Move splitting after instrumented PGO use
Tue, Feb 5, 8:29 PM
tejohnson closed D57805: [HotColdSplit] Move splitting after instrumented PGO use.
Tue, Feb 5, 8:29 PM · Restricted Project
tejohnson updated the diff for D57805: [HotColdSplit] Move splitting after instrumented PGO use.

Implement suggestion

Tue, Feb 5, 8:29 PM · Restricted Project
tejohnson created D57805: [HotColdSplit] Move splitting after instrumented PGO use.
Tue, Feb 5, 8:23 PM · Restricted Project
tejohnson updated the diff for D55153: [ThinLTO] Implement index-based WPD.

Handle exported symbols correctly

Tue, Feb 5, 8:30 AM · Restricted Project
Herald added a project to D55153: [ThinLTO] Implement index-based WPD: Restricted Project.
In D55153#1373111, @pcc wrote:

Does this correctly handle the case where the only implementation is internal? I didn't see any code handling that. In the existing implementation this is done here: http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/WholeProgramDevirt.cpp#814

This is a good catch. I suspect I didn't hit it since I am marking all the added call edges as Hot, which means we are highly likely to import them, which will result in them getting promoted. But if we were to ever not import it would cause an issue. Let me see if I can confirm this, and the fix should be straightforward (ensure they are all treated as exported if we have references on type tests in other modules so we mark them as promoted in the index).

Tue, Feb 5, 7:53 AM · Restricted Project

Mon, Feb 4

tejohnson committed rGb0bf530fb5f1: [SamplePGO] More pipeline changes when flattened profile used in ThinLTO… (authored by tejohnson).
[SamplePGO] More pipeline changes when flattened profile used in ThinLTO…
Mon, Feb 4, 8:09 PM
tejohnson committed rL353135: [SamplePGO] More pipeline changes when flattened profile used in ThinLTO….
[SamplePGO] More pipeline changes when flattened profile used in ThinLTO…
Mon, Feb 4, 8:09 PM
tejohnson closed D57705: [SamplePGO] More pipeline changes when flattened profile used in ThinLTO postlink.
Mon, Feb 4, 8:09 PM · Restricted Project
tejohnson accepted D57726: [SamplePGO][NFC] Minor improvement to replace a temporary vector with a brace-enclosed init list.

LGTM

Mon, Feb 4, 4:54 PM · Restricted Project
tejohnson added a comment to D57706: [SamplePGO] Minor efficiency improvement in samplePGO ICP.

Ugh sorry for committing before your LGTM - I saw the LGTM come in from you on the other patch but committed this one instead! Will leave it since you subsequently LGTM'ed.

Mon, Feb 4, 4:37 PM · Restricted Project
tejohnson committed rG4bdf82ce7990: [SamplePGO] Minor efficiency improvement in samplePGO ICP (authored by tejohnson).
[SamplePGO] Minor efficiency improvement in samplePGO ICP
Mon, Feb 4, 4:20 PM
tejohnson committed rL353123: [SamplePGO] Minor efficiency improvement in samplePGO ICP.
[SamplePGO] Minor efficiency improvement in samplePGO ICP
Mon, Feb 4, 4:18 PM
tejohnson closed D57706: [SamplePGO] Minor efficiency improvement in samplePGO ICP.
Mon, Feb 4, 4:18 PM · Restricted Project
tejohnson created D57706: [SamplePGO] Minor efficiency improvement in samplePGO ICP.
Mon, Feb 4, 12:26 PM · Restricted Project
tejohnson created D57705: [SamplePGO] More pipeline changes when flattened profile used in ThinLTO postlink.
Mon, Feb 4, 11:51 AM · Restricted Project

Fri, Feb 1

tejohnson updated the diff for D54815: [ThinLTO] Add summary entries for index-based WPD.

Restore a couple of new tests that were lost when I updated the patch last.

Fri, Feb 1, 11:47 AM · Restricted Project
tejohnson updated the diff for D57561: [ThinLTO] Detect partially split modules during the thin link.

Address suggestion: move error checking to LTO.cpp

Fri, Feb 1, 11:22 AM · Restricted Project

Thu, Jan 31

tejohnson created D57561: [ThinLTO] Detect partially split modules during the thin link.
Thu, Jan 31, 6:46 PM · Restricted Project
tejohnson committed rL352770: Recommit "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT….
Recommit "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT…
Thu, Jan 31, 9:58 AM
tejohnson committed rL352768: Revert "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader".
Revert "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader"
Thu, Jan 31, 9:58 AM
tejohnson committed rL352763: [ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader.
[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader
Thu, Jan 31, 9:58 AM
tejohnson closed D57395: [ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader.
Thu, Jan 31, 9:58 AM

Wed, Jan 30

tejohnson created D57470: [ThinLTO] Restructor AliasSummary to contain ValueInfo of Aliasee.
Wed, Jan 30, 1:43 PM · Restricted Project
tejohnson added a comment to D54175: [PGO] context sensitive PGO.

Mostly minor comments inline. But still missing tests. Needs testing via new gold-plugin and opt options. Should also add options to llvm-lto2 to enable testing the LTOBackend path without needing gold or any other specific linker.

Wed, Jan 30, 12:54 PM

Tue, Jan 29

tejohnson added a comment to D55153: [ThinLTO] Implement index-based WPD.
In D55153#1373111, @pcc wrote:

Does this correctly handle the case where the only implementation is internal? I didn't see any code handling that. In the existing implementation this is done here: http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/WholeProgramDevirt.cpp#814

Tue, Jan 29, 9:50 AM · Restricted Project
tejohnson created D57395: [ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader.
Tue, Jan 29, 9:46 AM

Mon, Jan 28

tejohnson committed rL352446: Try to make new test more resilient to different orderings.
Try to make new test more resilient to different orderings
Mon, Jan 28, 6:04 PM
tejohnson committed rL352441: [ThinLTO] Add option to dump per-module summary dot graph.
[ThinLTO] Add option to dump per-module summary dot graph
Mon, Jan 28, 3:44 PM
tejohnson closed D57206: [ThinLTO] Add option to dump per-module summary dot graph.
Mon, Jan 28, 3:44 PM
tejohnson committed rL352438: [ThinLTO] Refine reachability check to fix compile time increase.
[ThinLTO] Refine reachability check to fix compile time increase
Mon, Jan 28, 2:28 PM
tejohnson closed D57203: [ThinLTO] Refine reachability check to fix compile time increase.
Mon, Jan 28, 2:28 PM
tejohnson added inline comments to D57203: [ThinLTO] Refine reachability check to fix compile time increase.
Mon, Jan 28, 1:30 PM

Fri, Jan 25

tejohnson added a comment to D54815: [ThinLTO] Add summary entries for index-based WPD.

Two clarifications below:

Fri, Jan 25, 4:45 PM · Restricted Project
tejohnson updated the diff for D54815: [ThinLTO] Add summary entries for index-based WPD.

Use an abbrev id in the bitcode for the new TYPE_ID_METADATA records.
This makes them a bit more compact.

Fri, Jan 25, 4:40 PM · Restricted Project
tejohnson added a comment to D54815: [ThinLTO] Add summary entries for index-based WPD.

Ping on review plus some numbers and a small improvement that I will upload shortly.

Fri, Jan 25, 4:37 PM · Restricted Project
tejohnson updated the diff for D57203: [ThinLTO] Refine reachability check to fix compile time increase.

Use any_of as suggested in comments.

Fri, Jan 25, 8:32 AM
tejohnson added inline comments to D57203: [ThinLTO] Refine reachability check to fix compile time increase.
Fri, Jan 25, 7:19 AM
tejohnson added inline comments to D57206: [ThinLTO] Add option to dump per-module summary dot graph.
Fri, Jan 25, 7:15 AM

Thu, Jan 24

tejohnson created D57206: [ThinLTO] Add option to dump per-module summary dot graph.
Thu, Jan 24, 4:55 PM
tejohnson created D57203: [ThinLTO] Refine reachability check to fix compile time increase.
Thu, Jan 24, 4:03 PM

Jan 23 2019

tejohnson added inline comments to D57082: [HotColdSplit] Move splitting earlier in the pipeline.
Jan 23 2019, 12:53 PM
tejohnson accepted D57082: [HotColdSplit] Move splitting earlier in the pipeline.

LGTM with suggestion about cutting down test.

Jan 23 2019, 11:54 AM
tejohnson added inline comments to D57082: [HotColdSplit] Move splitting earlier in the pipeline.
Jan 23 2019, 7:03 AM

Jan 18 2019

tejohnson committed rL351589: Make ThinLTO test run single threaded to try to avoid flakiness.
Make ThinLTO test run single threaded to try to avoid flakiness
Jan 18 2019, 12:46 PM

Jan 17 2019

tejohnson accepted D54819: [SampleFDO] Skip profile reading when flatten profile is used in ThinLTO postlink phase.

LGTM

Jan 17 2019, 11:14 AM
tejohnson committed rL351455: Revert "[ThinLTO] Add summary entries for index-based WPD".
Revert "[ThinLTO] Add summary entries for index-based WPD"
Jan 17 2019, 8:10 AM
tejohnson reopened D54815: [ThinLTO] Add summary entries for index-based WPD.

Reverted in r351455.

Jan 17 2019, 8:10 AM · Restricted Project
tejohnson added a comment to D54815: [ThinLTO] Add summary entries for index-based WPD.

This was a mistaken commit, reverting!

Jan 17 2019, 8:07 AM · Restricted Project
tejohnson committed rL351454: Add -dump-input=always to cfi-devirt test to debug flake.
Add -dump-input=always to cfi-devirt test to debug flake
Jan 17 2019, 7:53 AM
tejohnson committed rL351453: [ThinLTO] Add summary entries for index-based WPD.
[ThinLTO] Add summary entries for index-based WPD
Jan 17 2019, 7:53 AM
tejohnson closed D54815: [ThinLTO] Add summary entries for index-based WPD.
Jan 17 2019, 7:53 AM · Restricted Project

Jan 16 2019

tejohnson added a comment to D56749: [NFC] Make pgo related options in opt more consistent. .

(belated lgtm)

Jan 16 2019, 4:49 PM
tejohnson added a comment to D54175: [PGO] context sensitive PGO.

Bunch of mostly nit suggestions about names and commenting, and a few questions. I like the improved profile action expression.

Jan 16 2019, 1:47 PM

Jan 15 2019

tejohnson added a comment to D54177: PGO] change InstrProfData.inc for context sensitive PGO.

I noticed that this file is out of sync with the llvm version already, which you are also fixing here. Can you go ahead and commit just the sync with the current llvm head version now as an NFC change so that this patch is just the new changes?

Jan 15 2019, 11:54 AM
tejohnson added inline comments to D54819: [SampleFDO] Skip profile reading when flatten profile is used in ThinLTO postlink phase.
Jan 15 2019, 11:08 AM

Jan 11 2019

tejohnson added a comment to D54815: [ThinLTO] Add summary entries for index-based WPD.

Vtable funcs info should be in the initializers of vtable global variables right?

Jan 11 2019, 6:11 PM · Restricted Project
tejohnson added a comment to D54815: [ThinLTO] Add summary entries for index-based WPD.

A very high level meta question: can thinLink phase read symbol table from each module? If yes, why duplicate the symtab information into summary?

Jan 11 2019, 2:00 PM · Restricted Project
tejohnson updated the diff for D54815: [ThinLTO] Add summary entries for index-based WPD.

Update patch to reflect changes at head.

Jan 11 2019, 12:58 PM · Restricted Project
tejohnson added a reviewer for D54815: [ThinLTO] Add summary entries for index-based WPD: davidxl.
Jan 11 2019, 12:57 PM · Restricted Project
tejohnson added a comment to D54819: [SampleFDO] Skip profile reading when flatten profile is used in ThinLTO postlink phase.

I just stumbled on this patch - sorry for the late review! I thought this was already in. A few comments and a question below. Also, presumably this should be done for the old PM as well?

Jan 11 2019, 11:14 AM
tejohnson committed rC350949: [LTO] Add option to enable LTOUnit splitting, and disable unless needed.
[LTO] Add option to enable LTOUnit splitting, and disable unless needed
Jan 11 2019, 10:36 AM
tejohnson committed rL350949: [LTO] Add option to enable LTOUnit splitting, and disable unless needed.
[LTO] Add option to enable LTOUnit splitting, and disable unless needed
Jan 11 2019, 10:36 AM
tejohnson committed rL350948: [LTO] Record whether LTOUnit splitting is enabled in index.
[LTO] Record whether LTOUnit splitting is enabled in index
Jan 11 2019, 10:36 AM
tejohnson closed D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed.
Jan 11 2019, 10:36 AM