Page MenuHomePhabricator

tejohnson (Teresa Johnson)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 27 2015, 11:17 AM (208 w, 2 d)

Recent Activity

Yesterday

tejohnson created D61042: Make post-link regular LTO pipelines match between new and old PM.
Tue, Apr 23, 3:06 PM · Restricted Project
tejohnson committed rG867bc3951bff: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM (authored by tejohnson).
[ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
Tue, Apr 23, 11:55 AM
tejohnson committed rC359025: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
[ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
Tue, Apr 23, 11:54 AM
tejohnson committed rL359025: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
[ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
Tue, Apr 23, 11:54 AM
tejohnson closed D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
Tue, Apr 23, 11:54 AM · Restricted Project, Restricted Project
tejohnson added inline comments to D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
Tue, Apr 23, 10:34 AM · Restricted Project, Restricted Project
tejohnson added inline comments to D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
Tue, Apr 23, 9:41 AM · Restricted Project, Restricted Project
tejohnson added inline comments to D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
Tue, Apr 23, 9:24 AM · Restricted Project, Restricted Project
tejohnson updated subscribers of D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
Tue, Apr 23, 9:24 AM · Restricted Project, Restricted Project
tejohnson created D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
Tue, Apr 23, 8:36 AM · Restricted Project, Restricted Project

Thu, Apr 18

tejohnson added a comment to D60162: [ThinLTO] Support TargetLibraryInfoImpl in the backend.

I wonder if we could add a module flag for the TLI, and then store that in the summary. Would it work for both implementations in that case?

Thu, Apr 18, 12:17 PM · Restricted Project
tejohnson added a comment to D60163: [ThinLTO] Handle -fno-builtin* options for distributed backends.

ping

Thu, Apr 18, 11:25 AM · Restricted Project
tejohnson added a comment to D60162: [ThinLTO] Support TargetLibraryInfoImpl in the backend.

ping

Thu, Apr 18, 11:25 AM · Restricted Project
tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

ping

Thu, Apr 18, 11:25 AM · Restricted Project

Wed, Apr 17

tejohnson added a comment to D60516: [LTO] Add plumbing to save stats during LTO on Darwin..

LGTM with a minor fix needed below.

Wed, Apr 17, 10:23 AM · Restricted Project, Restricted Project

Tue, Apr 16

tejohnson accepted D60421: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

LGTM
Sorry for the delay, just catching up after a vacation.

Tue, Apr 16, 8:59 AM · Restricted Project

Mon, Apr 8

tejohnson added a comment to D60421: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

What part of the patch caused the need to change the internalize action to do promote+internalize in one go?

Mon, Apr 8, 1:58 PM · Restricted Project
tejohnson accepted D60226: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

lgtm

Mon, Apr 8, 10:27 AM · Restricted Project

Fri, Apr 5

tejohnson added a comment to D60226: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

Just noticed something else looking through it again, question below.

Fri, Apr 5, 5:35 PM · Restricted Project
tejohnson added a comment to D60226: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

I have mostly small suggestions below. Overall, it looks pretty good and straightforward.

Fri, Apr 5, 11:33 AM · Restricted Project

Thu, Apr 4

tejohnson added a comment to D60226: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

Also do note, I did a quick benchmark on thin-link time for libLTO.dylib. thin-link was 2.6s before the change vs. 4.0s after the change. Building IRSymtab during thin-link does introduce non-negligible overhead. It might be possible to build them async in threads to reduce the overhead.

Thu, Apr 4, 2:21 PM · Restricted Project

Wed, Apr 3

tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

ping

Wed, Apr 3, 7:36 AM · Restricted Project

Tue, Apr 2

tejohnson created D60163: [ThinLTO] Handle -fno-builtin* options for distributed backends.
Tue, Apr 2, 4:08 PM · Restricted Project
tejohnson added a child revision for D60162: [ThinLTO] Support TargetLibraryInfoImpl in the backend: D60163: [ThinLTO] Handle -fno-builtin* options for distributed backends.
Tue, Apr 2, 4:08 PM · Restricted Project
tejohnson created D60162: [ThinLTO] Support TargetLibraryInfoImpl in the backend.
Tue, Apr 2, 4:08 PM · Restricted Project

Fri, Mar 29

tejohnson added inline comments to D59696: [CGP] Build the DominatorTree lazily.
Fri, Mar 29, 8:56 AM · Restricted Project
Herald added a project to D49434: Put "built-in" function definitions in global Used list, for LTO. (fix bug 34169): Restricted Project.

Hi Caroline, I was looking at this patch as I needed to reference it in another discussion, and realized there was no test committed with the patch. Looks like old review comments exist for a test/ThinLTO/X86/builtin-nostrip.ll on a prior version of the patch, but that seems to have disappeared on the version that got committed (probably just a mistake during the commit process since it was a new file). Can you grab that test off the prior diff and commit it now?

Fri, Mar 29, 8:24 AM · Restricted Project

Wed, Mar 27

tejohnson updated the diff for D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

Added a gold test so that we can test the native object case too.

Wed, Mar 27, 9:39 PM · Restricted Project
tejohnson updated the diff for D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

Address comments

Wed, Mar 27, 9:20 PM · Restricted Project
tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.
In D59709#1440303, @pcc wrote:

Does this has the correct behaviour in the case where the explicit instantiation is in a regular object file and the implicit instantiation is in bitcode?

Hmm, no. Nor will it work when the explicit instantiation is in a bitcode file without a summary. I can fix these cases by utilizing the VisibleOutsideSummary flag on the GlobalResolution to conservatively set CanAutoHide=false when that is set.

Wed, Mar 27, 9:19 PM · Restricted Project
tejohnson added inline comments to D59898: [LCG] Add aliased functions as LCG roots.
Wed, Mar 27, 1:54 PM · Restricted Project
tejohnson committed rGb7e213808c12: [CGP] Reset DT when optimizing select instructions (authored by tejohnson).
[CGP] Reset DT when optimizing select instructions
Wed, Mar 27, 11:44 AM
tejohnson committed rL357111: [CGP] Reset DT when optimizing select instructions.
[CGP] Reset DT when optimizing select instructions
Wed, Mar 27, 11:44 AM
tejohnson closed D59889: [CGP] Reset DT when optimizing select instructions.
Wed, Mar 27, 11:44 AM · Restricted Project
tejohnson created D59889: [CGP] Reset DT when optimizing select instructions.
Wed, Mar 27, 9:34 AM · Restricted Project
tejohnson added a comment to D59139: [CodeGenPrepare] Fix ModifiedDT flag in optimizeSelectInst.

If #1 is a one-liner that doesn't change any existing tests, that sounds like a good choice.

Wed, Mar 27, 9:03 AM · Restricted Project

Tue, Mar 26

tejohnson added a comment to D59139: [CodeGenPrepare] Fix ModifiedDT flag in optimizeSelectInst.

This change caused a compile time regression (that I referenced at the end of my summary in D59696). In this case, there are huge numbers of select instructions. After this change, since we now update the ModifiedDT correctly, the loop over the function in CodeGenPrepare::runOnFunction will break after each select is optimized, due to ModifiedDT being set. As mentioned in D59696, even after making the DT build lazy, there is still a large regression because of the number of times we re-walk the function.

Tue, Mar 26, 11:53 AM · Restricted Project

Mar 25 2019

tejohnson committed rG3bd4b5a925bd: [CGP] Build the DominatorTree lazily (authored by tejohnson).
[CGP] Build the DominatorTree lazily
Mar 25 2019, 11:38 AM
tejohnson committed rL356937: [CGP] Build the DominatorTree lazily.
[CGP] Build the DominatorTree lazily
Mar 25 2019, 11:37 AM
tejohnson closed D59696: [CGP] Build the DominatorTree lazily.
Mar 25 2019, 11:37 AM · Restricted Project

Mar 24 2019

tejohnson updated the diff for D59696: [CGP] Build the DominatorTree lazily.

Address comments, sync in NFC change r356857.

Mar 24 2019, 8:29 AM · Restricted Project
tejohnson added inline comments to D59696: [CGP] Build the DominatorTree lazily.
Mar 24 2019, 8:29 AM · Restricted Project
tejohnson committed rG4dc851964c03: [CGP] Make several static functions member functions (NFC) (authored by tejohnson).
[CGP] Make several static functions member functions (NFC)
Mar 24 2019, 8:20 AM
tejohnson committed rL356857: [CGP] Make several static functions member functions (NFC).
[CGP] Make several static functions member functions (NFC)
Mar 24 2019, 8:20 AM

Mar 22 2019

tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.
In D59709#1440303, @pcc wrote:

Does this has the correct behaviour in the case where the explicit instantiation is in a regular object file and the implicit instantiation is in bitcode?

Mar 22 2019, 9:10 PM · Restricted Project
tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

I am not quite sure what is the semantics for ELF linker. When thinLTO picks the linkonce_odr version and promote it to weak, it is not changing the visibility. It is a later linker optimization that auto hide the linkonce_odr symbol, correct?

It is in fact ThinLTO that is changing the visibility (D43130). See the change I made to FunctionImport.cpp.

Interesting, I made that change! But I somehow remembered I got pushed back because of ELF semantics and end up implementing that in ld64 instead. Let me dig up some context and get back to you.

Mar 22 2019, 2:13 PM · Restricted Project
tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

I am not quite sure what is the semantics for ELF linker. When thinLTO picks the linkonce_odr version and promote it to weak, it is not changing the visibility. It is a later linker optimization that auto hide the linkonce_odr symbol, correct?

Mar 22 2019, 1:42 PM · Restricted Project
tejohnson created D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.
Mar 22 2019, 11:33 AM · Restricted Project
tejohnson created D59696: [CGP] Build the DominatorTree lazily.
Mar 22 2019, 7:09 AM · Restricted Project

Mar 19 2019

tejohnson accepted D58391: [TailCallElim] Add tailcall elimination pass to LTO Pipelines.

Sorry missed your earlier update. LGTM. Thanks for doing the measurements!

Mar 19 2019, 11:04 AM · Restricted Project
tejohnson abandoned D59378: [InstCombine] Prevent icmp transform that can cause inf loop if part of min/max.

Committed icmp test in r356463.

Mar 19 2019, 8:43 AM · Restricted Project
tejohnson committed rGbda581b83126: [InstCombine] Add missing test for icmp transformation (NFC) (authored by tejohnson).
[InstCombine] Add missing test for icmp transformation (NFC)
Mar 19 2019, 8:43 AM
tejohnson committed rL356463: [InstCombine] Add missing test for icmp transformation (NFC).
[InstCombine] Add missing test for icmp transformation (NFC)
Mar 19 2019, 8:42 AM

Mar 18 2019

tejohnson added a comment to D59378: [InstCombine] Prevent icmp transform that can cause inf loop if part of min/max.

@spatel I've created D59506 for the InstSimplify change.

Thanks! I think we can safely abandon this patch...until we hit the next infinite loop (although that extra regression test in this patch should still be committed).

Mar 18 2019, 7:40 PM · Restricted Project
tejohnson added a comment to D59378: [InstCombine] Prevent icmp transform that can cause inf loop if part of min/max.

I'd prefer that we IR simplify our way out of this infinite loop instead of looking the other way though. Ie, can we get this in instsimplify using a ConstantRange?

That should be relatively simple to do, we just need to support constant range calculation for min/max flavor selects in computeConstantRange().

Mar 18 2019, 7:48 AM · Restricted Project

Mar 15 2019

tejohnson added inline comments to D54815: [ThinLTO] Add summary entries for index-based WPD.
Mar 15 2019, 10:40 AM · Restricted Project
tejohnson committed rG70ec64cb7235: [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee (authored by tejohnson).
[ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee
Mar 15 2019, 8:13 AM
tejohnson committed rL356268: [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee.
[ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee
Mar 15 2019, 8:10 AM
tejohnson closed D57470: [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee.
Mar 15 2019, 8:10 AM · Restricted Project
tejohnson retitled D57470: [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee from [ThinLTO] Restructor AliasSummary to contain ValueInfo of Aliasee to [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee.
Mar 15 2019, 8:10 AM · Restricted Project
tejohnson updated the diff for D57470: [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee.

Rebase

Mar 15 2019, 8:10 AM · Restricted Project

Mar 14 2019

tejohnson created D59378: [InstCombine] Prevent icmp transform that can cause inf loop if part of min/max.
Mar 14 2019, 11:42 AM · Restricted Project

Mar 13 2019

tejohnson accepted D59304: Fix invocation of Gold plugin with LTO after r355331.

lgtm

Mar 13 2019, 9:35 AM · Restricted Project, Restricted Project

Mar 12 2019

tejohnson committed rG4ab0a9f0a4e7: [SCEV] Use depth limit for trunc analysis (authored by tejohnson).
[SCEV] Use depth limit for trunc analysis
Mar 12 2019, 11:28 AM
tejohnson committed rL355949: [SCEV] Use depth limit for trunc analysis.
[SCEV] Use depth limit for trunc analysis
Mar 12 2019, 11:27 AM
tejohnson closed D58994: [SCEV] Use depth limit for trunc analysis.
Mar 12 2019, 11:27 AM · Restricted Project
tejohnson updated the diff for D58994: [SCEV] Use depth limit for trunc analysis.

Implement suggestion

Mar 12 2019, 11:01 AM · Restricted Project
tejohnson added a comment to D55153: [ThinLTO] Implement index-based WPD.

Ping

Mar 12 2019, 6:50 AM · Restricted Project
tejohnson added a reviewer for D55153: [ThinLTO] Implement index-based WPD: davidxl.
Mar 12 2019, 6:50 AM · Restricted Project
tejohnson added a reviewer for D57470: [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee: evgeny777.
Mar 12 2019, 6:49 AM · Restricted Project
tejohnson added a comment to D54815: [ThinLTO] Add summary entries for index-based WPD.

Ping

Mar 12 2019, 6:49 AM · Restricted Project
tejohnson added a comment to D57470: [ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee.

ping

Mar 12 2019, 6:49 AM · Restricted Project

Mar 11 2019

tejohnson added a comment to D58994: [SCEV] Use depth limit for trunc analysis.

ping

Mar 11 2019, 8:57 PM · Restricted Project

Mar 8 2019

tejohnson accepted D59139: [CodeGenPrepare] Fix ModifiedDT flag in optimizeSelectInst.

LGTM

Mar 8 2019, 10:43 AM · Restricted Project
tejohnson added a comment to D59139: [CodeGenPrepare] Fix ModifiedDT flag in optimizeSelectInst.

Just got a report of an internal test failure from my commit, let me see if your patch fixes it.

Mar 8 2019, 10:32 AM · Restricted Project
tejohnson added a comment to D59139: [CodeGenPrepare] Fix ModifiedDT flag in optimizeSelectInst.

Just got a report of an internal test failure from my commit, let me see if your patch fixes it.

Mar 8 2019, 10:25 AM · Restricted Project
tejohnson added a comment to D59139: [CodeGenPrepare] Fix ModifiedDT flag in optimizeSelectInst.

Thanks for the fix. A couple comments below.

Mar 8 2019, 10:03 AM · Restricted Project

Mar 6 2019

tejohnson committed rGb1daf0aef67d: [CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC) (authored by tejohnson).
[CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC)
Mar 6 2019, 6:57 AM
tejohnson committed rL355512: [CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC).
[CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC)
Mar 6 2019, 6:56 AM
tejohnson closed D58995: [CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC).
Mar 6 2019, 6:56 AM · Restricted Project
tejohnson updated the summary of D58995: [CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC).
Mar 6 2019, 6:29 AM · Restricted Project
tejohnson added a comment to D58995: [CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC).

LGTM
Sorry I didn't see this problem, and thanks for fixing it!
If you have any perf data that you can share, it would be good to include that here or in the commit message. I'm curious if this change recovers all of the compile-time perf or if walking the icmp users is also expensive (although I'm not sure how to make that any faster).

Mar 6 2019, 6:27 AM · Restricted Project

Mar 5 2019

tejohnson created D58995: [CGP] Avoid repeatedly building DominatorTree causing long compile-time (NFC).
Mar 5 2019, 1:20 PM · Restricted Project
tejohnson created D58994: [SCEV] Use depth limit for trunc analysis.
Mar 5 2019, 12:53 PM · Restricted Project

Feb 24 2019

tejohnson added a comment to D58391: [TailCallElim] Add tailcall elimination pass to LTO Pipelines.

What's the general state of the LTO pipeline at this point? PassManagerBuilder::addLTOOptimizationPasses is adding passes in a really weird order (in particular, the placement of the inliner is really strange). Has anyone looked at it recently? Would it be worth killing it off in favor of the ThinLTO pipeline just so we don't have to maintain it?

Feb 24 2019, 8:52 AM · Restricted Project

Feb 16 2019

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.

Feb 16 2019, 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.

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

Needs a test.

Feb 16 2019, 7:48 AM · Restricted Project

Feb 14 2019

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

LGTM

Feb 14 2019, 6:51 PM · Restricted Project
tejohnson added inline comments to D58258: [HotColdSplit] Schedule splitting late to fix perf regression.
Feb 14 2019, 4:56 PM · Restricted Project
tejohnson added inline comments to D58258: [HotColdSplit] Schedule splitting late to fix perf regression.
Feb 14 2019, 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.

Feb 14 2019, 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
Feb 14 2019, 1:23 PM
tejohnson committed rL354062: [ThinLTO] Detect partially split modules during the thin link.
[ThinLTO] Detect partially split modules during the thin link
Feb 14 2019, 1:23 PM
tejohnson closed D57561: [ThinLTO] Detect partially split modules during the thin link.
Feb 14 2019, 1:23 PM · Restricted Project
tejohnson updated the diff for D57561: [ThinLTO] Detect partially split modules during the thin link.

Address comment

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

ping

Feb 14 2019, 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.

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