Page MenuHomePhabricator

[OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime
Needs ReviewPublic

Authored by lildmh on Sep 20 2019, 6:51 AM.

Details

Summary

This patch implements the code generation to use OpenMP 5.0 declare mapper (e.g., user-defined mapper) constructs. It looks up the proper mapper function for each map, to, or from clause that has a user-defined mapper associated, and passes them to the OpenMP runtime function.
The design slides can be found at https://github.com/lingda-li/public-sharing/blob/master/mapper_runtime_design.pptx

Diff Detail

Event Timeline

lildmh created this revision.Sep 20 2019, 6:51 AM
Herald added a project: Restricted Project. · View Herald Transcript
lildmh updated this revision to Diff 221578.Sep 24 2019, 1:00 PM
lildmh added reviewers: ABataev, Meinersbur, hfinkel.
lildmh added a project: Restricted Project.

I would suggest to split this patch into 2 part: the first one is NFC and just makes the ompiler to use the new interfaces and the second with the new functionality for mappers.

include/clang/AST/OpenMPClause.h
4298

Not in Camel format

4543

Why changed to const Expr * here? I believe, ArrayRef makes it constant automatically.

4596

Not in Camel format

4599

No need for const in ArrayRef

4621

Not in Camel format

4743

ArrayRef<const Expr *>()->llvm::None?

4750

ArrayRef<const Expr *>()->llvm::None

4763

Same here

lib/CodeGen/CGOpenMPRuntime.cpp
742

Better to fix it in a separate patch

755–794

I believe these new functions replace completely the old one, so old functions must be removed

2746

Same here, remove processing for old functions.

7388

CamelCase

7388

Seems to me, with mapper you're changing the contract of this function. It is supposed to return the size in bytes, with Mapper it returns just number of elements. Bad decision. Better to convert size in bytes to number of elements in place where you need it.

7411

Why do we return 1 here?

7412

No need for else here

7431–7434

Same here

7445

No need for else here

7463–7472

Same here

7564–7573

Too many params in the function, worth thinking about wrapping them in a record.

7940–7943

Mappers.push_back(HasMappers?Mapper:nullptr);

8716–8718

const auto *VD = dyn_cast_or_null<VarDecl>(std::get<0>(L));

8821–8822

In general, there should not be branch for default mapping during the codegen, the implicit map clauses must be generated in sema.

8953–8955

Better to use Builder.CreateConstArrayGEP

8956–8957

Easier cast function to the voidptr, I think

8991–8994

Hmm, just cast of the Info.MappersArray to the correct type if all indices are 0?

8995–8996

If then sub-statement in braces, the else substatement also must be in braces

9359

Better to use MapperCGF.EmitNounwindRuntimeCall.

lib/CodeGen/CGOpenMPRuntime.h
1533

Camel case

1549

The flag HasMappers also must be cleared, no?