HomePhabricator

[OPENMP50]Codegen for iterator construct.

Authored by ABataev on Apr 6 2020, 8:21 AM.

Description

[OPENMP50]Codegen for iterator construct.

Implemented codegen for the iterator expression in the depend clauses.
Iterator construct is emitted the following way:
iterator(cnt1, cnt2, ...), in : <dep>

<TotalNumDeps> = <cnt1_size> * <cnt2_size> * ...;
kmp_depend_t deps[<TotalNumDeps>];
deps_counter = 0;
for (cnt1) {

for (cnt2) {
  ...
  deps[deps_counter].base_addr = &<dep>;
  deps[deps_counter].size = sizeof(<dep>);
  deps[deps_counter].flags = in;
  deps_counter += 1;
  ...
}

}

For depobj construct the codegen is very similar, but the memory is
allocated dynamically and added extra first item reserved for internal use.

Details

Committed
ABataevApr 7 2020, 12:26 PM
Parents
rGe3ba652a1440: [SampleFDO] Add flag for partial profile.
Branches
Unknown
Tags
Unknown

Event Timeline

rogfer01 added inline comments.
/clang/lib/CodeGen/CGOpenMPRuntime.cpp
5606

Hi Alexey,

Sorry for reviving this but we hit a problem with this code.

CFG.EmitVarDecl invokes CodeGenFunction::EmitVariablyModifiedType which checks an caches the address of the Expr* containing the size in the map VLASizeMap.

Unfortunately OVE is in the stack. So its address might be reused and so we end generating wrong LLVM IR. This is what happens to us.

I don't think OpaqueValueExpr when used for VLAs should be allocated in the stack (I've seen several usages in this file). I failed to find a Create-like factory for OpaqueValueExpr like happens with other Exprs. Perhaps we should add this method so we can allocate them in the ASTContext instead?

If you prefer I can raise a PR for this.

Thanks!

ABataev added inline comments.Aug 6 2021, 11:30 AM
/clang/lib/CodeGen/CGOpenMPRuntime.cpp
5606

Hi, it is not required to have Create to allocate something using ASTContext allocator, just enough to have something like new (Context) .... The best option would be implementing FIXME for VLASizeMap in CodeGenFunction.h. You can prepare a temp patch with allocating these vars in ASTContext instead of the stack and submit it for review.