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
4292

Not in Camel format

4537–4541

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

4590

Not in Camel format

4593

No need for const in ArrayRef

4615–4616

Not in Camel format

4737

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

4744

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

4757

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

2687

Same here, remove processing for old functions.

7456

CamelCase

7456

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.

7477

Why do we return 1 here?

7478

No need for else here

7497–7500

Same here

7509

No need for else here

7513–7522

Same here

7608–7617

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

7984–7987

Mappers.push_back(HasMappers?Mapper:nullptr);

8760–8762

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

8865–8866

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

8997–8999

Better to use Builder.CreateConstArrayGEP

9000–9001

Easier cast function to the voidptr, I think

9035–9038

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

9039–9040

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

9400

Better to use MapperCGF.EmitNounwindRuntimeCall.

lib/CodeGen/CGOpenMPRuntime.h
1525

Camel case

1541

The flag HasMappers also must be cleared, no?