steven_wu (Steven Wu)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 4 2014, 3:46 PM (197 w, 1 d)

Recent Activity

Tue, Jul 31

steven_wu added inline comments to D50066: [IRMover] Don't materialise values from different source module.
Tue, Jul 31, 11:48 AM
steven_wu added a comment to D50066: [IRMover] Don't materialise values from different source module.

Can you add a bit more explanation other than just saying fixing PR38372?

Tue, Jul 31, 9:14 AM

Fri, Jul 20

steven_wu added a comment to D49434: Put "built-in" function definitions in global Used list, for LTO. (fix bug 34169).
In D49434#1170267, @pcc wrote:

I would imagine that folks who care about code size would be linking with --gc-sections (or -dead_strip in ld64) so the library functions would end up being dropped if they aren't actually needed.

Fri, Jul 20, 1:46 PM
steven_wu added a comment to D49434: Put "built-in" function definitions in global Used list, for LTO. (fix bug 34169).

For the old API, I don't think it is reasonable to expect user or linker to pass in the correct preserve list. There is likely no knowledge outside the LTO modules that these lib funcs are needed.

Fri, Jul 20, 11:34 AM

Wed, Jul 18

steven_wu added a comment to D49434: Put "built-in" function definitions in global Used list, for LTO. (fix bug 34169).
In D49434#1165612, @pcc wrote:

You wouldn't be making changes to the IR, only the summary. I was thinking that you might add code to http://llvm-cs.pcc.me.uk/lib/Analysis/ModuleSummaryAnalysis.cpp#534 that would effectively do:

if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID("memcpy")))
    for (auto &Summary : VI.getSummaryList())
      Summary->setLive(true);
if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID("memmove")))
    for (auto &Summary : VI.getSummaryList())
      Summary->setLive(true);
// etc.

If I recall correctly it wasn't the LTO dead code elimination that was removing them. Caroline, I forget what was happening in the original case - were they being internalized by LTO and then eliminated by GlobalDCE? If so, then Peter's approach should fix the issue by preventing them from being internalized (at least by ThinLTO, not sure about regular LTO).

Wed, Jul 18, 10:15 AM

Jul 16 2018

steven_wu added a comment to D49362: [ThinLTO] Compute constant references.

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type. That will avoid the need to do any modification to the optimization passes. On the exporting side, we could do prevent the promotion if we knew that all modules importing a reference also imported the referenced variable definition. It does however look like we should be importing all non-interposable linkage constant variables that are referenced, which should be all we care about for this anyway.

What if the constant is really big? Looks like the importing decision has to be made by the thin linker directly. You can not import the "constant" global as something like "available_externally constant" because the original copy might not be visible from the original module (just like the example), unless you want to modify the original copy to be "external hidden".

Not sure I understand your question, can you clarify the concern? Right now, computeImportForReferencedGlobals during the thin link we import any referenced variable, unless it has interposable linkage or references other summaries (in which case it is not a constant). The imported variables are imported as available externally definitions. The original definition is currently promoted to external hidden.

Jul 16 2018, 10:09 AM
steven_wu added a comment to D49362: [ThinLTO] Compute constant references.

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type. That will avoid the need to do any modification to the optimization passes. On the exporting side, we could do prevent the promotion if we knew that all modules importing a reference also imported the referenced variable definition. It does however look like we should be importing all non-interposable linkage constant variables that are referenced, which should be all we care about for this anyway.

Jul 16 2018, 10:00 AM

Jul 10 2018

steven_wu accepted D48783: [ThinLTO] Use std::map to get determistic imports files.

LGTM. Thanks!

Jul 10 2018, 9:25 AM

Jul 9 2018

steven_wu committed rL336574: Add bitcode compatibility test for 6.0.
Add bitcode compatibility test for 6.0
Jul 9 2018, 11:02 AM
steven_wu closed D49086: Add bitcode compatibility test for 6.0.
Jul 9 2018, 11:02 AM
steven_wu created D49086: Add bitcode compatibility test for 6.0.
Jul 9 2018, 9:58 AM
steven_wu committed rL336560: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.
[BitcodeReader] Infer the correct runtime preemption for GlobalValue
Jul 9 2018, 9:57 AM
steven_wu closed D49039: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.
Jul 9 2018, 9:57 AM

Jul 6 2018

steven_wu updated the diff for D49039: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.

Add testcase using 6.0 bitcode.

Jul 6 2018, 7:20 PM
steven_wu updated the diff for D49039: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.

Address review feedback

Jul 6 2018, 4:06 PM
steven_wu abandoned D49048: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.

oops, accidentally opened a new review. closing.

Jul 6 2018, 4:03 PM
steven_wu created D49048: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.
Jul 6 2018, 4:02 PM
steven_wu added a comment to D49039: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.

In that case, we can enable verification on checked-in bitcode files as a follow-up. How about testing this by round-tripping textual IR through llvm-as and llvm-dis? You just need a function, alias, and global var with local linkage and without dso_local set, right?

Jul 6 2018, 4:01 PM
steven_wu added a comment to D49039: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.
In D49039#1154801, @vsk wrote:

Thanks for doing this!

Could you add a test? Adding "opt -verify <old-bitcode-file>" run lines to the compatibility tests would be fine.

Jul 6 2018, 3:18 PM
steven_wu created D49039: [BitcodeReader] Infer the correct runtime preemption for GlobalValue.
Jul 6 2018, 1:36 PM

Jul 2 2018

steven_wu committed rC336168: [Driver][Darwin] Use Host Triple to infer target os version.
[Driver][Darwin] Use Host Triple to infer target os version
Jul 2 2018, 9:20 PM
steven_wu committed rL336168: [Driver][Darwin] Use Host Triple to infer target os version.
[Driver][Darwin] Use Host Triple to infer target os version
Jul 2 2018, 9:20 PM
steven_wu closed D48849: [Driver][Darwin] Use Host Triple to infer target os version.
Jul 2 2018, 9:20 PM
steven_wu updated the diff for D48849: [Driver][Darwin] Use Host Triple to infer target os version.

It is easier and cleaner if I just fold everything into getOSVersion.

Jul 2 2018, 6:01 PM
steven_wu updated the diff for D48849: [Driver][Darwin] Use Host Triple to infer target os version.

handle *-apple-macosx target option

Jul 2 2018, 4:18 PM
steven_wu added a comment to D48849: [Driver][Darwin] Use Host Triple to infer target os version.

Hmm, the driver should not call inferDeploymentTargetFromArch when -target is passed. Or am I missing something?

Jul 2 2018, 3:59 PM
steven_wu updated the diff for D48849: [Driver][Darwin] Use Host Triple to infer target os version.

Rebase the commit correctly

Jul 2 2018, 3:36 PM
steven_wu updated the diff for D48849: [Driver][Darwin] Use Host Triple to infer target os version.

Update patch. Use a better API.

Jul 2 2018, 3:33 PM
steven_wu added a comment to D48849: [Driver][Darwin] Use Host Triple to infer target os version.

Unfortunately, I wasn't able to write a test for this because the host triple in the configuration can either be x86_64-apple-darwin* or x86_64-apple-macosx*, but the one used passed by driver is always macosx one. I can't reliably compare those two.

Jul 2 2018, 2:33 PM
steven_wu created D48849: [Driver][Darwin] Use Host Triple to infer target os version.
Jul 2 2018, 2:30 PM

Jun 29 2018

steven_wu added a comment to D48783: [ThinLTO] Use std::map to get determistic imports files.

Does the iteration order of those map matters other than changing the hash? Will it change symbol resolution or importing different function?

No effect on importing/symbol resolution etc. In fact, this is not an input to the compiler at all, it is used by the build system. The change in order will just make it look to the build system like it has changed when it hasn't.

Jun 29 2018, 1:48 PM
steven_wu added a comment to D48783: [ThinLTO] Use std::map to get determistic imports files.

Does the iteration order of those map matters other than changing the hash? Will it change symbol resolution or importing different function?

Jun 29 2018, 1:05 PM

Jun 22 2018

steven_wu committed rC335366: Add const qualifier on FieldChainInfoComparator::operator().
Add const qualifier on FieldChainInfoComparator::operator()
Jun 22 2018, 9:55 AM
steven_wu committed rL335366: Add const qualifier on FieldChainInfoComparator::operator().
Add const qualifier on FieldChainInfoComparator::operator()
Jun 22 2018, 9:55 AM

Jun 13 2018

steven_wu accepted D48108: Handle an Xcode path with spaces in it.

LGTM.

Jun 13 2018, 9:59 AM

Jun 11 2018

steven_wu added a comment to D47994: [Darwin] Do not error on '-lto_library' option.

I am not working on lld but this looks good for me. I like this solution better than the alternative you mentioned.

Jun 11 2018, 9:37 AM

May 23 2018

steven_wu committed rL333148: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr.
[Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr
May 23 2018, 6:06 PM
steven_wu committed rC333148: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr.
[Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr
May 23 2018, 6:06 PM
steven_wu closed D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr.
May 23 2018, 6:05 PM
steven_wu created D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr.
May 23 2018, 1:44 PM

May 14 2018

steven_wu added a comment to D46858: Signal handling should be signal-safe.

I think it is easier to unregister all the signal handlers in the beginning of the llvm_shutdown() since you should be done with all the work when you call llvm_shutdown(). This can also allow safe access ManagedStatics in signal handler, which I don't know if we have more of those other than the lock you are trying to fix. The only difference might be that if an interrupt is triggered when freeing ManagedStatics, llvm signal handler will not be used.

May 14 2018, 6:46 PM

May 10 2018

steven_wu accepted D45930: [Support] Upstream anonymization and manipulating of BCSymbolMaps.

Are you planning to upstream the module pass to justify this to be part of Support library?

May 10 2018, 8:27 AM

Apr 23 2018

steven_wu added a comment to D45930: [Support] Upstream anonymization and manipulating of BCSymbolMaps.

Thanks for working on this Jonas. Duncan and Adrian has already covered pretty much all I want to say. A small additional comments.

Apr 23 2018, 9:28 AM

Apr 19 2018

steven_wu committed rL330338: [CXX] Templates specialization visibility can be wrong.
[CXX] Templates specialization visibility can be wrong
Apr 19 2018, 8:52 AM
steven_wu committed rC330338: [CXX] Templates specialization visibility can be wrong.
[CXX] Templates specialization visibility can be wrong
Apr 19 2018, 8:52 AM
steven_wu closed D44670: [CXX] Templates specialization visibility can be wrong.
Apr 19 2018, 8:52 AM
steven_wu closed D44670: [CXX] Templates specialization visibility can be wrong.
Apr 19 2018, 8:52 AM

Apr 18 2018

steven_wu added inline comments to D44670: [CXX] Templates specialization visibility can be wrong.
Apr 18 2018, 10:30 AM
steven_wu added a reviewer for D44670: [CXX] Templates specialization visibility can be wrong: doug.gregor.
Apr 18 2018, 10:29 AM
steven_wu updated the diff for D44670: [CXX] Templates specialization visibility can be wrong.

Address review feedback

Apr 18 2018, 10:29 AM

Apr 16 2018

steven_wu committed rL330166: [Availability] Improve availability to consider functions run at load time.
[Availability] Improve availability to consider functions run at load time
Apr 16 2018, 4:37 PM
steven_wu committed rC330166: [Availability] Improve availability to consider functions run at load time.
[Availability] Improve availability to consider functions run at load time
Apr 16 2018, 4:37 PM
steven_wu closed D45699: [Availability] Improve availability to consider functions run at load time.
Apr 16 2018, 4:37 PM
steven_wu updated the diff for D45699: [Availability] Improve availability to consider functions run at load time.

Address review feedback

Apr 16 2018, 4:35 PM
steven_wu added a reviewer for D45699: [Availability] Improve availability to consider functions run at load time: erik.pilkington.
Apr 16 2018, 4:05 PM
steven_wu updated the diff for D45699: [Availability] Improve availability to consider functions run at load time.

Address review feedback. Fix typos and comments.

Apr 16 2018, 3:59 PM
steven_wu added a comment to D45699: [Availability] Improve availability to consider functions run at load time.

Thanks for reviewing Erik!

Apr 16 2018, 3:54 PM
steven_wu created D45699: [Availability] Improve availability to consider functions run at load time.
Apr 16 2018, 2:10 PM

Apr 10 2018

steven_wu committed rL329752: [MachO] Emit Weak ReadOnlyWithRel to ConstDataSection.
[MachO] Emit Weak ReadOnlyWithRel to ConstDataSection
Apr 10 2018, 1:19 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Apr 10 2018, 1:19 PM

Apr 9 2018

steven_wu created D45472: [MachO] Emit Weak ReadOnlyWithRel to ConstDataSection.
Apr 9 2018, 6:41 PM

Mar 30 2018

steven_wu accepted D45076: Prevent data races in concurrent ThinLTO processes.

LGTM. Thanks!

Mar 30 2018, 1:52 PM
steven_wu added a comment to D45076: Prevent data races in concurrent ThinLTO processes.

LGTM with some feedback inline.

Mar 30 2018, 10:31 AM

Mar 21 2018

steven_wu added a comment to D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros.

I am not trying to discuss which english word is best here. My point is simply:

  1. macros are evaluated during compile time
  2. "host"means either the platform you compiled on during compile time or the platform you run on during the runtime
  3. __is_host_* is not a good name, because it is misleading as it either implies "runtime" as a compile-time constant, or indicates the wrong platform.
Mar 21 2018, 3:40 PM
steven_wu added a comment to D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros.

It is not about matching command line name to builtin marco name. "target" is the platform we are compiling for, whether it is host or device or something else. It is a different concept when you talking about cross-compiling, which "target" is strictly not host and "build" or "host" doesn't matter to compiler at all.

Mar 21 2018, 2:55 PM
steven_wu added a comment to D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros.

I disagree. I think "target" is the correct name, even for cross compiling. For something compiled with -target foo, we are consistent calling it "target" during compile time. It only becomes appropriate to call it "host" during the runtime of the executable. There is no such concept as "host" when you are doing cross compiling.

Mar 21 2018, 2:34 PM

Mar 19 2018

steven_wu abandoned D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.
Mar 19 2018, 7:16 PM
steven_wu created D44670: [CXX] Templates specialization visibility can be wrong.
Mar 19 2018, 7:08 PM

Mar 15 2018

steven_wu added a comment to D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.

If we all agree that dropping local_unnamed_addr is invalid, I can abandon this. Thanks for the feedback!

Mar 15 2018, 6:29 PM
steven_wu added a comment to D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.

I might have missed something. Can you be more specific about how it is going to work?

Mar 15 2018, 5:02 PM
steven_wu added a comment to D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.

The solution is to use canBeOmittedFromSymbolTable from include/llvm/Object/IRSymtab.h.

The symbol table is computed earlier and so the above predicate still returns 1. Your examples work with lld and ThinLTO if you want to check the implementation details.

Mar 15 2018, 4:39 PM
steven_wu added a comment to D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.

So the original of this patch is thinLTO is promoting linkonce_odr to weak_odr. Think the following two situations:

Mar 15 2018, 4:01 PM

Mar 13 2018

steven_wu added a comment to D44373: Fix reading objects created with -fembed-bitcode-marker..

Additional comment, I didn't realize llvm-nm now prioritize to print bitcode symbol table, rather than the symbol table from the object file. There should be an option to choose which one to see and maybe also an option to see both.

Mar 13 2018, 11:31 AM
steven_wu accepted D44373: Fix reading objects created with -fembed-bitcode-marker..
Mar 13 2018, 10:39 AM
steven_wu added a comment to D44373: Fix reading objects created with -fembed-bitcode-marker..

Thanks for fixing this. LGTM.

Mar 13 2018, 10:39 AM

Mar 1 2018

steven_wu added a comment to D43959: [asan] Fix a false positive ODR violation due to LTO ConstantMerge pass.

Thanks, yes, AFAIK, ld64 is fine and I don't really know how to test lld on Darwin (is it even supposed to work?).

Mar 1 2018, 11:13 AM · Restricted Project
steven_wu accepted D43959: [asan] Fix a false positive ODR violation due to LTO ConstantMerge pass.

The idea is to prevent LTO (ConstMerge pass) from merging global constants that has the same value, which has the same side-effect as linker merging two weak symbols that has ODR violation.

Mar 1 2018, 10:54 AM · Restricted Project

Feb 28 2018

steven_wu accepted D42446: [ThinLTO] Add a couple of more knobs to C API to control cache size..

LGTM. And yes, order doesn't matter.

Feb 28 2018, 4:22 PM

Feb 26 2018

steven_wu added a comment to D42446: [ThinLTO] Add a couple of more knobs to C API to control cache size..

The API change is fine for ld64. But you need to do the following when you update LTO C API:

  • You need to declare the interface in include/llvm-c/lto.h
  • You need to bump LTO_API_VERSION in that header and document the new API is available since the new LTO_API_VERSION.
  • You need to update tools/lto/lto.exports. This is the export list used by ld64 and only the interface listed in that file will be exported on darwin platform.
Feb 26 2018, 5:11 PM

Feb 19 2018

steven_wu committed rL325525: bitcode support change for fast flags compatibility.
bitcode support change for fast flags compatibility
Feb 19 2018, 11:24 AM
steven_wu closed D43253: bitcode support change for fast flags compatibility.
Feb 19 2018, 11:24 AM
steven_wu accepted D43253: bitcode support change for fast flags compatibility.

Michael doesn't have commit right yet so I will commit this for him.

Feb 19 2018, 11:17 AM

Feb 16 2018

steven_wu added a comment to D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.

I am using local_unnamed_addr as an example. Maybe you are right but I think my point is still valid. That is, LTO optimization and code generation can modify the symbol table from the bitcode file. It is already the case that symbols can be internalized, removed, backend can also insert new symbols. Unless there is a rule saying LTO passes cannot modify the linkage or visibility or any other attributes to the symbols, linker cannot trust whatever the bitcode symbol table says and firmly it will not be changed for some good reason.

Feb 16 2018, 12:07 PM
steven_wu added a comment to D43253: bitcode support change for fast flags compatibility.

I think this approach is much better. Can you update the title because it is not really a version bump anymore? From my understanding, this is good enough for backwards-compatibility. Thanks!

Feb 16 2018, 10:20 AM
steven_wu added a comment to D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.
In D43361#1009616, @pcc wrote:

There is an alternative which is to teach linker to deduct auto hide property from the bitcode file and overwrite the linkage type afterwards. For C API, there is a new scope LTO_SYMBOL_SCOPE_DEFAULT_CAN_BE_HIDDEN can be used to do that but that will be forced the work onto all the linker which is probably not desirable but it solves the same problem.

This seems better to me. Ideally the linker should be reading and resolving symbols from bitcode files in the same way as regular object files, including the auto-hide bit. Linkers would need something similar for any other properties that are contained only in bitcode files but not in object files (such as a general address-taken bit).

Feb 16 2018, 9:46 AM

Feb 15 2018

steven_wu added a comment to D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.
In D43361#1009486, @pcc wrote:

The last time this was attempted (D28978) we established that the linker can do this itself. I don't think the situation has changed since then.

Feb 15 2018, 4:22 PM
steven_wu created D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.
Feb 15 2018, 3:28 PM
steven_wu added a dependent revision for D43360: [Bitcode] Add UnnamedAddr attributes to GlobalValueSummary: D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.
Feb 15 2018, 3:28 PM
steven_wu added a dependency for D43361: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr: D43360: [Bitcode] Add UnnamedAddr attributes to GlobalValueSummary.
Feb 15 2018, 3:28 PM
steven_wu created D43360: [Bitcode] Add UnnamedAddr attributes to GlobalValueSummary.
Feb 15 2018, 3:25 PM
steven_wu abandoned D43358: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.

Generated by mistake. Abandon and I will reformat the patch.

Feb 15 2018, 3:24 PM
steven_wu created D43358: [ThinLTO] Enable AutoHide on symbols with local_unnamed_addr.
Feb 15 2018, 3:21 PM

Feb 9 2018

steven_wu created D43139: [GlobalOpts] mark linkonce_odr unnamed_addr GV as hidden.
Feb 9 2018, 12:37 PM
steven_wu committed rL324757: [ThinLTO] Teach ThinLTO about auto hide symbols.
[ThinLTO] Teach ThinLTO about auto hide symbols
Feb 9 2018, 10:36 AM
steven_wu closed D43130: [ThinLTO] Teach ThinLTO about auto hide symbols.
Feb 9 2018, 10:36 AM
steven_wu updated the diff for D43130: [ThinLTO] Teach ThinLTO about auto hide symbols.

Move the check later so it is only check when needed.

Feb 9 2018, 10:31 AM
steven_wu created D43130: [ThinLTO] Teach ThinLTO about auto hide symbols.
Feb 9 2018, 10:12 AM

Feb 6 2018

steven_wu added a comment to D42267: [ThinLTO] Allow 0 to be a valid value for pruning interval for C LTO API..

I am definitely not against the change. It would be good to have the number means the same thing in C/C++ API. I think ld64 can be updated in advance before we release 7.0 so we can have a larger compatibility window so I don't think this should be a blocker.

So, can I assume that I got an "OK" from the other main stakeholder who is using C LTO API? BTW, I CC-ed you to another small patch https://reviews.llvm.org/D42446, where I added a couple of missing APIs for controlling cache policy. You might want to expose it to ld64 users.

Feb 6 2018, 4:22 PM
steven_wu added a comment to D42267: [ThinLTO] Allow 0 to be a valid value for pruning interval for C LTO API..

This indeed affects how ld64 interact with libLTO. If user didn't specify a value, ld64 will always call thinlto_codegen_set_cache_pruning_interval with interval 0. This is currently a no-op so it will use whatever the default value in libLTO. With this change, it will be force pruning.

It is possible for us to make change to future ld64 to use different value but it is not going work very well for compatibility. If old ld64 loaded a new libLTO, it is going to lose incremental build.

Well, the main reason for this change was:

(a) compatibility with C++ API. A lot of code in libLTO is common for C API and C++ API. Having the same semantics for the same options in two different APIs will make code simpler and prevents from someone who only cares about C+ API to unintentionally break the C API functionality. The latter is particularly important, since incremental linking is not thoroughly tested.

(b) giving a possibility to a user to run a pruner immediately, vs. "no-op" which could get achieved simply by not passing "pruning-interval" option to the linker, making option value 0 useless.

I understand your compatibility issue, but I don't think it's very serious.

  • I doubt it's common for a user to pass a option to the linker, if the same effect could be achieved by not passing any option at all (why would user do extra work?).
  • This option will cause the pruner to run, which not necessarily means that the cache files will get removed and incremental build won't happen. Cache files will get removed only if at the same time the files are stale, or their sizes are larger then certain thresholds, which are passed as parameters.
  • The pruner runs just 20 minutes earlier than it would have run by default. So, only the builds that are done more frequent than 20 minutes might be affected.
  • Only the users who pick up the linker and libLTO from different packages will get affected.

    Let me know what you decide. If you want to keep things the old way, I could modify this patch for PS4 only.

    So, to summarize, we could add a meaningful value for pruning_interval option, which currently can't get achieved otherwise, and additionally we will offer compatibility with C++API. The only users that will be affected are the ones who because of whatever reason decided to explicitly specify a default option on the command line rather than skipping it, whose builds are happening more frequently than 20 minutes and who had mixed and matched different releases of ld64 and libLTO.
Feb 6 2018, 3:39 PM
steven_wu added a comment to D42267: [ThinLTO] Allow 0 to be a valid value for pruning interval for C LTO API..

This indeed affects how ld64 interact with libLTO. If user didn't specify a value, ld64 will always call thinlto_codegen_set_cache_pruning_interval with interval 0. This is currently a no-op so it will use whatever the default value in libLTO. With this change, it will be force pruning.

Feb 6 2018, 1:46 PM