Page MenuHomePhabricator

lxfind (Xun Li)
User

Projects

User does not belong to any projects.

User Details

User Since
May 3 2020, 4:37 PM (19 w, 4 d)

Recent Activity

Yesterday

lxfind added inline comments to D87596: [Coroutines] Reuse storage for local variables with non-overlapping lifetimes.
Thu, Sep 17, 11:57 AM · Restricted Project
lxfind committed rG5b533d6cdeed: [Coroutine] Fix a bug where Coroutine incorrectly spills phi and invoke defs… (authored by lxfind).
[Coroutine] Fix a bug where Coroutine incorrectly spills phi and invoke defs…
Thu, Sep 17, 8:13 AM
lxfind closed D87810: [Coroutine] Fix a bug where Coroutine incorrectly spills phi and invoke defs before CoroBegin.
Thu, Sep 17, 8:13 AM · Restricted Project

Wed, Sep 16

lxfind updated the diff for D87810: [Coroutine] Fix a bug where Coroutine incorrectly spills phi and invoke defs before CoroBegin.

fix lint

Wed, Sep 16, 7:14 PM · Restricted Project
lxfind requested review of D87810: [Coroutine] Fix a bug where Coroutine incorrectly spills phi and invoke defs before CoroBegin.
Wed, Sep 16, 6:26 PM · Restricted Project
lxfind added inline comments to D87777: [ASAN] Properly deal with musttail calls in ASAN.
Wed, Sep 16, 5:34 PM · Restricted Project
lxfind updated the diff for D87777: [ASAN] Properly deal with musttail calls in ASAN.

fix test

Wed, Sep 16, 10:38 AM · Restricted Project
lxfind updated the diff for D87777: [ASAN] Properly deal with musttail calls in ASAN.

add test

Wed, Sep 16, 10:34 AM · Restricted Project
lxfind updated the diff for D87777: [ASAN] Properly deal with musttail calls in ASAN.

comments

Wed, Sep 16, 10:33 AM · Restricted Project
lxfind requested review of D87777: [ASAN] Properly deal with musttail calls in ASAN.
Wed, Sep 16, 10:26 AM · Restricted Project

Tue, Sep 15

lxfind committed rG7b4cc0961b14: [TSAN] Handle musttail call properly in EscapeEnumerator (and TSAN) (authored by lxfind).
[TSAN] Handle musttail call properly in EscapeEnumerator (and TSAN)
Tue, Sep 15, 3:21 PM
lxfind closed D87620: [TSAN] Handle musttail call properly in EscapeEnumerator (and TSAN).
Tue, Sep 15, 3:21 PM · Restricted Project
lxfind added a comment to D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

@lxfind , could you backport this to branch 11?

Tue, Sep 15, 11:12 AM · Restricted Project

Mon, Sep 14

lxfind added inline comments to D87596: [Coroutines] Reuse storage for local variables with non-overlapping lifetimes.
Mon, Sep 14, 9:22 PM · Restricted Project
lxfind added inline comments to D87596: [Coroutines] Reuse storage for local variables with non-overlapping lifetimes.
Mon, Sep 14, 8:20 PM · Restricted Project
lxfind added a comment to D87596: [Coroutines] Reuse storage for local variables with non-overlapping lifetimes.

Thank you for working on this optimization!

This type of optimizations is typically not enabled in O0, and we probably don't want to either. Should we gate it under the optimization level?

This optimization depends on lifetime markers, which would not be produced in O0. So this optimization won't be enabled in O0.

Mon, Sep 14, 8:04 PM · Restricted Project
lxfind committed rG1f837265eb08: [Coroutines] Fix a typo in documentation (authored by lxfind).
[Coroutines] Fix a typo in documentation
Mon, Sep 14, 7:47 PM
lxfind closed D83563: [Coroutines] Fix a typo in documentation.
Mon, Sep 14, 7:47 PM · Restricted Project
lxfind added reviewers for D83563: [Coroutines] Fix a typo in documentation: rjmccall, wenlei, junparser, ChuanqiXu.
Mon, Sep 14, 1:20 PM · Restricted Project
lxfind updated the diff for D83563: [Coroutines] Fix a typo in documentation.

rebase

Mon, Sep 14, 1:19 PM · Restricted Project
lxfind added a comment to D87596: [Coroutines] Reuse storage for local variables with non-overlapping lifetimes.

Thank you for working on this optimization!

Mon, Sep 14, 1:12 PM · Restricted Project
lxfind added inline comments to D87620: [TSAN] Handle musttail call properly in EscapeEnumerator (and TSAN).
Mon, Sep 14, 12:54 PM · Restricted Project
lxfind added inline comments to D87620: [TSAN] Handle musttail call properly in EscapeEnumerator (and TSAN).
Mon, Sep 14, 12:43 PM · Restricted Project
lxfind updated the diff for D87620: [TSAN] Handle musttail call properly in EscapeEnumerator (and TSAN).

fix test failures

Mon, Sep 14, 12:41 PM · Restricted Project
lxfind requested review of D87620: [TSAN] Handle musttail call properly in EscapeEnumerator (and TSAN).
Mon, Sep 14, 10:12 AM · Restricted Project

Fri, Sep 11

lxfind committed rGdf477db5f9e0: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle (authored by lxfind).
[Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle
Fri, Sep 11, 1:36 PM
lxfind closed D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.
Fri, Sep 11, 1:36 PM · Restricted Project
lxfind added a comment to D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

Thanks, LGTM.

Fri, Sep 11, 1:33 PM · Restricted Project
lxfind updated the diff for D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

remove asan option, not needed

Fri, Sep 11, 1:22 PM · Restricted Project
lxfind updated the summary of D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.
Fri, Sep 11, 1:19 PM · Restricted Project
lxfind updated the diff for D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

Add test to verify that lifetime.end appears right after the address call.

Fri, Sep 11, 1:17 PM · Restricted Project
lxfind updated the diff for D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

Remove unstable test

Fri, Sep 11, 9:51 AM · Restricted Project
lxfind added a comment to D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

hmm @rjmccall, I don't think there is a stable way to test this. The code generated for symmetric transfer is way too complicated to stably pattern match one less item in the frame.

Fri, Sep 11, 9:49 AM · Restricted Project

Thu, Sep 10

lxfind updated the diff for D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

Add test case. Verify that the size of the frame reduced by 8.

Thu, Sep 10, 11:37 PM · Restricted Project
lxfind added a comment to D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.

Thanks for the change. LGTM, and testcase?

Thu, Sep 10, 9:48 PM · Restricted Project
lxfind requested review of D87470: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle.
Thu, Sep 10, 10:27 AM · Restricted Project

Tue, Sep 8

lxfind committed rG59a467ee4fae: [Coroutine] Make dealing with alloca spills more robust (authored by lxfind).
[Coroutine] Make dealing with alloca spills more robust
Tue, Sep 8, 11:10 AM
lxfind closed D86859: [Coroutine] Make dealing with alloca spills more robust.
Tue, Sep 8, 11:10 AM · Restricted Project
lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Tue, Sep 8, 9:52 AM · Restricted Project
lxfind updated the diff for D86859: [Coroutine] Make dealing with alloca spills more robust.

rebase before land

Tue, Sep 8, 9:23 AM · Restricted Project
lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Tue, Sep 8, 9:06 AM · Restricted Project

Fri, Sep 4

lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Fri, Sep 4, 9:12 PM · Restricted Project
lxfind updated the diff for D86859: [Coroutine] Make dealing with alloca spills more robust.

address typo

Fri, Sep 4, 5:37 PM · Restricted Project
lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Fri, Sep 4, 11:13 AM · Restricted Project
lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Fri, Sep 4, 8:59 AM · Restricted Project

Thu, Sep 3

lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Thu, Sep 3, 10:36 PM · Restricted Project
lxfind added a comment to D86859: [Coroutine] Make dealing with alloca spills more robust.

Looking at https://bugs.llvm.org/show_bug.cgi?id=36578:
Andrew Kelley 2019-08-11 09:00:59 PDT
I'm no longer subscribed to this bug report. I've come to the conclusion that LLVM's coroutine API is not worth using, and resorting to implementing coroutines directly in the frontend.

That's an option to look into for solutions here. Also official docs for coro.begin: https://llvm.org/docs/Coroutines.html#llvm-coro-begin-intrinsic

Having anything emitted before frame-setup is tricky because any operation there needs to be tied into DWARF unwind codes. Mirroring these operations as the current design is doing may be a better solution given that. The assertion will definitely be a bug farm as every corner gets tested.

Thu, Sep 3, 10:13 PM · Restricted Project
lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Thu, Sep 3, 10:00 PM · Restricted Project
lxfind added inline comments to D86859: [Coroutine] Make dealing with alloca spills more robust.
Thu, Sep 3, 9:48 PM · Restricted Project
lxfind updated the diff for D86859: [Coroutine] Make dealing with alloca spills more robust.

address lint

Thu, Sep 3, 9:19 PM · Restricted Project
lxfind abandoned D87124: [Coroutine] Properly deal with alloca spills.

By mistake

Thu, Sep 3, 9:18 PM · Restricted Project
lxfind requested review of D87124: [Coroutine] Properly deal with alloca spills.
Thu, Sep 3, 9:17 PM · Restricted Project
lxfind added a comment to D66230: [coroutine] Fixes "cannot move instruction since its users are not dominated by CoroBegin" problem..

Yes, we at Apple had similar problems and basically had to revert this internally. The basic problem is that LLVM's alloca representation is not well-suited for coroutines — we really need a guarantee that there are no stack allocations live across the coro.begin, or at least none that stay alive until the first suspend. That would be easy with something like SIL's alloc_stack, but LLVM's desire to push local allocations into the entry block really makes it difficult to even talk about the appropriate restrictions here.

Maybe we could force all local allocations in coroutines to be done with the llvm.coro.alloca.* intrinsics that I added? We could then actually verify that no allocations are live across the coro.begin. That would require a fair bit of abstraction in Clang, though, and possibly in some other passes that introduce allocas, and it might severely block optimization unless we teach mem2reg how to handle those intrinsics.

Thu, Sep 3, 9:31 AM · Restricted Project
lxfind added a comment to D86859: [Coroutine] Make dealing with alloca spills more robust.

@ChuanqiXu Thank you for taking a look!

Thu, Sep 3, 9:29 AM · Restricted Project

Tue, Sep 1

lxfind added reviewers for D86859: [Coroutine] Make dealing with alloca spills more robust: lewissbaker, bruno.
Tue, Sep 1, 10:21 AM · Restricted Project
lxfind updated the diff for D86859: [Coroutine] Make dealing with alloca spills more robust.

Update description

Tue, Sep 1, 10:18 AM · Restricted Project
lxfind updated the diff for D86859: [Coroutine] Make dealing with alloca spills more robust.

Use a different approach

Tue, Sep 1, 10:13 AM · Restricted Project

Mon, Aug 31

lxfind planned changes to D86859: [Coroutine] Make dealing with alloca spills more robust.
Mon, Aug 31, 11:03 AM · Restricted Project
lxfind updated the diff for D86859: [Coroutine] Make dealing with alloca spills more robust.

Add reviewers

Mon, Aug 31, 10:05 AM · Restricted Project
lxfind published D86859: [Coroutine] Make dealing with alloca spills more robust for review.
Mon, Aug 31, 10:02 AM · Restricted Project

Sun, Aug 30

lxfind added a comment to D66230: [coroutine] Fixes "cannot move instruction since its users are not dominated by CoroBegin" problem..

@GorNishanov This fix seems problematic.
Consider this code:

%var = alloca i32
%1 = getelementptr .. %var; stays put
%f = call i8* @llvm.coro.begin
store ... %1
Sun, Aug 30, 9:54 AM · Restricted Project

Wed, Aug 19

lxfind added inline comments to D86190: [LICM][Coroutine] Don't sink stores from loops with coro.suspend instructions..
Wed, Aug 19, 4:36 PM · Restricted Project

Aug 12 2020

lxfind abandoned D85745: emit int32 for each version part.

After some thought, I think this is not a good change. Many part of the system/toolchain expects each part to be 16-bit.

Aug 12 2020, 10:01 AM · Restricted Project

Aug 11 2020

lxfind abandoned D85477: Use z instead of ZLIB::ZLIB.
Aug 11 2020, 9:27 PM · Restricted Project
lxfind updated the diff for D85745: emit int32 for each version part.

Address comments

Aug 11 2020, 11:47 AM · Restricted Project
lxfind requested review of D85745: emit int32 for each version part.
Aug 11 2020, 9:11 AM · Restricted Project

Aug 7 2020

lxfind added a comment to D79219: [CMake] Simplify CMake handling for zlib.

@phosek, Under this change, now when I build LLVM (with a basic config cmake -G Ninja --LLVM_ENABLE_PROJECTS=clang ../llvm), in file build_dir/lib/cmake/llvm/LLVMExports.cmake, I see this:

set_target_properties(LLVMSupport PROPERTIES
  INTERFACE_LINK_LIBRARIES "curses;m;ZLIB::ZLIB;LLVMDemangle"

This seems broken to me. Can you take a look?

This is correct. That target is provided by find_package(ZLIB). In LLVMConfig.cmake, we invoke find_package(ZLIB) when zlib support is enabled. If you're using LLVMExports.cmake, you'll need to invoke find_package(ZLIB) yourself.

Aug 7 2020, 1:51 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Aug 6 2020

lxfind added a comment to D85477: Use z instead of ZLIB::ZLIB.

@arsenm, we use the direct names everywhere else though (if you scan through the rest of this file). Anyway, I don't mind using it as long as it works properly. I have seen (and have seen others asking in the other commit as well), that ZLIB::ZLIB shows up in build_dir/lib/cmake/llvm/LLVMExports.cmake. That needs to be resolved.

Aug 6 2020, 3:22 PM · Restricted Project
lxfind requested review of D85477: Use z instead of ZLIB::ZLIB.
Aug 6 2020, 3:13 PM · Restricted Project
lxfind added a comment to D79219: [CMake] Simplify CMake handling for zlib.

@phosek, Under this change, now when I build LLVM (with a basic config cmake -G Ninja --LLVM_ENABLE_PROJECTS=clang ../llvm), in file build_dir/lib/cmake/llvm/LLVMExports.cmake, I see this:

set_target_properties(LLVMSupport PROPERTIES
  INTERFACE_LINK_LIBRARIES "curses;m;ZLIB::ZLIB;LLVMDemangle"
Aug 6 2020, 1:55 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Jul 14 2020

lxfind added reviewers for D83563: [Coroutines] Fix a typo in documentation: modocache, lewissbaker.
Jul 14 2020, 8:21 AM · Restricted Project

Jul 10 2020

Herald added a project to D83563: [Coroutines] Fix a typo in documentation: Restricted Project.
Jul 10 2020, 8:59 AM · Restricted Project

Jul 8 2020

lxfind accepted D83379: [Coroutines] Refactor sinkLifetimeStartMarkers.

Thank you!

Jul 8 2020, 11:35 PM · Restricted Project
lxfind added a comment to D83379: [Coroutines] Refactor sinkLifetimeStartMarkers.

Thank you for looking into the fix!

Jul 8 2020, 9:16 AM · Restricted Project

Jul 1 2020

lxfind added a comment to D82992: clang CoverageMapping tests bot cleanup.

Hi, your git commit contains extra Phabricator tags. You can drop Reviewers: Subscribers: Tags: and the text Summary: from the git commit with the following script:

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. Please keep the tag. (--date=now is my personal preference (author dates are usually not useful. Using committer dates can make log almost monotonic in time))

llvm/utils/git/pre-push.py can validate the message does not include unneeded tags.

Jul 1 2020, 5:18 PM · Restricted Project
lxfind committed rG9fc877213e07: clang CoverageMapping tests bot cleanup (authored by lxfind).
clang CoverageMapping tests bot cleanup
Jul 1 2020, 4:17 PM
lxfind closed D82992: clang CoverageMapping tests bot cleanup.
Jul 1 2020, 4:16 PM · Restricted Project
lxfind added a comment to D82986: [Coroutines] Fix test breakage in D82928.

That's not enough, you also need to add an rm to remove the stale .LL file still on disk, see my comment on your original change.

Jul 1 2020, 2:36 PM · Restricted Project
lxfind created D82992: clang CoverageMapping tests bot cleanup.
Jul 1 2020, 1:32 PM · Restricted Project
lxfind committed rGddcf063dd52f: [Coroutines] Fix test breakage in D82928 (authored by lxfind).
[Coroutines] Fix test breakage in D82928
Jul 1 2020, 11:22 AM
lxfind closed D82986: [Coroutines] Fix test breakage in D82928.
Jul 1 2020, 11:22 AM · Restricted Project
lxfind abandoned D82984: Revert "[Coroutines] Fix code coverage for coroutine".

Fix up in https://reviews.llvm.org/D82986

Jul 1 2020, 11:20 AM · Restricted Project
lxfind added a comment to D82928: [Coroutines] Fix code coverage for coroutine.

Looks like this causes a bunch of build bot failures, e.g http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465 b

It would be great if you could take a look.

Jul 1 2020, 11:20 AM · Restricted Project
lxfind created D82986: [Coroutines] Fix test breakage in D82928.
Jul 1 2020, 11:20 AM · Restricted Project
lxfind added a reverting change for rG565e37c7702d: [Coroutines] Fix code coverage for coroutine: D82984: Revert "[Coroutines] Fix code coverage for coroutine".
Jul 1 2020, 11:20 AM
lxfind created D82984: Revert "[Coroutines] Fix code coverage for coroutine".
Jul 1 2020, 11:20 AM · Restricted Project
lxfind added a comment to D82928: [Coroutines] Fix code coverage for coroutine.

Looks like this causes a bunch of build bot failures, e.g http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465 b

It would be great if you could take a look.

Jul 1 2020, 11:20 AM · Restricted Project
lxfind added a comment to D82928: [Coroutines] Fix code coverage for coroutine.

Looks like this causes a bunch of build bot failures, e.g http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465 b

It would be great if you could take a look.

Jul 1 2020, 10:51 AM · Restricted Project
lxfind committed rG565e37c7702d: [Coroutines] Fix code coverage for coroutine (authored by lxfind).
[Coroutines] Fix code coverage for coroutine
Jul 1 2020, 10:16 AM
lxfind closed D82928: [Coroutines] Fix code coverage for coroutine.
Jul 1 2020, 10:16 AM · Restricted Project
lxfind updated the diff for D82928: [Coroutines] Fix code coverage for coroutine.

Rebase

Jul 1 2020, 9:10 AM · Restricted Project
lxfind added a comment to D82314: [Coroutines] Optimize the lifespan of temporary co_await object.

@lxfind This patch causes some mismatch when variable is used in both resume and destroy function. Besides, we should move this patch and the check in buildCoroutineFrame.

@lxfind, Would you try to fix this? If you do not have time, then I'll try do this. Thanks

Jul 1 2020, 9:09 AM · Restricted Project, Restricted Project

Jun 30 2020

lxfind created D82928: [Coroutines] Fix code coverage for coroutine.
Jun 30 2020, 8:35 PM · Restricted Project

Jun 28 2020

lxfind committed rGc8755b6378c2: [Coroutines] Optimize the lifespan of temporary co_await object (authored by lxfind).
[Coroutines] Optimize the lifespan of temporary co_await object
Jun 28 2020, 10:48 AM
lxfind closed D82314: [Coroutines] Optimize the lifespan of temporary co_await object.
Jun 28 2020, 10:48 AM · Restricted Project, Restricted Project
lxfind updated the diff for D82314: [Coroutines] Optimize the lifespan of temporary co_await object.

Rebase before landing

Jun 28 2020, 9:12 AM · Restricted Project, Restricted Project

Jun 26 2020

lxfind retitled D82314: [Coroutines] Optimize the lifespan of temporary co_await object from [RFC][Coroutines] Optimize the lifespan of temporary co_await object to [Coroutines] Optimize the lifespan of temporary co_await object.
Jun 26 2020, 4:38 PM · Restricted Project, Restricted Project
lxfind added a comment to D82314: [Coroutines] Optimize the lifespan of temporary co_await object.

There are still room for improvements, in particular, GCC has a 4K frame for this function.

I think GCC having a smaller coroutine frame is probably because it does not yet implement the allocation-elision optimisation which inlines the nested coroutine frame (which is 4K) into the parent coroutine frame.
I have not looked yet, but I suspect that you'll see within the get_big_object2() code it will perform another heap allocation for the get_big_object() frame.

Until we have more general support for async RVO, I think 8K is probably as good as we're going to be able to get (4K for this coroutine's promise and 4K for child coroutine-frame and its promise).

Jun 26 2020, 4:38 PM · Restricted Project, Restricted Project

Jun 25 2020

lxfind committed rGc25acec84594: [Coroutines] Handle dependent promise types for final_suspend non-throw check (authored by lxfind).
[Coroutines] Handle dependent promise types for final_suspend non-throw check
Jun 25 2020, 11:55 AM
lxfind closed D82332: [Coroutines] Handle dependent promise types for final_suspend non-throw check.
Jun 25 2020, 11:55 AM · Restricted Project