steven_wu (Steven Wu)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 4 2014, 3:46 PM (206 w, 2 d)

Recent Activity

Thu, Oct 11

steven_wu added a comment to D52836: [LTO] Account for overriding lib calls via the alias attribute.

You need to do a release build without assertion. Local names like parameters are dropped from IR to save space/memory.
I think you just need to check the parameter name or the implementation in your test case.

Thu, Oct 11, 12:57 PM

Tue, Oct 9

steven_wu added a comment to D53051: [llvm-tapi] initial commit, supports reading ELF.

Thanks for posting this. I will start with some high level comments.

Tue, Oct 9, 5:27 PM

Wed, Sep 26

steven_wu committed rL343124: [libLTO] Expose LLVMCreateDisasmCPUFeatures from libLTO.
[libLTO] Expose LLVMCreateDisasmCPUFeatures from libLTO
Wed, Sep 26, 9:51 AM

Mon, Sep 24

steven_wu added a comment to D52252: Driver: render arguments for the embedded bitcode correctly.

Thanks for doing this!

Mon, Sep 24, 10:03 AM

Sep 14 2018

steven_wu committed rL342263: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.
[ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool
Sep 14 2018, 12:39 PM
steven_wu closed D52049: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.
Sep 14 2018, 12:39 PM
steven_wu updated the diff for D52049: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.

Update comments

Sep 14 2018, 12:38 PM

Sep 13 2018

steven_wu updated the diff for D52049: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.

Update patch

Sep 13 2018, 4:23 PM
steven_wu added a comment to D52049: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.

Also a bit clarification why ModuleToDefinedGVSummaries can have missing entries, because there can be an empty module, or a module has no GV in it. And this happens during thinLTO WebKit.
For example, in WebCore MacOS build, UnifiedSource26-mm.o has empty text and data section. Its object file only has debug info.

Sep 13 2018, 4:01 PM
steven_wu updated the diff for D52049: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.

Fix up my previous patch.

Sep 13 2018, 12:05 PM
steven_wu added a comment to D52049: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.

I can't find a super clean solution for this. Here is one possible solution, which is to iterate and insert the entry before entering thread pool

Sep 13 2018, 11:10 AM
steven_wu created D52049: [ThinLTOCodeGenerator] Avoid Rehash StringMap in ThreadPool.
Sep 13 2018, 11:08 AM
steven_wu accepted D52023: [ThinLTO]Allow setting of maximum cache size with 64-bit number, and provide C-interface function for large values.

LGTM

Sep 13 2018, 10:02 AM

Sep 7 2018

steven_wu accepted D51693: ADT: add <bit> header, implement C++20 bit_cast, use.

LGTM!

Sep 7 2018, 11:14 AM

Sep 5 2018

steven_wu added a comment to D51440: [ToolChains] Link to compiler-rt with -L + -l when possible.

I do prefer the current approach especially on Darwin. Some concerns of switching to use "-L + -l" are:

  1. clang and compiler-rt are rev-locked. Inferring the compiler-rt from resource-dir and passing to linker with the full path can prevent mistakes of mixing them up.
  2. This change does change linker semantics on Darwin. ld64 treats the inputs from command line with highest priority. Currently ld64 will use compiler-rt symbols before any other libraries passed in with -l flag. If clang decide to pass compiler-rt with -l flag, any other libraries (static or dynamic) that passed with -l flag will takes the priority over compiler-rt. This can lead to unexpected behavior.
Sep 5 2018, 3:47 PM

Sep 4 2018

steven_wu committed rL341422: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was….
[ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was…
Sep 4 2018, 3:55 PM
steven_wu closed D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.
Sep 4 2018, 3:55 PM
steven_wu accepted D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.

LGTM. Thanks!

Sep 4 2018, 3:29 PM
steven_wu requested changes to D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.

Remove the declaration in ThinLTOCodeGenerator.h?

Sep 4 2018, 3:18 PM
steven_wu added a comment to D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.

I dont mind removing codege(). This interface exists since the first prototype and no longer exposed through C API. It seems to have the same model of optimize(), which is also not thread safe. optimize() is still used by llvm-lto but not in thread context.

Sep 4 2018, 1:56 PM
steven_wu added a comment to D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.

I don't think we need another instance of TMBuilder. Can we just call codegenModule directly from run()? TMBuilder should be initialized already from addModule.

Sep 4 2018, 1:07 PM

Aug 23 2018

steven_wu added a comment to D51189: [Sema][ObjC] Infer availability of +new from availability of -init.

I feel like this is a much tricky situation than just new and init. Following example is the same situation.

__attribute__((objc_root_class))
@interface NSObject
- (void) foo;
- (void) bar;
@end
Aug 23 2018, 7:02 PM

Jul 31 2018

steven_wu added inline comments to D50066: [IRMover] Don't materialise values from different source module.
Jul 31 2018, 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?

Jul 31 2018, 9:14 AM

Jul 20 2018

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.

Jul 20 2018, 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.

Jul 20 2018, 11:34 AM

Jul 18 2018

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).

Jul 18 2018, 10:15 AM

Jul 16 2018

steven_wu added a comment to D49362: [ThinLTO] Internalize read only globals.

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] Internalize read only globals.

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