This is an archive of the discontinued LLVM Phabricator instance.

[flang] addition of Coarray and some header files
AbandonedPublic

Authored by schweitz on Jun 17 2020, 6:41 PM.

Details

Summary

This continues to upstream small chunks of the bridge from the flang front-end to the FIR dialect of MLIR.

Add Coarray lowering hooks.
Also add several header files.

    
AbstractConverter is an interface for lowering front-end data structures t
BoxValue defines tuples of related values used in the lowering process.
RTBuilder defines models for lowering C++ functions decls to MLIR.
SymbolMap defines a mapping of front-end symbols to box values.

For now, Bridge.h is a placeholder. It will be upstreamed in a later diff.

Diff Detail

Event Timeline

schweitz created this revision.Jun 17 2020, 6:41 PM
schweitz updated this revision to Diff 271546.Jun 17 2020, 6:51 PM

minor edits on a comments

I'm not qualified to comment on the contents of this patch, but everything builds and tests correctly in an out-of-tree build.

I'm not qualified to comment on the contents of this patch, but everything builds and tests correctly in an out-of-tree build.

Thanks for checking, Pete.

I have a few comments/questions.

Could this have been better as more than one patch? Box, symbolmap in one, RTBuilder in another, and corray in another?

flang/include/flang/Lower/Support/BoxValue.h
176–177

RangeBoxValue is not used.
Also, is the name a misnomer since it has nothing to do with the abstract box classes?

flang/lib/Lower/Coarray.cpp
15–16

Are these two headers needed here?

flang/lib/Lower/RTBuilder.h
28

should this header be in the include directory?

flang/lib/Lower/SymbolMap.h
129
schweitz marked 4 inline comments as done.Jun 18 2020, 2:29 PM

I have a few comments/questions.

Could this have been better as more than one patch? Box, symbolmap in one, RTBuilder in another, and corray in another?

Indeed, I really did consider that. With this one, I just went with increasing the scope a bit to include four smaller support files that will be need to be merged to start upstreaming future modules. It is a change of pace from the eyedropper approach.

Trying to figure out what size patch is too big or too small is always a problem. If this patch must be broken down into 4 or 5 much smaller pieces, it can be abandoned.

flang/include/flang/Lower/Support/BoxValue.h
176–177

Nice spot.

It will be used. It's included now rather than create fussy merge conflicts.

flang/lib/Lower/Coarray.cpp
15–16

Technically, no.

On the other hand, it's hard to imagine how one might lower a coarray without access to symbols or calls to runtime libraries with runtime library building code.

flang/lib/Lower/RTBuilder.h
28

It's only germane to lowering itself, so it is better not to expose it.

flang/lib/Lower/SymbolMap.h
129

Thanks. I'll get rid of these.

schweitz marked an inline comment as done.Jun 18 2020, 2:33 PM
schweitz added inline comments.
flang/lib/Lower/RTBuilder.h
28

I misunderstood the question. My bad.

Flang has some pieces stuck in different places. The runtime is one of those.

schweitz abandoned this revision.Jun 18 2020, 4:18 PM