This is an archive of the discontinued LLVM Phabricator instance.

[OPENMP] Fix assert of "Unable to find base lambda address" from adjustMemberOfForLambdaCaptures.
ClosedPublic

Authored by jyu2 on Oct 4 2021, 9:55 PM.

Details

Summary

The problem is happening when user passes lambda function with reference
type in the map clause.

The natural of the problem is when processing generateInfoForCapture, the
BasePointer is generated with new load for a lambda variable with reference
type. It is not expected in adjustMemberOfForLambdaCaptures.

One way to fix this is to skipping call to generateInfoForCapture for
map(to:lambda). The map info will be generated later in the call to
generateDefaultMapInfo, samiler as firsprivate clase.

This to fix https://bugs.llvm.org/show_bug.cgi?id=52071

Diff Detail

Event Timeline

jyu2 created this revision.Oct 4 2021, 9:55 PM
jyu2 requested review of this revision.Oct 4 2021, 9:55 PM
ABataev added inline comments.Oct 5 2021, 5:45 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
8445–8463

Maybe add a new data container for mapped lambdas, similar to FirstPrivateDecls to avoid all those extra checks here?

jyu2 updated this revision to Diff 377288.Oct 5 2021, 10:11 AM

Thanks Alexey's review. This patch is address Alexe's comments by adding new field LambdasMap.

ABataev added inline comments.Oct 5 2021, 10:22 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
8448–8451

No need to create empty ArrayRefs, just pass llvm::None instead.

8928–8942

What if we have to(lambda) in data motion directive? What shall we do if there mapping modifiers? Not sure we shall ignore them.

jyu2 added inline comments.Oct 5 2021, 11:55 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
8928–8942

to(lambda) is not isOpenMPTargetExecutionDirective. I don't see generateInfoForLambdaCaptures and adjustMemberOfForLambdaCaptures functions get called in that part. Those two functions only called in emitTargetCall.
So I don't think we need that.
What do you think?
Thanks.
Jennifer

ABataev added inline comments.Oct 5 2021, 12:11 PM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
8928–8942

Ok, let's skip it for now but still need to keep in mind to fix/implement it in the future.

jyu2 updated this revision to Diff 377344.Oct 5 2021, 1:48 PM

Address Alexey's comments.

ABataev added inline comments.Oct 5 2021, 1:50 PM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
8448–8451

What if there are modifiers in the clause?

jyu2 updated this revision to Diff 377357.Oct 5 2021, 2:36 PM

Thanks Alexey! Fix missing Modifiers.

Tests with modifiers?

jyu2 updated this revision to Diff 377421.Oct 5 2021, 9:22 PM

Adding check for map type in test. Thanks Alexey.

This revision is now accepted and ready to land.Oct 6 2021, 4:38 AM
This revision was automatically updated to reflect the committed changes.