tejohnson (Teresa Johnson)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

tejohnson updated the diff for D47905: [ThinLTO] Parse module summary index from assembly.

Address comments, rebase on top of D47844 and D47842.

Wed, Jun 20, 2:55 PM
tejohnson added a dependent revision for D47844: [ThinLTO] Compute GUID directly from GV when building per-module index: D47905: [ThinLTO] Parse module summary index from assembly.
Wed, Jun 20, 2:55 PM
tejohnson added a dependent revision for D47842: [ThinLTO] Add string saver onto index for value names: D47905: [ThinLTO] Parse module summary index from assembly.
Wed, Jun 20, 2:55 PM
tejohnson added a comment to D47905: [ThinLTO] Parse module summary index from assembly.

I've address the comments except for adding a new test, which I will work on and that will probably get added tomorrow. I'm about to upload a new patch which is properly stacked on top of the 2 patches I had sent separately.

Wed, Jun 20, 2:54 PM
tejohnson updated the diff for D47842: [ThinLTO] Add string saver onto index for value names.

Fixed patch and also rebased to HEAD

Wed, Jun 20, 1:45 PM
tejohnson added a comment to D47842: [ThinLTO] Add string saver onto index for value names.

My patch update accidentally pulled in a separate patch, let me fix that now...

Wed, Jun 20, 1:33 PM
tejohnson added inline comments to D47842: [ThinLTO] Add string saver onto index for value names.
Wed, Jun 20, 1:08 PM
tejohnson updated the diff for D47842: [ThinLTO] Add string saver onto index for value names.

Address comment.

Wed, Jun 20, 1:08 PM
tejohnson added a comment to D47905: [ThinLTO] Parse module summary index from assembly.

I just have a couple of additional (minor) comments on the code, although I admit it was hard to be thorough as a it's a monolithic patch. I might have preferred if this were committed incrementally, adding support for 1-2 constructs at a time. Because the lexing/parsing code is fairly mechanical, I think splitting it up now would be quite likely to introduce errors so I'm not going to ask for that.

In terms of testing, I don't see any positive assembly parsing lit tests (aside from implicit ones when round-tripping bitcode through llvm-as), and I think that would be valuable. I'm not sure if it's pragmatic to write targeted tests for each construct, or if a couple of monolithic tests would be better. What do you think?

Wed, Jun 20, 12:44 PM
tejohnson accepted D34156: [LTO] Enable module summary emission by default for regular LTO.

LGTM with following suggestions.

Wed, Jun 20, 10:22 AM
tejohnson added a comment to D47844: [ThinLTO] Compute GUID directly from GV when building per-module index.

ping

Wed, Jun 20, 8:38 AM
tejohnson added a comment to D47842: [ThinLTO] Add string saver onto index for value names.

ping

Wed, Jun 20, 8:38 AM
tejohnson accepted D47898: IRMover: Account for matching types present across modules.

LGTM - this seems consistent with the fix made in D26212, we're just hitting the issue at a different point during type mapping.

Wed, Jun 20, 8:13 AM
tejohnson added a comment to D47969: [Verifier] Check that ValueAsMetadata belongs to the module.

IIRC, sharing uniqued metadata across modules should only happen as a temporary state. It's a trick for when loading modules that will be merged together, to avoid unnecessarily creating multiple copies of the graphs.

I feel like the verifier should reject this state, unless it's specifically being called in a context where modules-to-be-merged are still being loaded.

The crash noted in the test occurs during cross-module importing, does that count as merging? Both of the modules are independently codegened, not merged together, but there is importing. To me it seems like cross-module references are a temporary state while merging for monolithic LTO, but not a temporary state under ThinLTO.

Wed, Jun 20, 6:34 AM
tejohnson accepted D48350: [LIT] Enable testing of LLVM gold plugin on Mac OS X.
Wed, Jun 20, 5:55 AM
tejohnson added a comment to D48350: [LIT] Enable testing of LLVM gold plugin on Mac OS X.

LGTM

Wed, Jun 20, 5:55 AM

Tue, Jun 19

tejohnson added a comment to D47905: [ThinLTO] Parse module summary index from assembly.

Ping

Tue, Jun 19, 6:39 AM

Mon, Jun 18

tejohnson removed reviewers for D41222: Handle previously ASAN-instrumented IR gracefully when ASAN re-invoked: vsk, vitalybuka, pcc.
Mon, Jun 18, 6:02 AM

Thu, Jun 7

tejohnson added a comment to D47905: [ThinLTO] Parse module summary index from assembly.

FYI I will be on vacation tomorrow morning through next week, so my responses to any comments will likely come June 18 or later. I wanted to get this out before I left so that hopefully I get some comments by the time I return. =)

Thu, Jun 7, 1:36 PM
tejohnson created D47906: [ThinLTO] Add testing of summary index parsing to a couple CFI tests.
Thu, Jun 7, 1:35 PM
tejohnson added a dependent revision for D47905: [ThinLTO] Parse module summary index from assembly: D47906: [ThinLTO] Add testing of summary index parsing to a couple CFI tests.
Thu, Jun 7, 1:34 PM
tejohnson created D47905: [ThinLTO] Parse module summary index from assembly.
Thu, Jun 7, 1:33 PM

Wed, Jun 6

tejohnson committed rLLD334141: [ThinLTO/lld] Document constant bool ModuleSummaryIndex parameter (NFC).
[ThinLTO/lld] Document constant bool ModuleSummaryIndex parameter (NFC)
Wed, Jun 6, 3:27 PM
tejohnson committed rL334141: [ThinLTO/lld] Document constant bool ModuleSummaryIndex parameter (NFC).
[ThinLTO/lld] Document constant bool ModuleSummaryIndex parameter (NFC)
Wed, Jun 6, 3:26 PM
tejohnson committed rL334140: [ThinLTO] Rename index IsAnalysis flag to HaveGVs (NFC).
[ThinLTO] Rename index IsAnalysis flag to HaveGVs (NFC)
Wed, Jun 6, 3:26 PM
tejohnson created D47844: [ThinLTO] Compute GUID directly from GV when building per-module index.
Wed, Jun 6, 1:29 PM
tejohnson added a comment to D47842: [ThinLTO] Add string saver onto index for value names.

I forgot to add, StringSaver is not copyable (due to BumpPtrAllocator reference), therefore this required changes to the Index value on the ModuleSummaryIndexWrapperPass to hold a unique_ptr instead of Optional, so that it could be initialized without a copy.

Wed, Jun 6, 1:26 PM
tejohnson created D47842: [ThinLTO] Add string saver onto index for value names.
Wed, Jun 6, 12:48 PM
tejohnson committed rL334111: [ThinLTO] Make ValueInfo operator!= consistent with operator== (NFC).
[ThinLTO] Make ValueInfo operator!= consistent with operator== (NFC)
Wed, Jun 6, 11:36 AM
tejohnson added a comment to D46584: [Evaluator] Improve evaluation of call instruction.

Sorry I missed this one.

Wed, Jun 6, 7:01 AM

Mon, Jun 4

tejohnson committed rC333966: [ThinLTO] Add testing of new summary index format to a couple CFI tests.
[ThinLTO] Add testing of new summary index format to a couple CFI tests
Mon, Jun 4, 4:09 PM
tejohnson committed rL333966: [ThinLTO] Add testing of new summary index format to a couple CFI tests.
[ThinLTO] Add testing of new summary index format to a couple CFI tests
Mon, Jun 4, 4:09 PM
tejohnson closed D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests.
Mon, Jun 4, 4:09 PM
tejohnson added a comment to D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests.

ping

Mon, Jun 4, 2:21 PM
tejohnson committed rL333942: Fix for llvm-dis/llvm-bcanalyzer overflows.
Fix for llvm-dis/llvm-bcanalyzer overflows
Mon, Jun 4, 12:24 PM
tejohnson closed D47731: Fix for llvm-dis/llvm-bcanalyzer overflows.
Mon, Jun 4, 12:24 PM
tejohnson created D47731: Fix for llvm-dis/llvm-bcanalyzer overflows.
Mon, Jun 4, 10:56 AM

Thu, May 31

tejohnson accepted D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index..

LGTM thanks!

Thu, May 31, 11:25 AM

Sat, May 26

tejohnson updated the diff for D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests.

Update tests so they won't get bot failures (don't try to match path, module
hash).

Sat, May 26, 4:11 PM

Fri, May 25

tejohnson committed rL333338: [ThinLTO] Fix a few more test match issues.
[ThinLTO] Fix a few more test match issues
Fri, May 25, 8:54 PM
tejohnson committed rL333337: [ThinLTO] Fix another bot failure due to test mismatch.
[ThinLTO] Fix another bot failure due to test mismatch
Fri, May 25, 8:24 PM
tejohnson committed rL333336: [ThinLTO] Fix bot failures from r333335.
[ThinLTO] Fix bot failures from r333335
Fri, May 25, 7:57 PM
tejohnson committed rL333335: [ThinLTO] Print module summary index to assembly.
[ThinLTO] Print module summary index to assembly
Fri, May 25, 7:38 PM
tejohnson closed D46699: [ThinLTO] Print module summary index to assembly.
Fri, May 25, 7:38 PM
tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.

Comments addressed.

Fri, May 25, 4:09 PM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Address comment, add new tests.

Fri, May 25, 4:08 PM
tejohnson added inline comments to D46699: [ThinLTO] Print module summary index to assembly.
Fri, May 25, 3:42 PM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Address comments and sync with HEAD.

Fri, May 25, 9:20 AM
tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.

Uploading new version momentarily with the following changes:

  • Merged in recommended linkage name printer NFC changes (r333281).
  • Changed SkipModuleSummaryEntry so that it would handle multi-line summary entries (by skipping the set of nested parentheses).
  • Undid changes to SkipLineComment due to the above change.
  • Added a test to ensure the round-tripping of summary entries through llvm-as (in test/Bitcode/thinlto-function-summary-callgraph-profile-summary.ll).
  • Synced with HEAD
Fri, May 25, 9:20 AM
tejohnson committed rL333281: [NFC] Restructure linkage name printing in AsmWriter.
[NFC] Restructure linkage name printing in AsmWriter
Fri, May 25, 8:19 AM

Thu, May 24

tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.
In D46699#1111653, @pcc wrote:

Although the support in this patch to skip the summary assembly will not handle this, as it only skips a line at a time and won't understand the subsequent lines which don't start with the summary ID.

You could always count parens and stop when you reach 0. That's probably what the end state for skipping should look like as well.

Thu, May 24, 3:45 PM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Address LangRef comment

Thu, May 24, 1:53 PM
tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.
In D46699#1111535, @pcc wrote:

This is somewhat of a bikeshed comment but I'm not really a big fan of putting everything on one line. (I know we do that for debug metadata too, but really I'd be in favour of changing that as well.) I'm wondering whether something like this would be easier to read:

^9 = gv: ( ; guid = 17381606045411660303
  name: "hot_function",
  summaries: (
    function: (
      module: ^0,
      flags: (
        linkage: external,
        notEligibleToImport: 0,
        live: 0,
        dsoLocal: 0
      ),
      insts: 16,
      calls: (
        (callee: ^5, hotness: hot),
        (callee: ^6, hotness: cold),
        (callee: ^4, hotness: hot),
        (callee: ^7, hotness: cold),
        (callee: ^10, hotness: none),
        (callee: ^3, hotness: hot),
        (callee: ^2, hotness: none),
        (callee: ^8, hotness: none),
        (callee: ^1, hotness: critical)
      )
    )
  )
)
Thu, May 24, 1:53 PM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Manually merge r333212 comment fix.

Thu, May 24, 10:48 AM
tejohnson committed rL333212: [ThinLTO/CFI] Minor comment clarification.
[ThinLTO/CFI] Minor comment clarification
Thu, May 24, 10:46 AM
tejohnson closed D47338: [ThinLTO/CFI] Minor comment clarification.
Thu, May 24, 10:46 AM
tejohnson created D47338: [ThinLTO/CFI] Minor comment clarification.
Thu, May 24, 10:43 AM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Address comments.

Thu, May 24, 10:31 AM
tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.

Thanks for the comments! I think I have addressed them all.

Thu, May 24, 10:30 AM

May 21 2018

tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.

Ping

May 21 2018, 7:49 PM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Add LangRef documentation.

May 21 2018, 7:49 PM

May 16 2018

tejohnson added inline comments to D46699: [ThinLTO] Print module summary index to assembly.
May 16 2018, 8:21 AM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Merge in minor fixes committed separately.

May 16 2018, 8:06 AM
tejohnson committed rL332476: [ThinLTO] Make llvm-lto module ID numbering consistent with linkers.
[ThinLTO] Make llvm-lto module ID numbering consistent with linkers
May 16 2018, 8:04 AM
tejohnson committed rL332475: [ThinLTO] Add const qualifier to a couple of flag getter methods.
[ThinLTO] Add const qualifier to a couple of flag getter methods
May 16 2018, 7:59 AM
tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.

I haven't had time to give this a full review, but skimming this I noticed that there's no update to llvm/docs/LangRef.rst. Can you document the syntax and semantics there?

May 16 2018, 6:52 AM

May 15 2018

tejohnson added a comment to D46699: [ThinLTO] Print module summary index to assembly.

Ping on review. I don't expect more changes from my side at this point.

May 15 2018, 8:24 AM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

Update tests for previous change.

May 15 2018, 8:22 AM
tejohnson updated the diff for D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests.

Update tests for changes to D46699

May 15 2018, 8:22 AM
tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

A few changes motivated when writing the AsmParser support:

  • Use "external" instead of "extern" as linkage, so that existing

linkage type parser can be utilized

  • Quote string values (e.g. names and paths)
  • Emit only one of edge hotness and relative branch freq (to be

consistent with bitcode writer), and only write them if they have a
specified value.

May 15 2018, 7:41 AM

May 14 2018

tejohnson added a comment to D43521: [ThinLTO] Compute synthetic function entry count.

Sorry for the delay.

May 14 2018, 7:15 AM

May 10 2018

tejohnson updated the diff for D46699: [ThinLTO] Print module summary index to assembly.

A little bit of minor comment cleanup

May 10 2018, 8:56 AM
tejohnson added a dependent revision for D46699: [ThinLTO] Print module summary index to assembly: D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests.
May 10 2018, 8:40 AM
tejohnson created D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests.
May 10 2018, 8:40 AM
tejohnson created D46699: [ThinLTO] Print module summary index to assembly.
May 10 2018, 8:40 AM

May 7 2018

tejohnson committed rL331709: [NewPM] Emit inliner NoDefinition missed optimization remark.
[NewPM] Emit inliner NoDefinition missed optimization remark
May 7 2018, 6:49 PM
tejohnson closed D46526: [NewPM] Emit inliner NoDefinition missed optimization remark.
May 7 2018, 6:49 PM
tejohnson added a comment to D46480: Update ThinLTO Indexing logic.

This LGTM, but wait until pcc or ruiu accept it.

May 7 2018, 4:16 PM
tejohnson added inline comments to D46526: [NewPM] Emit inliner NoDefinition missed optimization remark.
May 7 2018, 11:59 AM
tejohnson created D46526: [NewPM] Emit inliner NoDefinition missed optimization remark.
May 7 2018, 7:47 AM

May 5 2018

tejohnson committed rL331597: Add -target to address errors in test from r331592.
Add -target to address errors in test from r331592
May 5 2018, 9:41 AM
tejohnson committed rC331597: Add -target to address errors in test from r331592.
Add -target to address errors in test from r331592
May 5 2018, 9:41 AM
tejohnson committed rL331596: Skip part of test added in r331592 to help debug bot failures.
Skip part of test added in r331592 to help debug bot failures
May 5 2018, 8:58 AM
tejohnson committed rC331596: Skip part of test added in r331592 to help debug bot failures.
Skip part of test added in r331592 to help debug bot failures
May 5 2018, 8:58 AM
tejohnson committed rL331593: Add required target to address bot failures from r331592.
Add required target to address bot failures from r331592
May 5 2018, 8:18 AM
tejohnson committed rC331593: Add required target to address bot failures from r331592.
Add required target to address bot failures from r331592
May 5 2018, 8:18 AM
tejohnson committed rC331592: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.
[ThinLTO] Support opt remarks options with distributed ThinLTO backends
May 5 2018, 7:43 AM
tejohnson committed rL331592: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.
[ThinLTO] Support opt remarks options with distributed ThinLTO backends
May 5 2018, 7:43 AM
tejohnson committed rL331591: [LTO] Handle Task=-1 passed to addSaveTemps.
[LTO] Handle Task=-1 passed to addSaveTemps
May 5 2018, 7:43 AM
tejohnson closed D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.
May 5 2018, 7:43 AM
tejohnson closed D46488: [LTO] Handle Task=-1 passed to addSaveTemps.
May 5 2018, 7:43 AM

May 4 2018

tejohnson added inline comments to D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.
May 4 2018, 8:19 PM
tejohnson updated the diff for D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.

Update test for change to pass -1 as the Task ID for distributed backends,
and to reflect companion llvm change (D46488).

May 4 2018, 8:15 PM
tejohnson created D46488: [LTO] Handle Task=-1 passed to addSaveTemps.
May 4 2018, 8:14 PM
tejohnson committed rL331569: [LTO] Allow pass remarks with hotness to be set when emitting to stderr.
[LTO] Allow pass remarks with hotness to be set when emitting to stderr
May 4 2018, 5:05 PM
tejohnson closed D46387: [LTO] Allow pass remarks with hotness to be set when emitting to stderr.
May 4 2018, 5:05 PM
tejohnson added a comment to D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.

Fixed the output file as suggested. Also note that this test will fail and can't go in until after D46387

May 4 2018, 4:32 PM
tejohnson updated the diff for D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.

Address comments

May 4 2018, 4:32 PM
tejohnson added inline comments to D46480: Update ThinLTO Indexing logic.
May 4 2018, 3:42 PM
tejohnson created D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends.
May 4 2018, 1:50 PM