This is an archive of the discontinued LLVM Phabricator instance.

[flang][fir] Upstream FIR support changes
AbandonedPublic

Authored by schweitz on Jan 25 2021, 3:12 PM.

Details

Summary

FIR support changes.

  1. Adds context information to describe properties such as the target triple.
  2. Adds an error handler.
  3. Support for Fortran type codes.
  4. Fixes various bugs.

A history of all these sources, including all contributors, previous reviews and revisions, etc., can be found at https://github.com/flang-compiler/f18-llvm-project/

These diffs depend upon D95399.

Diff Detail

Event Timeline

schweitz created this revision.Jan 25 2021, 3:12 PM
schweitz requested review of this revision.Jan 25 2021, 3:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 25 2021, 3:12 PM
schweitz updated this revision to Diff 319915.Jan 28 2021, 10:34 AM

Updating diff chain for buildability.

A few minor comments.

flang/include/flang/Optimizer/Support/FIRContext.h
13

Is this stray or a directive to some tool?

flang/include/flang/Optimizer/Support/InternalNames.h
45

Nit: Was this previously in alphabetical order?

flang/lib/Optimizer/Support/InternalNames.cpp
102

Did we have tests for all these?

flang/lib/Optimizer/Support/KindMapping.cpp
310

Should this be 'd'?

schweitz added inline comments.Jan 28 2021, 11:13 AM
flang/include/flang/Optimizer/Support/FIRContext.h
13

It's a doxygen directive. Not sure it is needed though.

flang/include/flang/Optimizer/Support/InternalNames.h
45

Good catch. Thanks.

flang/lib/Optimizer/Support/InternalNames.cpp
102

No. I don't think we do.

flang/lib/Optimizer/Support/KindMapping.cpp
310

Another good catch. Thanks again.

mehdi_amini added inline comments.Jan 28 2021, 12:40 PM
flang/lib/Optimizer/Support/FIRContext.cpp
25

The triple seems fairly fundamental to processing a Module, it isn't clear to me that this can be an OpaqueAttr (i.e. something that should be implicitly dropped freely with no consequence). How do you manage this?

schweitz added inline comments.Jan 28 2021, 12:52 PM
flang/lib/Optimizer/Support/FIRContext.cpp
25

It is definitely important, actually essential, for conversion to LLVM IR.

Prior to that, the IR is abstract enough that it can be reasoned about without knowing the target triple precisely.

The trick here is that the driver of the code gen bridge will supply a triple. Thus, we can keep the IR and the triple loosely coupled.

I know that Alex is looking at solutions to this on the MLIR side. So it could be the case that solution can just be substituted.

mehdi_amini added inline comments.Jan 28 2021, 12:53 PM
flang/lib/Optimizer/Support/FIRContext.cpp
25

Even here why don't you set it as a string attribute which can round-trip and not be an opaque attribute?

schweitz updated this revision to Diff 319945.Jan 28 2021, 1:01 PM

changes per review comments

schweitz abandoned this revision.Jan 29 2021, 5:41 PM