Page MenuHomePhabricator

tejohnson (Teresa Johnson)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 27 2015, 11:17 AM (216 w, 2 h)

Recent Activity

Fri, Jun 14

tejohnson added inline comments to D59673: [Clang] Harmonize Split DWARF options with llc.
Fri, Jun 14, 9:54 AM · Restricted Project, Restricted Project
tejohnson added a comment to D59673: [Clang] Harmonize Split DWARF options with llc.

lgtm for the LTO bits. Suggestion below for comment.

Fri, Jun 14, 7:09 AM · Restricted Project, Restricted Project

Wed, Jun 12

tejohnson added inline comments to D59673: [Clang] Harmonize Split DWARF options with llc.
Wed, Jun 12, 12:54 PM · Restricted Project, Restricted Project
tejohnson accepted D63078: [ThinLTO][Bitcode] Add 'entrycount' to FS_COMBINED_PROFILE. NFC.

Interesting - does this mean we just encoded it with a VBR 4 (what numrefs was to be encoded with), and everything else just got shifted down in the encoding (so immutablerefcnt got encoded as the first VBR 8 in the array at the end)? Unfortunate that there isn't a way to test for this.

Wed, Jun 12, 12:06 PM · Restricted Project
tejohnson added inline comments to D59673: [Clang] Harmonize Split DWARF options with llc.
Wed, Jun 12, 6:39 AM · Restricted Project, Restricted Project

Wed, May 29

tejohnson committed rG5b2088d1fac1: [ThinLTO] Use original alias visibility when importing (authored by tejohnson).
[ThinLTO] Use original alias visibility when importing
Wed, May 29, 9:50 AM
tejohnson committed rL361989: [ThinLTO] Use original alias visibility when importing.
[ThinLTO] Use original alias visibility when importing
Wed, May 29, 9:50 AM
tejohnson closed D62535: [ThinLTO] Use original alias visibility when importing.
Wed, May 29, 9:50 AM · Restricted Project

Tue, May 28

tejohnson created D62535: [ThinLTO] Use original alias visibility when importing.
Tue, May 28, 11:04 AM · Restricted Project

Thu, May 23

tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

AFAIU here is a coarse plan of what needs to happen

  1. Add a no-builtin clang function attribute that has the same semantic as the no-builtin cmd line argument
  2. Propagate it to the IR.
    • In the light of recent discussions and as @theraven suggested it seems more appropriate to encode them as individual IR attributes (e.g. "no-builtin-memcpy", "no-builtin-sqrt", etc..)
  3. Propagate/merge the no-builtin IR attribute for LTO by "updating AttributeFuncs::areInlineCompatible and/or AttributeFuncs::mergeAttributesForInlining and adding a new MergeRule in include/llvm/IR/Attributes.td and writing a function like adjustCallerStackProbeSize."
Thu, May 23, 10:00 PM · Restricted Project, Restricted Project
tejohnson abandoned D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

As per D61634, we are going to use function attributes instead.

Thu, May 23, 10:00 PM · Restricted Project, Restricted Project

May 17 2019

tejohnson added a comment to rL309966: Disable loop peeling during full unrolling pass..

I may not understand the implementation completely. So could you explain a little bit how this patch only affect peeling for full unrolling? It seems to me you disable peeling for all the unrolling (full, partial or run-time). i.e, where is the code that you distinguish peeling according to unrolling type? I cannot find that logic. Thanks a lot!

May 17 2019, 10:53 AM

May 15 2019

tejohnson added a comment to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

There is discussion of using function attributes to control this instead, see https://reviews.llvm.org/D61634.
I'll hold off on making the planned changes here until the overall approach is decided.

May 15 2019, 8:02 AM · Restricted Project, Restricted Project
tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

Using function level attributes instead of module flags does provide finer grained control and avoids the conservativeness when merging IR for LTO. The downsides I see, mostly just in terms of the engineering effort to get this to work, are:

  • need to prevent inlining with different attributes

IIUC this is needed regardless of the proposed change. Correct?

May 15 2019, 8:00 AM · Restricted Project, Restricted Project

May 14 2019

tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

I have a related patch that turns -fno-builtin* options into module flags

Do you have any opinion on representing -fno-builtin using a module flag vs. a function attribute in IR? It seems generally more flexible and easier to reason about a function attribute from my perspective. But I might be missing something about the semantics of -fno-builtin that would make that representation awkward. Or I guess it might just be more work to implement, given we have some IPO passes that use TargetLibraryInfo?

I think that a function attribute would be better. We generally use these flags only in the context of certain translation units, and when we use LTO, it would be sad if we had to take the most-conservative settings across the entire application. When we insert new function call to a standard library, we always do it in the context of some function. We'd probably need to block inlining in some cases, but that's better than a global conservative setting.

May 14 2019, 3:53 PM · Restricted Project, Restricted Project
tejohnson added inline comments to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.
May 14 2019, 1:36 PM · Restricted Project, Restricted Project
tejohnson added inline comments to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.
May 14 2019, 1:33 PM · Restricted Project, Restricted Project
tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

My main blocker is that I want to make sure we're moving in the right direction: towards LLVM IR with clear semantics, towards straightforward rules for writing freestanding C code, and towards solutions which behave appropriately for all targets. There's clearly a problem here that's worth solving, but I want to make sure we're adding small features that interact with existing features in an obvious way. The patch as written is adding a new attribute that changes the semantics of C and LLVM IR in a subtle way that interacts with existing optimizations and lowering in ways that are complex and hard to understand.

This makes a lot of sense, I'm totally on board to reduce entropy and confusion along the way.

I don't want to mix general restrictions on calling C library functions, with restrictions that apply specifically to memcpy: clang generally works on the assumption that a "memcpy" symbol is available in freestanding environments, and we don't really want to change that.

With -fno-builtin, specifically, we currently apply the restriction that optimizations will not introduce memcpy calls that would not exist with optimization disabled. This is sort of artificial, and and maybe a bit confusing, but it seems to work well enough in practice. gcc does something similar.

I don't really want to add an attribute that has a different meaning from -fno-builtin. An attribute that has the same meaning is a lot less problematic: it reduces potential confusion, and solves a related problem that -fno-builtin currently doesn't really work with LTO, because we don't encode it into the IR.

Adding @tejohnson to the conversation.

IIUC you're offering to introduce something like __attribute__((no-builtin-memcpy)) or __attribute__((no-builtin("memcpy"))).
As attributes they would compose nicely with (Thin)LTO.

I believe we still want to turn -fno-builtin flags into attributes so there's no impedance mismatch between the flag and the attribute right?

May 14 2019, 6:42 AM · Restricted Project, Restricted Project
tejohnson retitled D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends from [ThinLTO] Support TargetLibraryInfoImpl in the backend to [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.
May 14 2019, 6:39 AM · Restricted Project, Restricted Project

May 13 2019

tejohnson abandoned D60163: [ThinLTO] Handle -fno-builtin* options for distributed backends.

The parent revision now includes both clang and llvm side changes.

May 13 2019, 1:27 PM · Restricted Project
tejohnson added a comment to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

Working on this one again as it previously wasn't causing any issues but just now got exposed in multiple places and started causing correctness issues. Two questions:

  1. What should be the behavior when merging modules e.g. for LTO. I'm thinking of the following, with multiple module flags to specify different aspects: a) For the NoBuiltinFuncs I'm thinking of a module flag containing the list of strings, with AppendUnique merging behavior, so we get a superset in the merged module b) For the flag to disable all builtins, have a separate module flag with a Max merge (so we get the most conservative behavior) c) For the VectorLibrary, not sure - should it be an Error to merge modules with different libraries?
  2. While we get the above hammered out, would it be ok to submit this one (and companion clang change) to fix the bug (then remove when the module flags are added)?

Weird, phabricator added numbering to all of my bullets, which formatted very differently than intended. Modified the above a bit to hopefully format better (2 high level questions, and some subquestions on the first one).

May 13 2019, 1:27 PM · Restricted Project, Restricted Project
tejohnson updated the diff for D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

Rework using module flags.

May 13 2019, 1:24 PM · Restricted Project, Restricted Project

May 10 2019

tejohnson committed rG962a6f35b5e0: [ThinLTO] Clang test changes for new CanAutoHide flag (authored by tejohnson).
[ThinLTO] Clang test changes for new CanAutoHide flag
May 10 2019, 1:39 PM
tejohnson committed rC360468: [ThinLTO] Clang test changes for new CanAutoHide flag.
[ThinLTO] Clang test changes for new CanAutoHide flag
May 10 2019, 1:39 PM
tejohnson committed rL360468: [ThinLTO] Clang test changes for new CanAutoHide flag.
[ThinLTO] Clang test changes for new CanAutoHide flag
May 10 2019, 1:39 PM
tejohnson committed rG37b80122bd12: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible (authored by tejohnson).
[ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible
May 10 2019, 1:10 PM
tejohnson committed rL360466: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.
[ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible
May 10 2019, 1:06 PM
tejohnson closed D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.
May 10 2019, 1:06 PM · Restricted Project
tejohnson added a comment to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

Working on this one again as it previously wasn't causing any issues but just now got exposed in multiple places and started causing correctness issues. Two questions:

  1. What should be the behavior when merging modules e.g. for LTO. I'm thinking of the following, with multiple module flags to specify different aspects: a) For the NoBuiltinFuncs I'm thinking of a module flag containing the list of strings, with AppendUnique merging behavior, so we get a superset in the merged module b) For the flag to disable all builtins, have a separate module flag with a Max merge (so we get the most conservative behavior) c) For the VectorLibrary, not sure - should it be an Error to merge modules with different libraries?
  2. While we get the above hammered out, would it be ok to submit this one (and companion clang change) to fix the bug (then remove when the module flags are added)?
May 10 2019, 12:23 PM · Restricted Project, Restricted Project
tejohnson added a comment to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

Working on this one again as it previously wasn't causing any issues but just now got exposed in multiple places and started causing correctness issues. Two questions:

  1. What should be the behavior when merging modules e.g. for LTO. I'm thinking of the following, with multiple module flags to specify different aspects:
  2. For the NoBuiltinFuncs I'm thinking of a module flag containing the list of strings, with AppendUnique merging behavior, so we get a superset in the merged module
  3. For the flag to disable all builtins, have a separate module flag with a Max merge (so we get the most conservative behavior)
  4. For the VectorLibrary, not sure - should it be an Error to merge modules with different libraries?
  5. While we get the above hammered out, would it be ok to submit this one (and companion clang change) to fix the bug (then remove when the module flags are added)?
May 10 2019, 10:57 AM · Restricted Project, Restricted Project
tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

Ping - ok if I submit this bugfix?

May 10 2019, 9:49 AM · Restricted Project

May 9 2019

tejohnson accepted D61255: [ThinLTO] Make weak data symbols prevailing when they're visible to regular object.

Fix LGTM. A couple of requests for comments below. Also, please update title of patch, and add a description to the patch summary.

May 9 2019, 9:42 PM · Restricted Project

May 2 2019

tejohnson accepted D61420: [ELF] --plugin-opt=thinlto-index-only: create empty index even if all bitcode files are lazy.

Thanks! LGTM with one suggestion below.

May 2 2019, 6:46 AM · Restricted Project

May 1 2019

tejohnson committed rGb3203ec078ca: [ThinLTO] Fix unreachable code when parsing summary entries. (authored by tejohnson).
[ThinLTO] Fix unreachable code when parsing summary entries.
May 1 2019, 9:28 AM
tejohnson committed rL359697: [ThinLTO] Fix unreachable code when parsing summary entries..
[ThinLTO] Fix unreachable code when parsing summary entries.
May 1 2019, 9:25 AM
tejohnson closed D61355: [ThinLTO] Fix unreachable code when parsing summary entries..
May 1 2019, 9:24 AM · Restricted Project

Apr 30 2019

tejohnson created D61355: [ThinLTO] Fix unreachable code when parsing summary entries..
Apr 30 2019, 5:27 PM · Restricted Project
tejohnson added a comment to D61042: Make post-link regular LTO pipelines match between new and old PM.

ping

Apr 30 2019, 1:39 PM · Restricted Project
tejohnson added a comment to D61255: [ThinLTO] Make weak data symbols prevailing when they're visible to regular object.

Maybe using ReadOnly bits in summary

ReadOnly can be used only after computeDeadSymbols is executed, so IMO not gonna work

Apr 30 2019, 1:30 PM · Restricted Project
tejohnson added a comment to D55153: [ThinLTO] Implement index-based WPD.
In D55153#1481171, @pcc wrote:

Could you please revert the changes from D54815 out of this change so that it is easier to read?

Yep, sorry, looks like that got messed up when I merged in some other changes awhile back. I'm working on merging in the changes I made this morning and last week to the underlying index patch, and then will update this to contain only the delta. Hopefully it will get updated later this afternoon.

Apr 30 2019, 9:47 AM · Restricted Project
tejohnson updated the diff for D55153: [ThinLTO] Implement index-based WPD.

Rebase

Apr 30 2019, 9:42 AM · Restricted Project

Apr 29 2019

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

Could you please revert the changes from D54815 out of this change so that it is easier to read?

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

Implement review suggestions

Apr 29 2019, 12:04 PM · Restricted Project
tejohnson added a comment to D61255: [ThinLTO] Make weak data symbols prevailing when they're visible to regular object.

I'm not convinced this is the right approach. Why isn't r323633 working here ("[ThinLTO] - Stop internalizing and drop non-prevailing symbols")?

Apr 29 2019, 6:58 AM · Restricted Project
tejohnson added a comment to D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible.

ping - @pcc any more comments or can this fix go in now?

Apr 29 2019, 6:35 AM · Restricted Project

Apr 25 2019

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

@pcc - do you have any comments on this patch or are you ok with @davidxl completing the review?

Apr 25 2019, 1:10 PM · Restricted Project
tejohnson updated the diff for D54815: [ThinLTO] Add summary entries for index-based WPD.

Address review comments

Apr 25 2019, 1:10 PM · Restricted Project
tejohnson added a comment to D60793: [Evaluator] Walk initial elements when handling load through bitcast.

I am not familiar with this code, so happy with @evgeny777 's review.

Apr 25 2019, 8:18 AM · Restricted Project

Apr 23 2019

tejohnson created D61042: Make post-link regular LTO pipelines match between new and old PM.
Apr 23 2019, 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
Apr 23 2019, 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
Apr 23 2019, 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
Apr 23 2019, 11:54 AM
tejohnson closed D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM.
Apr 23 2019, 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.
Apr 23 2019, 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.
Apr 23 2019, 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.
Apr 23 2019, 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.
Apr 23 2019, 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.
Apr 23 2019, 8:36 AM · Restricted Project, Restricted Project

Apr 18 2019

tejohnson added a comment to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

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?

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

ping

Apr 18 2019, 11:25 AM · Restricted Project
tejohnson added a comment to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

ping

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

ping

Apr 18 2019, 11:25 AM · Restricted Project

Apr 17 2019

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

LGTM with a minor fix needed below.

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

Apr 16 2019

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

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

Apr 16 2019, 8:59 AM · Restricted Project

Apr 8 2019

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?

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

lgtm

Apr 8 2019, 10:27 AM · Restricted Project

Apr 5 2019

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

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

Apr 5 2019, 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.

Apr 5 2019, 11:33 AM · Restricted Project

Apr 4 2019

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.

Apr 4 2019, 2:21 PM · Restricted Project

Apr 3 2019

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

ping

Apr 3 2019, 7:36 AM · Restricted Project

Apr 2 2019

tejohnson created D60163: [ThinLTO] Handle -fno-builtin* options for distributed backends.
Apr 2 2019, 4:08 PM · Restricted Project
tejohnson added a child revision for D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends: D60163: [ThinLTO] Handle -fno-builtin* options for distributed backends.
Apr 2 2019, 4:08 PM · Restricted Project, Restricted Project
tejohnson created D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.
Apr 2 2019, 4:08 PM · Restricted Project, Restricted Project

Mar 29 2019

tejohnson added inline comments to D59696: [CGP] Build the DominatorTree lazily.
Mar 29 2019, 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?

Mar 29 2019, 8:24 AM · Restricted Project

Mar 27 2019

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.

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

Address comments

Mar 27 2019, 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.

Mar 27 2019, 9:19 PM · Restricted Project
tejohnson added inline comments to D59898: [LCG] Add aliased functions as LCG roots.
Mar 27 2019, 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
Mar 27 2019, 11:44 AM
tejohnson committed rL357111: [CGP] Reset DT when optimizing select instructions.
[CGP] Reset DT when optimizing select instructions
Mar 27 2019, 11:44 AM
tejohnson closed D59889: [CGP] Reset DT when optimizing select instructions.
Mar 27 2019, 11:44 AM · Restricted Project
tejohnson created D59889: [CGP] Reset DT when optimizing select instructions.
Mar 27 2019, 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.

Mar 27 2019, 9:03 AM · Restricted Project

Mar 26 2019

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.

Mar 26 2019, 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