Merge r310850 into the 5.0 branch: Fix for #33676 : Debug info for OpenMP region is broken.

[hahnjo]

Bugzilla Link	35176
Resolution	LATER
Resolved on	Nov 15, 2017 07:14
Version	unspecified
OS	All
Blocks	#33840
CC	@alexey-bataev,@hfinkel,@tstellar
Fixed by commit(s)	r310850

Extended Description

After some changes in clang/LLVM debug info for task-based regions was
not generated at all. Patch fixes this problem.

[hahnjo]

assigned to @alexey-bataev

[tstellar]

Hi Alexey,

Is this OK to merge to the release_50 branch: https://reviews.llvm.org/rL310850

[alexey-bataev]

Yes

[tstellar]

Hi Jonas,

Do we also need to backport r310387 ?

[hahnjo]

Hi Jonas,

Do we also need to backport r310387 ?

Hi Tom,

in theory we could, but I'd prefer not to if we don't need it (e.g. to avoid merge conflicts): The patch is much more intrusive and deals with target constructs and offloading, GPUs in particular. This doesn't work in 5.0 anyway so I don't see much value in backporting (I've skipped most / all of the target patches when going over the backport candidates). Anyway, assigning back to Alexey for a final decision.

[tstellar]

Hi Jonas,

Do we also need to backport r310387 ?

Hi Tom,

in theory we could, but I'd prefer not to if we don't need it (e.g. to avoid
merge conflicts): The patch is much more intrusive and deals with target
constructs and offloading, GPUs in particular. This doesn't work in 5.0
anyway so I don't see much value in backporting (I've skipped most / all of
the target patches when going over the backport candidates). Anyway,
assigning back to Alexey for a final decision.

Ok, we can skip it then. It touched the same parts of the code as r310850 so I just wanted to make sure r310850 was still useful without it.

[tstellar]

Also, make check fails when I apply r310850 to the release_50 branch. Jonas, Can you merge this one?

[hahnjo]

Also, make check fails when I apply r310850 to the release_50 branch.
Jonas, Can you merge this one?

Ok, I think I see the problem. r310850 builds on a refactoring that went in after the branching. So we would need to backport all of these:
https://reviews.llvm.org/rL310098
https://reviews.llvm.org/rL310840 (modulo the test changes to parallel_codegen.cpp which only work after https://reviews.llvm.org/rL310511)
Finally https://reviews.llvm.org/rL310850

I'm attaching the aggregated diff:
lib/CodeGen/CGOpenMPRuntime.cpp | 47 +++++++++++++++------
lib/CodeGen/CGOpenMPRuntime.h | 12 +++++
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp | 64 ++++++++++++++++++-----------
lib/CodeGen/CGOpenMPRuntimeNVPTX.h | 6 ++
lib/CodeGen/CGStmtOpenMP.cpp | 15 ++++--
test/OpenMP/ordered_codegen.cpp | 4 -
test/OpenMP/taskloop_reduction_codegen.cpp | 3 -

I really don't think we should merge that, I'll have to accept that the debugging experience will be better again in 6.0.0.

[alexey-bataev]

Agree with Jonas, closing it