User Details
- User Since
- May 3 2020, 4:37 PM (37 w, 4 d)
Yesterday
Tue, Jan 19
Mon, Jan 18
Fri, Jan 15
address comments
Wed, Jan 13
Tue, Jan 12
Thank you!
Thanks. I agree your fix is correct.
Some small suggestions commented
Mon, Jan 11
Mon, Jan 4
Wed, Dec 30
Tue, Dec 29
Thank you for working on this! LGTM. I will let others who are more familiar with SCC to accept it.
A few nits commented inline.
Dec 18 2020
Dec 17 2020
rebase
Dec 16 2020
Update test. A few things: 1) we don't need and shouldn't add --coro-early. Both functions are already marked with coroutine.presplit=1, ready for CoroSplit. coro-early will actually mess it up. 2) It turns out we need the meta data and dbg info since SampleProfileLoader depends on it. 3) We don't need all the attributes. Removed the unused ones.
Dec 15 2020
Dec 10 2020
Dec 9 2020
Fix test
Fix all failing tests
Dec 8 2020
Dec 7 2020
Add missing file. Rename isCoroutine to isPresplitCoroutine
Dec 4 2020
Dec 2 2020
Nov 19 2020
Hi @dongAxis1944, @junparser, do you still plan to reland this patch?
Nov 18 2020
@rsmith, @v.g.vassilev hey I stamped this patch assuming it looks ok. But definitely shout at me if more feedback needs to be addressed. Happy to follow up.
Nov 16 2020
Nov 14 2020
@aeubanks, I was wondering if you are familiar with this but it seems that the SCC update after CoroSplit couldn't handle the case when there is recursion in the original coroutine, that is, the newly generated functions by CoroSplit will contain cycles (foo calling foo.resume which then calls foo). Do you have suggestions/thoughts on how to fix it?
For example, test like this will fail assertions:
; RUN: opt < %s --enable-new-pm --enable-coroutines -O3 -S ; Function Attrs: argmemonly nounwind readonly declare token @llvm.coro.id(i32, i8* readnone, i8* nocapture readonly, i8*) #0
Nov 13 2020
Do we really need handle this in non-optimized mode? @bruno I'm afraid that this patch may affect the debugging quality fixed in D90772, @lxfind, have you ever checked this?
Nov 12 2020
Nov 11 2020
Landed by accident. Reverted and reopening for review.
Thank you!
Are all uses of on_ompt_callback_master expected to be replaced?
I see this is still using it:
https://github.com/llvm/llvm-project/blob/master/openmp/tools/multiplex/tests/custom_data_storage/first-tool.h#L126
Good catch. Thanks for the fix! The IsImplicit argument for buildCoawaitCalls function can also be removed now.
Nov 10 2020
ping :)
Add AST test
Nov 9 2020
Curious, is the plan to eventually replace .recon lowering with .async lowering? If so, would you mind explain in a bit more detail what's the advantage of .async over .retcon lowering and what motivates this new lowering? Thanks!
LGTM. Thanks!
Nov 6 2020
Could you add a test (or update existing tests) to demonstrate that the issue is fixed?