Page MenuHomePhabricator

tejohnson (Teresa Johnson)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 27 2015, 11:17 AM (281 w, 5 d)

Recent Activity

Yesterday

tejohnson updated the diff for D87970: [ThinLTO] Avoid temporaries when loading global decl attachment metadata.

Add test options that exercise code, fix exposed bug.

Sat, Sep 19, 4:11 PM · Restricted Project
tejohnson updated the summary of D87970: [ThinLTO] Avoid temporaries when loading global decl attachment metadata.
Sat, Sep 19, 4:11 PM · Restricted Project
tejohnson added a comment to D87970: [ThinLTO] Avoid temporaries when loading global decl attachment metadata.

I realized after intentionally suppressing the new method that there was no regression test that broke. It turns out that the lazy loading index is only emitted if there is a certain number of metadatas, which the tests that exercise this code apparently don't have. Therefore, I added an option to drop the threshold for one of the ThinLTO WPD tests. This exposed a bug: Because parseGlobalObjectAttachment now resolves forward references via the index, the index cursor has moved when it returns, and we weren't parsing all of the global decl attachment metadatas. Fixed by saving and restoring the current position around that call.

Sat, Sep 19, 4:11 PM · Restricted Project
tejohnson requested review of D87970: [ThinLTO] Avoid temporaries when loading global decl attachment metadata.
Sat, Sep 19, 10:30 AM · Restricted Project
tejohnson added a comment to D87966: [ThinLTO] Re-order modules for optimal multi-threaded processing.

Thanks for doing this!

Sat, Sep 19, 9:05 AM · Restricted Project

Fri, Sep 18

tejohnson added a comment to D87949: [ThinLTO] Option to bypass function importing..

Minor comments.

Fri, Sep 18, 4:35 PM · Restricted Project, Restricted Project
tejohnson accepted D87943: [clang] Remove profile available check for fsplit-machine-functions..

lgtm with a couple of minor suggestions

Fri, Sep 18, 2:22 PM · Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Rebase

Fri, Sep 18, 12:28 PM · Restricted Project
tejohnson added a comment to D87792: [sanitizer] Add facility to print the full StackDepot.

I lost connection to my Win box in the office. I will need Windows build
next week anyway. I will try to figure out why stderr is not captured there.
For now I relanded your patch and disabled the test on Windows.

Fri, Sep 18, 8:06 AM · Restricted Project

Thu, Sep 17

tejohnson added a comment to D87792: [sanitizer] Add facility to print the full StackDepot.

I had to revert due to 2 bot failures.

Thu, Sep 17, 9:25 PM · Restricted Project
tejohnson added a reverting change for rG2ffaa9a1732c: [sanitizer] Add facility to print the full StackDepot: rG6e475e1288e3: Revert "[sanitizer] Add facility to print the full StackDepot".
Thu, Sep 17, 9:20 PM
tejohnson committed rG6e475e1288e3: Revert "[sanitizer] Add facility to print the full StackDepot" (authored by tejohnson).
Revert "[sanitizer] Add facility to print the full StackDepot"
Thu, Sep 17, 9:20 PM
tejohnson added a reverting change for D87792: [sanitizer] Add facility to print the full StackDepot: rG6e475e1288e3: Revert "[sanitizer] Add facility to print the full StackDepot".
Thu, Sep 17, 9:20 PM · Restricted Project
tejohnson committed rG2ffaa9a1732c: [sanitizer] Add facility to print the full StackDepot (authored by tejohnson).
[sanitizer] Add facility to print the full StackDepot
Thu, Sep 17, 8:25 PM
tejohnson closed D87792: [sanitizer] Add facility to print the full StackDepot.
Thu, Sep 17, 8:25 PM · Restricted Project
tejohnson updated the diff for D87792: [sanitizer] Add facility to print the full StackDepot.

Implement test suggestion

Thu, Sep 17, 5:18 PM · Restricted Project
tejohnson added inline comments to D87792: [sanitizer] Add facility to print the full StackDepot.
Thu, Sep 17, 5:18 PM · Restricted Project
tejohnson updated the diff for D87792: [sanitizer] Add facility to print the full StackDepot.

Fix tmp file handling to work on all platforms

Thu, Sep 17, 8:03 AM · Restricted Project
tejohnson added a comment to D87792: [sanitizer] Add facility to print the full StackDepot.

I realized I had to make a change, PTAL.

Thu, Sep 17, 8:03 AM · Restricted Project

Wed, Sep 16

tejohnson added a comment to D86847: [Bitcode] Add BITCODE_SIZE_BLOCK_ID to encode the size of the bitcode.

I am a bit worry that linker might concatenate bitcode file with padding to achieve alignment requirement, etc. I guess you can create a termination block to mark the end but it is hard to seek the next start.

We can set the section alignment to 1 to avoid padding: (I rushed a bit, sorry: 6ae7b403c3e1aebcb825d3dd4777d3c1149d6d67)

I didn't see that. I don't really have concerns then.

@bartell mentioned something about a bitcode wrapper that does provide the length - how does this patch's strategy compare to that approach? Are they redundant? Should the wrapper be replaced with this patch's approach? (or should the bitcode wrapper approach be used instead of adding this patch?)

I guess that is an implementation choice. This approach prefers a simple linker implementation, while the bitcode wrapper requires linker know to treat this section differently and write bitcode wrapper into section.
@MaskRay Do you have any specific use case in mind for this? If you are tied to a linker like lld, it might be better just teach lld to treat this section differently so we don't need worry about concatenated bitcode file.

It is not tied to a linker like lld. I believe in most binary formats, if you specify an alignment of 1, their linkers will concatenate input sections in the output without padding. Actually, the section content of .llvmbc is entirely opaque to lld/GNU ld/gold. ("dumb linker, smart format" design).

If the bitcode wrapper requires the linker to understand its format, I'd vote against that solution. (It will not work with GNU ld/gold.)

Wed, Sep 16, 3:38 PM · Restricted Project
tejohnson added a comment to D85649: [AArch64] PAC/BTI code generation for LLVM generated functions.

The only question I have about module level attributes is what happens for LTO (which we use for Linux kernels used in Android)? Is there any conflict linking together objects from two different translation units with two different flags? Maybe this should fail hard; but it would be cool to alert developers at compile time, rather than have an awful bug to debug at runtime. Just food for thought; I don't know enough about LTO to know if it already has machinery to handle module level conflicts or not. Maybe @tejohnson or @pcc knows?

Wed, Sep 16, 2:45 PM · Restricted Project
tejohnson added a comment to D87120: [MemProf] Memory profiling runtime support.

I pulled out the sanitizer_common handling for printing the stack depot into another patch. I also removed the internal_getcpu handling as I couldn't reproduce why I needed it in the first place. Calling sched_getcpu seems to work fine without it.

Wed, Sep 16, 2:33 PM · Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Remove sanitizer common handling from this patch

Wed, Sep 16, 2:23 PM · Restricted Project
tejohnson requested review of D87792: [sanitizer] Add facility to print the full StackDepot.
Wed, Sep 16, 1:49 PM · Restricted Project

Tue, Sep 15

tejohnson accepted D87636: [ThinLTO] add post-thinlto-merge option to -lto-embed-bitcode.

LGTM, just needs a comment update as noted below and also an update to the patch title.

Tue, Sep 15, 3:41 PM · Restricted Project, Restricted Project
tejohnson added inline comments to D87636: [ThinLTO] add post-thinlto-merge option to -lto-embed-bitcode.
Tue, Sep 15, 3:06 PM · Restricted Project, Restricted Project
tejohnson retitled D87120: [MemProf] Memory profiling runtime support from [HeapProf] Heap profiling runtime support to [MemProf] Memory profiling runtime support.
Tue, Sep 15, 11:12 AM · Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Remove a few extraneous files from commit. I will be splitting out the
sanitizer_common changes into another patch shortly, and will update the
patch with a note afterwards when this is ready to review again.

Tue, Sep 15, 11:09 AM · Restricted Project
tejohnson added a comment to D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes).

Please go ahead and commit to the branch.

Tue, Sep 15, 8:54 AM · Restricted Project, Restricted Project

Mon, Sep 14

tejohnson committed rG226d80ebe20e: [MemProf] Rename HeapProfiler to MemProfiler for consistency (authored by tejohnson).
[MemProf] Rename HeapProfiler to MemProfiler for consistency
Mon, Sep 14, 1:16 PM
tejohnson closed D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency.
Mon, Sep 14, 1:16 PM · Restricted Project, Restricted Project
tejohnson added a comment to D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency.

LGTM (if the option is documented, the documentation part also needs to be updated).

Mon, Sep 14, 1:14 PM · Restricted Project, Restricted Project
tejohnson retitled D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency from [MemProf] Rename HeapProfiler to MemProfiler for consistency (NFC) to [MemProf] Rename HeapProfiler to MemProfiler for consistency.
Mon, Sep 14, 12:59 PM · Restricted Project, Restricted Project
tejohnson added a comment to D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency.

LG. (Nit: technically this is not NFC: it affects command line options and runtime function names (ABI changes).)

Mon, Sep 14, 12:59 PM · Restricted Project, Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Rename from heapprof to memprof.

Mon, Sep 14, 12:28 PM · Restricted Project
tejohnson requested review of D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency.
Mon, Sep 14, 11:00 AM · Restricted Project, Restricted Project

Sun, Sep 13

tejohnson accepted D86905: Flush bitcode incrementally for LTO output.

lgtm but please wait to see if @MaskRay or @evgeny777 have more comments

Sun, Sep 13, 8:02 AM · Restricted Project

Sat, Sep 12

tejohnson added a comment to D86905: Flush bitcode incrementally for LTO output.

There are a number of single statement if and for bodies in the patch that have braces but should not per llvm coding style.

Sat, Sep 12, 10:59 PM · Restricted Project
tejohnson added a comment to D86905: Flush bitcode incrementally for LTO output.

Please mention in the summary somewhere that this is only enabled for lld right now.

Sat, Sep 12, 10:03 AM · Restricted Project

Fri, Sep 11

tejohnson added inline comments to D86905: Flush bitcode incrementally for LTO output.
Fri, Sep 11, 7:06 PM · Restricted Project
tejohnson added a comment to D86905: Flush bitcode incrementally for LTO output.

Can you rebase this on top of D86913 so that it doesn't include those changes?

Fri, Sep 11, 6:43 PM · Restricted Project
tejohnson added inline comments to D87120: [MemProf] Memory profiling runtime support.
Fri, Sep 11, 12:53 PM · Restricted Project
tejohnson accepted D87477: [ThinLTO] Make -lto-embed-bitcode an enum.

LGTM

Fri, Sep 11, 12:18 PM · Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Manually fixed the formatting issues causing clang-format to get confused

Fri, Sep 11, 11:37 AM · Restricted Project
tejohnson added a comment to D87477: [ThinLTO] Make -lto-embed-bitcode an enum.

The change itself looks good to me, but I don't understand this comment from the summary:

Fri, Sep 11, 10:52 AM · Restricted Project

Thu, Sep 10

tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Fix incorrect clang-format

Thu, Sep 10, 10:47 PM · Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Remove offset field and use from_memalign

Thu, Sep 10, 10:39 PM · Restricted Project
tejohnson added inline comments to D87120: [MemProf] Memory profiling runtime support.
Thu, Sep 10, 10:29 PM · Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Address most of the comments

Thu, Sep 10, 3:51 PM · Restricted Project
tejohnson added a comment to D87120: [MemProf] Memory profiling runtime support.

The next patch update should address all of the comments except:

Thu, Sep 10, 3:34 PM · Restricted Project
tejohnson updated the diff for D87120: [MemProf] Memory profiling runtime support.

Rebase and update flag

Thu, Sep 10, 3:19 PM · Restricted Project
tejohnson accepted D87339: [NFC][ThinLTO] Let llvm::EmbedBitcodeInModule handle serialization..

LGTM with comment updates as suggested below.

Thu, Sep 10, 9:56 AM · Restricted Project

Wed, Sep 9

tejohnson added inline comments to D87359: [Asan] Don't use allocator Metadata.
Wed, Sep 9, 4:16 PM · Restricted Project
tejohnson added inline comments to D87359: [Asan] Don't use allocator Metadata.
Wed, Sep 9, 3:32 PM · Restricted Project
tejohnson added inline comments to D87120: [MemProf] Memory profiling runtime support.
Wed, Sep 9, 9:45 AM · Restricted Project

Tue, Sep 8

tejohnson added a comment to D87120: [MemProf] Memory profiling runtime support.

Responses to some of @MaskRay's comments below (if no explicit response then ack).

Tue, Sep 8, 8:51 PM · Restricted Project
tejohnson added a comment to D87339: [NFC][ThinLTO] Let llvm::EmbedBitcodeInModule handle serialization..

Does this rely on the following check in EmbedBitcodeInModule returning true?

Tue, Sep 8, 5:42 PM · Restricted Project
tejohnson accepted D87336: [NFC][ThinLTO] EmbedBitcodeSection doesn't need the Config.

LGTM

Tue, Sep 8, 5:13 PM · Restricted Project
tejohnson added inline comments to D87336: [NFC][ThinLTO] EmbedBitcodeSection doesn't need the Config.
Tue, Sep 8, 4:58 PM · Restricted Project
tejohnson added a comment to D87336: [NFC][ThinLTO] EmbedBitcodeSection doesn't need the Config.

Why not just make this one an NFC change to remove the unused Conf parameter. Then pass in CmdLineOpts in the patch where its use is added?

Tue, Sep 8, 4:51 PM · Restricted Project
tejohnson added a comment to D87120: [MemProf] Memory profiling runtime support.

Thanks for the comments @vitalybuka and @MaskRay. Starting with Vitaly's comments. Responses and follow up questions below.

Tue, Sep 8, 11:44 AM · Restricted Project
tejohnson added a comment to D81911: [ThinLTO] Work around getBaseObject returning null for alias-to-ifunc.

I have updated the diff to include the changes you suggested. However, as you said, for the test to have a chance of passing, D82745 is required. I will send out a mail to discuss the design issue on llvm-dev soon.

Tue, Sep 8, 8:03 AM · Restricted Project

Fri, Sep 4

tejohnson added a comment to D87120: [MemProf] Memory profiling runtime support.

Thanks for taking a look. A couple questions/comments below. I'll address the other ones.

Fri, Sep 4, 11:19 AM · Restricted Project
tejohnson added a comment to rG7ed8124d46f9: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

Committed fixes: https://github.com/llvm/llvm-project/commit/45c3560384814d04c9813e644efa8e2155ecae52

Fri, Sep 4, 9:01 AM
tejohnson committed rG45c356038481: [HeapProf] Address post-review comments in instrumentation code (authored by tejohnson).
[HeapProf] Address post-review comments in instrumentation code
Fri, Sep 4, 8:59 AM

Thu, Sep 3

tejohnson added inline comments to D87120: [MemProf] Memory profiling runtime support.
Thu, Sep 3, 5:37 PM · Restricted Project
tejohnson requested review of D87120: [MemProf] Memory profiling runtime support.
Thu, Sep 3, 5:26 PM · Restricted Project
tejohnson added a comment to D81911: [ThinLTO] Work around getBaseObject returning null for alias-to-ifunc.

The problem with what @DmitryPolukhin suggested (which he later acknowledged in an additional comment), is that getBaseObject() returns a GlobalObject, but GlobalIFunc does not inherit from GlobalObject, so that can't compile.

I get a sense that GlobalIFunc should derive directly from GlobalObject (since it should be a valid final target for aliases), instead of GlobalIndirectSymbol; but that's a pretty big, cross-cutting change which I'm not even sure 100% makes sense :\

Thu, Sep 3, 8:51 AM · Restricted Project

Wed, Sep 2

tejohnson accepted D86675: [ThinLTO] Fix a metadata lost issue with DICompileUnit import..

LGTM

Wed, Sep 2, 2:37 PM · Restricted Project
tejohnson added a comment to D81911: [ThinLTO] Work around getBaseObject returning null for alias-to-ifunc.

@tejohnson would simply not emitting an AliasSummary in computeAliasSummary when getBaseObject() returns nullptr be sufficient, then?

Wed, Sep 2, 12:01 PM · Restricted Project
tejohnson added a comment to D87001: [IRMover] Avoid materializing global value that belongs to not-yet-linked module.

I need to think through this some more, but a couple questions:

Wed, Sep 2, 10:40 AM · Restricted Project
tejohnson added a comment to D86675: [ThinLTO] Fix a metadata lost issue with DICompileUnit import..

AFAICT this looks ok, but adding @dblaikie as he is more familiar with the DI metadata. Couple minor comments below.

Wed, Sep 2, 9:57 AM · Restricted Project
tejohnson added a reviewer for D86675: [ThinLTO] Fix a metadata lost issue with DICompileUnit import.: dblaikie.
Wed, Sep 2, 9:53 AM · Restricted Project

Tue, Sep 1

tejohnson added a comment to rG7ed8124d46f9: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

I am fine with -fmemory-profiler.

Tue, Sep 1, 7:02 PM
tejohnson updated subscribers of rG7ed8124d46f9: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

Thanks for your comments, @etiotto. I'll address them in the next day or so. Alternate suggestion below for flag.

Tue, Sep 1, 5:12 PM
tejohnson added a comment to D86967: [PassManager] Move load/store motion pass after DSE in LTO pipeline..
Tue, Sep 1, 3:28 PM · Restricted Project
tejohnson committed rG40fed0048655: First commit on the release/11.x branch. (authored by hans).
First commit on the release/11.x branch.
Tue, Sep 1, 11:44 AM
tejohnson updated the diff for D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes).

Address comments

Tue, Sep 1, 11:43 AM · Restricted Project, Restricted Project
tejohnson requested review of D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes).
Tue, Sep 1, 9:47 AM · Restricted Project, Restricted Project
tejohnson added a comment to D84999: dynamic_cast optimization in LTO.

I'm ok with regular LTO implementation going in first, but it would be great to have a ThinLTO follow on implementation. Happy to provide pointers on that when you are ready.

Tue, Sep 1, 8:59 AM · Restricted Project

Thu, Aug 27

tejohnson accepted D86727: [SVE] Remove bad call to VectorType::getNumElements() from HeapProfiler.

Looks like this got fixed in the AddressSanitizer code after I cloned it over...

Thu, Aug 27, 11:24 AM · Restricted Project
tejohnson committed rG5b9d462b7d3c: [HeapProf] Fix bot failures from instrumentation pass (authored by tejohnson).
[HeapProf] Fix bot failures from instrumentation pass
Thu, Aug 27, 10:21 AM
tejohnson added a comment to D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

This really needs some docs changes.

Good point, I'll send a patch for that later today.

Thu, Aug 27, 9:02 AM · Restricted Project, Restricted Project
tejohnson added a comment to D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

This really needs some docs changes.

Thu, Aug 27, 8:59 AM · Restricted Project, Restricted Project
tejohnson committed rG7ed8124d46f9: [HeapProf] Clang and LLVM support for heap profiling instrumentation (authored by tejohnson).
[HeapProf] Clang and LLVM support for heap profiling instrumentation
Thu, Aug 27, 8:51 AM
tejohnson closed D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation.
Thu, Aug 27, 8:51 AM · Restricted Project, Restricted Project

Wed, Aug 26

tejohnson updated the diff for D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

Remove unnecessary check

Wed, Aug 26, 10:28 PM · Restricted Project, Restricted Project
tejohnson added inline comments to D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation.
Wed, Aug 26, 10:26 PM · Restricted Project, Restricted Project
tejohnson accepted D86645: Fix an overflow issue at BackpatchWord.

lgtm

Wed, Aug 26, 11:09 AM · Restricted Project
tejohnson accepted D84789: [LTO] Don't apply LTOPostLink module flag during writeMergedModule.

lgtm

Wed, Aug 26, 10:42 AM · Restricted Project

Tue, Aug 25

tejohnson updated the diff for D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

Address comments

Tue, Aug 25, 11:09 PM · Restricted Project, Restricted Project
tejohnson added a comment to D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation.

I think I've addressed all the comments.

Tue, Aug 25, 11:08 PM · Restricted Project, Restricted Project
tejohnson committed rG72bdb41a06a2: [Docs] Document --lto-whole-program-visibility (authored by tejohnson).
[Docs] Document --lto-whole-program-visibility
Tue, Aug 25, 7:48 PM
tejohnson closed D75655: [Docs] Document -lto-whole-program-visibility.
Tue, Aug 25, 7:47 PM · Restricted Project
tejohnson updated the diff for D75655: [Docs] Document -lto-whole-program-visibility.

Address comments

Tue, Aug 25, 5:33 PM · Restricted Project
tejohnson added a comment to D75655: [Docs] Document -lto-whole-program-visibility.
In D75655#2237474, @pcc wrote:

I think that the part of the description beginning "This can be used when..." is somewhat misleading since you could pretty much say the same thing about specifying -fvisibility=hidden -fwhole-program-vtables at compile time.

I think it would be better to focus on the specific capability that --lto-visibility-hidden gives you over just compiling with -fvisibility=hidden. Let's keep @evgeny777 's use case out of this because I still have concerns about it being dangerous. Can you simply say:

"This flag can be used to defer specifying whether classes have hidden LTO visibility until link time."

Then, in order to try to forbid inappropriate use of the symbols, you could add:

"Due to an implementation limitation, symbols associated with classes with hidden LTO visibility may still be exported from the binary when using this flag. It is unsafe to refer to these symbols, and their visibility may be relaxed to hidden in a future compiler release."

Tue, Aug 25, 5:33 PM · Restricted Project
tejohnson accepted D86374: [TargetLoweringObjectFileImpl] Make .llvmbc and .llvmcmd non-SHF_ALLOC.

lgtm

Tue, Aug 25, 1:22 PM · Restricted Project
tejohnson added a comment to D83845: [LTO/WPD] Remove special type test handling for -flto-visibility-public-std.

ping

Tue, Aug 25, 11:48 AM · Restricted Project
tejohnson added a comment to D75655: [Docs] Document -lto-whole-program-visibility.

@pcc ping. I'd like to get something in for LLVM 11 if at all possible. How about for now I make the last sentence something like "This is useful in situations where it is not safe to specify`-fvisibility=hidden` at compile time, for example when bitcode objects will be consumed by different LTO links that don't all have whole program visibility." ?

Tue, Aug 25, 11:47 AM · Restricted Project
tejohnson added inline comments to D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools.
Tue, Aug 25, 11:30 AM · Restricted Project, Restricted Project