Page MenuHomePhabricator

[flang][driver] Add standard predefinitions
AcceptedPublic

Authored by FarisRehman on Tue, Jan 12, 9:37 AM.

Details

Summary

Add the following standard predefinitions that f18 supports: __flang__, __flang_major__, __flang__minor__, __flang_patchlevel__

Summary of changes:

  • Populate Fortran::parser::Options#predefinitions with the default supported predefinitions

Depends on: D94422

Diff Detail

Event Timeline

FarisRehman created this revision.Tue, Jan 12, 9:37 AM
FarisRehman requested review of this revision.Tue, Jan 12, 9:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Jan 12, 9:37 AM
FarisRehman edited the summary of this revision. (Show Details)Tue, Jan 12, 9:38 AM
sameeranjoshi added inline comments.Thu, Jan 14, 7:18 AM
flang/lib/Frontend/CompilerInvocation.cpp
272

Why is this 1 here?
Also why only x86_64, why don't we add other architectures here?

flang/test/Flang-Driver/compiler-defined-macros.f90
25

How do you test __x86_64__ ?
I think we can add architecture specific tests in lit based test cases, does that make sense to add them here?

Thank you for working on this @FarisRehman !

@FarisRehman Re __X86_64__, in my opinion it is too early for any target-specific predefinitions and I suggest that we remove it for now. Lets revisit in the future when code-generation is supported.

@sameeranjoshi How does it sound? I totally agree with you that once we start adding such predefinitions, we should provide a solution that will support multiple architectures.

flang/lib/Frontend/CompilerInvocation.cpp
263

[nit] IMO this is fine for now, but long term the list of standard predefinitions is likely to grow significantly. Could add a TODO here? e.g.

// TODO: When expanding this list of standard predefinitions, consider creating a dedicated API for this. Also, at some point we will need to differentiate between different targets.

Agreed. @awarzynski. Thank you for clarification.

FarisRehman updated this revision to Diff 317321.EditedMon, Jan 18, 3:46 AM
FarisRehman marked 3 inline comments as done.

Remove target-specific predefinition

Remove a target-specific predefinition (__x86_64__), addressing review comments by @awarzynski and @sameeranjoshi

Summary of changes:

  • Remove predefinition __x86_64__
  • Add __flang__ to predefinitions regression test
FarisRehman edited the summary of this revision. (Show Details)Mon, Jan 18, 3:47 AM
sameeranjoshi accepted this revision.Mon, Jan 18, 5:11 AM

Thank you.
LGTM.

This revision is now accepted and ready to land.Mon, Jan 18, 5:11 AM