This is an archive of the discontinued LLVM Phabricator instance.

[flang] Allow all OSes in fir::CodeGenSpecifics::get
ClosedPublic

Authored by MaskRay on Oct 3 2022, 1:41 PM.

Details

Summary

This allows all ELF operating systems to use target specifics tuned for Linux,
since they use mostly the same ABIs. If some triples are to excluded, it's
better done at the driver layer.

Diff Detail

Event Timeline

MaskRay created this revision.Oct 3 2022, 1:41 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 3 2022, 1:41 PM
MaskRay requested review of this revision.Oct 3 2022, 1:41 PM
schweitz added inline comments.Oct 4 2022, 3:25 PM
flang/lib/Optimizer/CodeGen/Target.cpp
348–349

Noting that the project may want the flexibility to create distinctions (or react to distinctions) between platforms at the OS level at some point. This code is largely focused on complex at the moment, but that may expand to other cases. Consider, for example, supporting different descriptor formats, different runtimes, etc.

For what it is worth, flang was originally written to be strict rather than permissive. Checks are made to encode things like "this configuration has been tested" rather than just letting everything fall through and sifting through the fallout after the fact. Broadly speaking, the intention was for the compiler to produce known correct output/code when it could. If something was untested, unsupported, unimplemented, etc. however, the compiler was to give a reasonable error message rather than a possibly false impression that everything might have been ok or not.

For what it is worth, flang was originally written to be strict rather than permissive. Checks are made to encode things like "this configuration has been tested" rather than just letting everything fall through and sifting through the fallout after the fact. Broadly speaking, the intention was for the compiler to produce known correct output/code when it could. If something was untested, unsupported, unimplemented, etc. however, the compiler was to give a reasonable error message rather than a possibly false impression that everything might have been ok or not.

The strictness is usually done in the driver, not in codegen.
This file has been touched by many groups adding various not-Linux systems. I think it should be open for all ELF OSes if an arch supports Linux.

Technically I can use a macOS/Windows/ELF dispatch but macOS/Windows aren't supported on many architectures anyway (LLVM doesn't allow) so I don't know whether that is useful.

emaste added a comment.Oct 4 2022, 4:19 PM

Checks are made to encode things like "this configuration has been tested"

This unfortunately introduces hurdles in the path of testing additional configurations.

I wouldn't object to a warning along the lines of "This OS/CPU architecture combination has not been extensively tested" but disallowing it completely in codegen makes it harder to transition to tested and supported.

I wasn't objecting. I was informing. (Flang did not even have a driver for a long, long time.)

Then wish I can get a stamp :)

emaste accepted this revision.Oct 17 2022, 12:24 PM

LGTM for FreeBSD

This revision is now accepted and ready to land.Oct 17 2022, 12:24 PM

I'll wait a bit and push this change.

MaskRay marked an inline comment as done.Oct 24 2022, 10:42 PM
MaskRay added inline comments.
flang/lib/Optimizer/CodeGen/Target.cpp
348–349

At that point, make the check at the driver, not in codegen:)

This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as done.