Page MenuHomePhabricator

[flang] Use the Flang cmake-functions to add targets.
ClosedPublic

Authored by ChinouneMehdi on Apr 15 2020, 8:59 AM.

Details

Summary

It also removes the cycle-dependency between FortranSemantics and FortranEvaluate.

Diff Detail

Event Timeline

ChinouneMehdi created this revision.Apr 15 2020, 8:59 AM
ChinouneMehdi created this object with visibility "All Users".
ChinouneMehdi created this object with edit policy "Subscribers".
Herald added a project: Restricted Project. · View Herald Transcript
PeteSteinfeld added a subscriber: PeteSteinfeld.
PeteSteinfeld removed a subscriber: PeteSteinfeld.
tskeith added inline comments.Apr 15 2020, 9:50 AM
flang/lib/Evaluate/CMakeLists.txt
35

This dependency was added here: https://reviews.llvm.org/rGa1726e65bc1fdbd2800839e48e68cb4a68cc7092
Have you verified that this change doesn't re-introduce the problem that that one was fixing?

ChinouneMehdi changed the visibility from "All Users" to "Public (No Login Required)".Apr 15 2020, 12:28 PM
ChinouneMehdi marked 2 inline comments as done.Apr 15 2020, 7:34 PM
ChinouneMehdi added inline comments.
flang/lib/Evaluate/CMakeLists.txt
35

I have verified with four configurations ( gcc-8+gold, gcc-9+gold, clang-8+lld and clang-9+lld) on Ubuntu 18.04 x86_64.
I can't be sure because I don't know about his build configuration.

tskeith added inline comments.Apr 15 2020, 8:12 PM
flang/lib/Evaluate/CMakeLists.txt
35

That sounds good to me.

This revision is now accepted and ready to land.Apr 15 2020, 9:18 PM

I don't have commit access.
Please, Someone who has commit access commit these changes.

DavidTruby accepted this revision.Apr 16 2020, 5:49 AM

@ChinouneMehdi this looks good to me and I can commit it for you, could you let me know what email address you want used?
Thanks!

DavidTruby added inline comments.Apr 16 2020, 5:51 AM
flang/lib/Evaluate/CMakeLists.txt
35

I'm not sure why it doesn't reintroduce the linker issue from there, but it doesn't seem to. I'll just double check on the configuration I used there.

DavidTruby marked an inline comment as done.Apr 16 2020, 6:04 AM
DavidTruby added inline comments.
flang/lib/Evaluate/CMakeLists.txt
35

Yep, no issue with this patch. Not sure why the issue has gone away, but it should make fixing shared library builds easier!

@ChinouneMehdi this looks good to me and I can commit it for you, could you let me know what email address you want used?
Thanks!

chinoune.mehdi@hotmail.com

flang still can't be build with -DBUILD_SHARED_LIBS=ON, at link time it fails. As of now I see following libraries libFortranDecimal libFortranCommon libFortranLower libLLVMDemangle and libLLVMSupport are getting building fine. I suspect there is some circular dependency between libFortranParser libFortranSemantics and libFortranEvaluate ?
Static build's are fine.

ChinouneMehdi added a comment.EditedApr 19 2020, 1:10 PM

flang still can't be build with -DBUILD_SHARED_LIBS=ON, at link time it fails. As of now I see following libraries libFortranDecimal libFortranCommon libFortranLower libLLVMDemangle and libLLVMSupport are getting building fine. I suspect there is some circular dependency between libFortranParser libFortranSemantics and libFortranEvaluate ?
Static build's are fine.

It's a known issue, there is a circular dependency between FortranEvaluate and FortranParser.

It's a known issue, there is a circular dependency between FortranEvaluate and FortranParser.

The circular dependency is between FortranEvaluate and FortranSemantics.