subject:"\[PATCH\] D95460\: \[flang\]\[driver\] Add forced form flags and \-ffixed\-line\-length"

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-03-29 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

Hi @protze.joachim ,

Interestingly, GCC does not _error_  when using `gfortran` flags:

  $ gcc -ffree-form file.c
  cc1: warning: command line option ‘-ffree-form’ is valid for Fortran but not 
for C

I've implemented similar behavior for Clang in https://reviews.llvm.org/D99353. 
Please, could you take a look and see whether that solves your problem?

-Andrzej


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-03-19 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

Thank you for you reply Joachim,

I see 3 options here:

**OPTION 1:** make Clang accept these options
This would be as simple as removing the `FlangOnlyOption` from the 
corresponding TableGen definitions in Options.td. However, since we added help 
texts to their definitions, these options would be displayed together with 
other options when using `clang --help`. Given that these options are _not 
supported_ by `clang`, I think that we should avoid this upstream. However, 
this could be a quick and easy fix for you downstream. More on `clang -help` in 
this RFC .

**OPTION 2:** tweak the driver
From your description, it seems like it would make more sense to tweak the 
driver so that it can work in .e.g _linker_ mode. In such a mode, it would skip 
the compilation phase and ignore the compilation flags altogether. IMHO this 
would be the most sound approach here, but it may require a bit of work. I 
would start by asking on cfe-dev - it's likely that more projects use `clang` 
as a linker driver. Perhaps it's already possible?

**OPTION 3:** tweak ` -Wno-unknown-argument`
I would expect ` -Wno-unknown-argument` to work here as you initially 
suggested. I'm not familiar with its semantics or e.g. DiagnosticDriverKinds.td 

 (there's a lot in there!). But sometimes you can tweak these things with a 
one-line change and then stuff auto-magically works. I can take a look at some 
point next week. It might be a low hanging fruit or a dead end.

What are you thoughts?

Best,
Andrzej


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-03-18 Thread Joachim Protze via Phabricator via cfe-commits

protze.joachim added a comment.

Hi Andrzej,

thanks for the detailed insights. I think I really misunderstood, what the -W 
flags can do here. I also was not up-to-date regarding the state of the flang 
implementation.

I just found, that compiling Spec OMP 2012 with clang-12 worked fine with my 
configuration, but compiling with current HEAD failed. Bisecting brought me to 
this patch.
What my build configuration basically does is to use gfortran to compile 
Fortran source to object files. Then I use clang/clang++ to link all the object 
files, basically:

  gfortran -c foo.f
  clang -lgfortran --gcc-toolchain=$(dirname $(dirname $(which gcc))) foo.o

In the build configuration, I can set FPORTABILITY flags for specific apps, but 
these flags are unfortunately passed to both the compile and the link step. 
Setting FLD to clang used to work, and now it broke.

One of my reasons for linking with clang is to make sure, that as much LLVM 
libraries as possible are linked. Especially, I want to pick up the LLVM 
version of the ThreadSanitizer runtime.

I tried to change my build configuration to use flang. After fixing some of the 
code (removing save keyword from variables, when having a global save), I could 
successfully compile one of the codes. Since flang under the hood uses gfortran 
for linking, this configuration picks up the GNU version of the ThreadSanitizer 
runtime. The GNU runtime is typically built as a dynamic library and comes with 
~30-40% performance penalty. So, even when compiling with flang, I would prefer 
to link using clang.

I have no idea about the compiler internals, but would it be possible to mark 
the flang flags as known to the compiler tool-chain, but not used in case of 
clang? Finally, this would result in a `-Wunused-command-line-argument` warning.

Best
Joachim


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-03-18 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

Hi @protze.joachim,

Thank you for your feedback. I'm sorry that this is causing issues in your 
set-up.

In D95460#2632844 , @protze.joachim 
wrote:

> Before this patch, clang would happily ignore a `-ffree-form` flag.

It's a bit more nuanced than this. Originally, `-ffree-form` (and other similar 
flags) were added to be forwarded to `gfortran`. This forwarding was 
effectively switched off ~6 months before this patch, please see this commit 
. As such, 
these flags stopped being used/needed by Clang. This patch makes Flang 
"re-claim" them.

> With this patch, none of `-Wno-error=unknown-argument`, 
> `-Wno-unknown-argument` , `-Qunused-arguments` help to avoid clang from 
> exiting with
>
>   error: unknown argument: '-ffree-form'
>
> Why can't clang ignore these flags as any other unknown flags?

IIUC, this is not something specific to the options refactored in this patch. 
For example:

  # Test -foo
  clang-cl -c -Wno-unknown-argument -foo  file.c
  clang -c -Wno-unknown-argument -foo  file.c
  clang-13: error: unknown argument: '-foo'
  # Test -ffree-form
  clang-cl -c -Wno-unknown-argument -ffree-form  file.c
  clang -c -Wno-unknown-argument -ffree-form  file.c
  clang-13: error: unknown argument: '-ffree-form'

Basically, `-Wno-unknown-argument` is only honored in `clang-cl`. Also, it 
applies to any option rather then just the options modified here.
I'm not particularly familiar with the semantics of the diagnostics options 
that you listed (`-Wno-error=unknown-argument`, `-Wno-unknown-argument` , 
`-Qunused-arguments`), but I get the impression that in general `clang` does 
not ignore options that it does not know about. I couldn't find any example 
that would demonstrate otherwise.

> As a background: in the build system I'm dealing with, I cannot avoid that 
> fortran flags are passed to the linking command. As C++ and fortran is 
> involved, I prefer using clang++ as the linking command and explicitly link 
> the fortran runtime library (at the moment gfortran, but in the future 
> probably the flang runtime library)

It sounds like your build system relies on the `gfortran` support in Clang, but 
this support has been "bit rotting 
". I'm 
keen to help you resolve your problems, but finding a solution that will work 
for `clang`, `flang` and `gfortran` might require some effort. Would you be 
able to step-up as Clang's `gfortran`-mode maintainer? (no pressure, just 
brainstorming!)
Alternatively, do you need ToT `clang` for your build system?

Thank you,
-Andrzej

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-03-17 Thread Joachim Protze via Phabricator via cfe-commits

protze.joachim added a comment.

Before this patch, clang would happily ignore a `-ffree-form` flag. With this 
patch, none of `-Wno-error=unknown-argument`, `-Wno-unknown-argument` , 
`-Qunused-arguments` help to avoid clang from exiting with

  error: unknown argument: '-ffree-form'

Why can't clang ignore these flags as any other unknown flags?

As a background: in the build system I'm dealing with, I cannot avoid that 
fortran flags are passed to the linking command. As C++ and fortran is 
involved, I prefer using clang++ as the linking command and explicitly link the 
fortran runtime library (at the moment gfortran, but in the future probably the 
flang runtime library)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-02-04 Thread Faris via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a1513c142f4: [flang][driver] Add forced form flags and 
-ffixed-line-length (authored by FarisRehman).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/Inputs/fixed-form-test.f
  flang/test/Flang-Driver/Inputs/fixed-line-length-test.f
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fixed-free-flag.f90
  flang/test/Flang-Driver/fixed-line-length.f90

Index: flang/test/Flang-Driver/fixed-line-length.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-line-length.f90
@@ -0,0 +1,56 @@
+! Ensure argument -ffixed-line-length=n works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -E -ffixed-line-length=-2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -E -ffixed-line-length=3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -E -ffixed-line-length=none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length=0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length=13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: %flang-new -fc1 -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length=-2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length=3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length=none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length=0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length=13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!-
+! COMMAND ALIAS -ffixed-line-length-n
+!-
+! RUN: %flang-new -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+! RUN: %flang-new -fc1 -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!-
+! EXPECTED OUTPUT WITH DEFAULT LENGTH
+!-
+! The line should be trimmed to 72 characters when reading based on the default value of fixed line length.
+! DEFAULTLENGTH: program{{(a{58})}}
+
+!-
+! EXPECTED OUTPUT WITH A NEGATIVE LENGTH
+!-
+! NEGATIVELENGTH: invalid value '-2' in 'ffixed-line-length=','value must be 'none' or a non-negative integer'
+
+!-
+! EXPECTED OUTPUT WITH LENGTH LESS THAN 7
+!-
+! INVALIDLENGTH: invalid value '3' in 'ffixed-line-length=','value must be at least seven'
+
+!---
+! EXPECTED OUTPUT WITH UNLIMITED LENGTH
+!---
+! The line should not be trimmed and so 73 characters (including spaces) should be read.
+! UNLIMITEDLENGTH: program{{(a{59})}}
+
+!
+! EXPECTED OUTPUT WITH LENGTH 13
+!
+! LENGTH13: program
Index: flang/test/Flang-Driver/fixed-free-flag.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-free-flag.f90
@@ -0,0 +1,25 @@
+! Ensure arguments -ffree-form and -ffixed-form work as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fsyntax-only -ffree-form %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-02-03 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski accepted this revision.
awarzynski added a comment.
This revision is now accepted and ready to land.

Thank you for working on this @FarisRehman !


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-02-02 Thread Faris via Phabricator via cfe-commits

FarisRehman updated this revision to Diff 320811.
FarisRehman marked an inline comment as done.
FarisRehman added a comment.

Address review comment

This revision addresses a review comment by @tskeith

Summary of changes:

- Add a new option, ffixed_line_length_EQ, that defines `-ffixed-line-length=`
- Make option ffixed_line_length_VALUE (`-ffixed-line-length-`) an alias of 
ffixed_line_length_EQ, to be consistent with gfortran
- Add flag FlangOnlyOption to ffixed-form, ffree-form and ffixed-line-length


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/Inputs/fixed-form-test.f
  flang/test/Flang-Driver/Inputs/fixed-line-length-test.f
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fixed-free-flag.f90
  flang/test/Flang-Driver/fixed-line-length.f90

Index: flang/test/Flang-Driver/fixed-line-length.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-line-length.f90
@@ -0,0 +1,56 @@
+! Ensure argument -ffixed-line-length=n works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -E -ffixed-line-length=-2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -E -ffixed-line-length=3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -E -ffixed-line-length=none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length=0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length=13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: %flang-new -fc1 -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length=-2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length=3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length=none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length=0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length=13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!-
+! COMMAND ALIAS -ffixed-line-length-n
+!-
+! RUN: %flang-new -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+! RUN: %flang-new -fc1 -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!-
+! EXPECTED OUTPUT WITH DEFAULT LENGTH
+!-
+! The line should be trimmed to 72 characters when reading based on the default value of fixed line length.
+! DEFAULTLENGTH: program{{(a{58})}}
+
+!-
+! EXPECTED OUTPUT WITH A NEGATIVE LENGTH
+!-
+! NEGATIVELENGTH: invalid value '-2' in 'ffixed-line-length=','value must be 'none' or a non-negative integer'
+
+!-
+! EXPECTED OUTPUT WITH LENGTH LESS THAN 7
+!-
+! INVALIDLENGTH: invalid value '3' in 'ffixed-line-length=','value must be at least seven'
+
+!---
+! EXPECTED OUTPUT WITH UNLIMITED LENGTH
+!---
+! The line should not be trimmed and so 73 characters (including spaces) should be read.
+! UNLIMITEDLENGTH: program{{(a{59})}}
+
+!
+! EXPECTED OUTPUT WITH LENGTH 13
+!
+! LENGTH13: program
Index: flang/test/Flang-Driver/fixed-free-flag.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-free-flag.f90
@@ -0,0 +1,25 @@
+! Ensure arguments

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-01-29 Thread Tim Keith via Phabricator via cfe-commits

tskeith added inline comments.



Comment at: clang/include/clang/Driver/Options.td:4133
 def fconvert_EQ : Joined<["-"], "fconvert=">, Group;
-def ffixed_line_length_VALUE : Joined<["-"], "ffixed-line-length-">, 
Group;
+def ffixed_line_length_VALUE : Joined<["-"], "ffixed-line-length-">, 
Flags<[FlangOption, FC1Option]>,
+  Group, HelpText<"Use  as character line width in 
fixed mode">,

Why isn't this `-ffixed-line-length=` (i.e. with an equals sign, not a 
hyphen)? Isn't that how clang does options with values? It's fine to support 
the gfortran form as an alternative but I think the primary goal should be good 
consistent style for options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-01-29 Thread Faris via Phabricator via cfe-commits

FarisRehman updated this revision to Diff 320145.
FarisRehman added a comment.

Remove -fno-fixed-form and -fno-free-form

Remove options `-fno-fixed-form` and `-fno-free-form` allowing for help 
messages to be added to `-ffixed-form` and `-ffree-form`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/Inputs/fixed-form-test.f
  flang/test/Flang-Driver/Inputs/fixed-line-length-test.f
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fixed-free-flag.f90
  flang/test/Flang-Driver/fixed-line-length.f90

Index: flang/test/Flang-Driver/fixed-line-length.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-line-length.f90
@@ -0,0 +1,50 @@
+! Ensure argument -ffixed-line-length-n works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -E -ffixed-line-length--2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -E -ffixed-line-length-3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: %flang-new -fc1 -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length--2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length-3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!-
+! EXPECTED OUTPUT WITH DEFAULT LENGTH
+!-
+! The line should be trimmed to 72 characters when reading based on the default value of fixed line length.
+! DEFAULTLENGTH: program{{(a{58})}}
+
+!-
+! EXPECTED OUTPUT WITH A NEGATIVE LENGTH
+!-
+! NEGATIVELENGTH: invalid value '-2' in 'ffixed-line-length-','value must be 'none' or a non-negative integer'
+
+!-
+! EXPECTED OUTPUT WITH LENGTH LESS THAN 7
+!-
+! INVALIDLENGTH: invalid value '3' in 'ffixed-line-length-','value must be at least seven'
+
+!---
+! EXPECTED OUTPUT WITH UNLIMITED LENGTH
+!---
+! The line should not be trimmed and so 73 characters (including spaces) should be read.
+! UNLIMITEDLENGTH: program{{(a{59})}}
+
+!
+! EXPECTED OUTPUT WITH LENGTH 13
+!
+! LENGTH13: program
Index: flang/test/Flang-Driver/fixed-free-flag.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-free-flag.f90
@@ -0,0 +1,25 @@
+! Ensure arguments -ffree-form and -ffixed-form work as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fsyntax-only -ffree-form %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s --check-prefix=FREEFORM
+! RUN: %flang-new -fsyntax-only -ffixed-form %S/Inputs/free-form-test.f90 2>&1 | FileCheck %s --check-prefix=FIXEDFORM
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: not %flang-new -fc1 -fsyntax-only -ffree-form %S/Inputs/fixed-form-test.f  2>&1 | FileCheck

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-01-29 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

@FarisRehman, thank you for addressing my comments! I've just realised that 
`gfortran` doesn't actually support `-fno-fixed-form` or `-fno-free-form`:

  $ gfortran -ffixed-form -fno-fixed-form test.f
  gfortran: error: unrecognized command line option ‘-fno-fixed-form’

I've checked the other spellings too. If that's the case then perhaps the 
definitions in `Options.td` should be updated to reflect that? And if we don't 
use `BooleanFFlag` for these options then it should be much easier to add a 
help text too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-01-28 Thread Faris via Phabricator via cfe-commits

FarisRehman marked an inline comment as done.
FarisRehman added inline comments.



Comment at: flang/lib/Frontend/CompilerInstance.cpp:151-158
+  if (invoc.frontendOpts().fortranForm_ == FortranForm::Unknown) {
+// Switch between fixed and free form format based on the input file
+// extension. Ideally we should have all Fortran options set before
+// entering this loop (i.e. processing any input files). However, we
+// can't decide between fixed and free form based on the file extension
+// earlier than this.
+invoc.fortranOpts().isFixedForm = fif.IsFixedForm();

awarzynski wrote:
> Hm, unwanted TABs? 
> 
> Also, please keep note of: https://reviews.llvm.org/D95464 (there should be 
> no conflicts from what I can tell).
I believe this is just Phabricator showing the comments have been indented


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-01-28 Thread Faris via Phabricator via cfe-commits

FarisRehman updated this revision to Diff 319842.
FarisRehman marked 12 inline comments as done.
FarisRehman added a comment.

Address review comment

Address the review comment by @awarzynski

Summary of changes:

- Shorten help text
- Add method AddFortranDialectOptions to Flang.cpp
- Change style of code to use quaternary operator
- Update regression tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95460/new/

https://reviews.llvm.org/D95460

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/Inputs/fixed-form-test.f
  flang/test/Flang-Driver/Inputs/fixed-line-length-test.f
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fixed-free-flag.f90
  flang/test/Flang-Driver/fixed-line-length.f90

Index: flang/test/Flang-Driver/fixed-line-length.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-line-length.f90
@@ -0,0 +1,50 @@
+! Ensure argument -ffixed-line-length-n works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -E -ffixed-line-length--2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -E -ffixed-line-length-3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: %flang-new -fc1 -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length--2 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=NEGATIVELENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length-3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!-
+! EXPECTED OUTPUT WITH DEFAULT LENGTH
+!-
+! The line should be trimmed to 72 characters when reading based on the default value of fixed line length.
+! DEFAULTLENGTH: program{{(a{58})}}
+
+!-
+! EXPECTED OUTPUT WITH A NEGATIVE LENGTH
+!-
+! NEGATIVELENGTH: invalid value '-2' in 'ffixed-line-length-','value must be 'none' or a non-negative integer'
+
+!-
+! EXPECTED OUTPUT WITH LENGTH LESS THAN 7
+!-
+! INVALIDLENGTH: invalid value '3' in 'ffixed-line-length-','value must be at least seven'
+
+!---
+! EXPECTED OUTPUT WITH UNLIMITED LENGTH
+!---
+! The line should not be trimmed and so 73 characters (including spaces) should be read.
+! UNLIMITEDLENGTH: program{{(a{59})}}
+
+!
+! EXPECTED OUTPUT WITH LENGTH 13
+!
+! LENGTH13: program
Index: flang/test/Flang-Driver/fixed-free-flag.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-free-flag.f90
@@ -0,0 +1,25 @@
+! Ensure arguments -ffree-form and -ffixed-form work as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fsyntax-only -ffree-form %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s --check-prefix=FREEFORM
+! RUN: %flang-new -fsyntax-only -ffixed-form %S/Inputs/free-form-test.f90 2>&1 | FileCheck %s --check-prefix=FIXEDFORM
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-01-27 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

Thank you for working on this @FarisRehman ! I've left a few inline comments, 
but nothing major. Really nice to see this being added and working as expected!




Comment at: clang/include/clang/Driver/Options.td:4074-4076
+def ffixed_line_length_VALUE : Joined<["-"], "ffixed-line-length-">, 
Flags<[FlangOption, FC1Option]>, 
+  HelpText<"Set column after which characters are ignored in typical 
fixed-form lines in the source file">,
+  Group;

This is a fairly long help message. This is what `gfortran` prints:
```
gfortran --help=joined | grep 'ffixed-line-length'
  -ffixed-line-length- Use n as character line width in fixed mode
```
A long version could be added using `DocBrief` field.



Comment at: clang/include/clang/Driver/Options.td:4110-4111
 defm f2c : BooleanFFlag<"f2c">, Group;
-defm fixed_form : BooleanFFlag<"fixed-form">, Group;
-defm free_form : BooleanFFlag<"free-form">, Group;
+defm fixed_form : BooleanFFlag<"fixed-form">, Flags<[FlangOption, FC1Option]>, 
Group;
+defm free_form : BooleanFFlag<"free-form">, Flags<[FlangOption, FC1Option]>, 
Group;
 defm frontend_optimize : BooleanFFlag<"frontend-optimize">, 
Group;

We can't have help text for these, can we? Perhaps worth mentioning in the 
commit message that this needs fixing. We are likely to experience this with 
other options too.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:24-26
+  Args.AddAllArgs(CmdArgs, {options::OPT_D, options::OPT_U, options::OPT_I,
+options::OPT_ffixed_form, options::OPT_ffree_form,
+options::OPT_ffixed_line_length_VALUE});

IIUC, you are adding Fortran dialect rather then preprocessor options here. See 
also `gfortran` docs: 
https://gcc.gnu.org/onlinedocs/gfortran/Fortran-Dialect-Options.html.



Comment at: flang/include/flang/Frontend/FrontendOptions.h:77-87
+enum class FortranForm {
+  /// The user has not specified a form. Base the form off the file extension.
+  Unknown,
+
+  /// -ffree-form
+  FixedForm,
+

[nit] Perhaps `Source file layout` above `enum class FortranForm`?



Comment at: flang/lib/Frontend/CompilerInstance.cpp:151-158
+  if (invoc.frontendOpts().fortranForm_ == FortranForm::Unknown) {
+// Switch between fixed and free form format based on the input file
+// extension. Ideally we should have all Fortran options set before
+// entering this loop (i.e. processing any input files). However, we
+// can't decide between fixed and free form based on the file extension
+// earlier than this.
+invoc.fortranOpts().isFixedForm = fif.IsFixedForm();

Hm, unwanted TABs? 

Also, please keep note of: https://reviews.llvm.org/D95464 (there should be no 
conflicts from what I can tell).



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:166-171
+if (currentArg->getOption().matches(
+clang::driver::options::OPT_ffixed_form)) {
+  opts.fortranForm_ = FortranForm::FixedForm;
+} else {
+  opts.fortranForm_ = FortranForm::FreeForm;
+}

Could be simplified using the ternary operator:
```
opts.fortranForm_ = 
(currentArg->getOption().matches(clang::driver::options::OPT_ffixed_form) ? 
FortranForm::FixedForm : FortranForm::FreeForm;
```
To me this would be more concise, but semantically it's identical. So please 
decide!

(similar comment for the bit below)



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:182
+  columns = 0;
+} else if (argValue.getAsInteger(10, columns)) {
+  columns = -1;

Please, could you comment hard-coded args:
```
 } else if (argValue.getAsInteger(/*Radix=*/10, columns)) {
```
See [[ https://llvm.org/docs/CodingStandards.html#comment-formatting | LLVM's 
coding style ]]



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:319-323
+  if (frontendOptions.fortranForm_ == FortranForm::FixedForm) {
+fortranOptions.isFixedForm = true;
+  } else if (frontendOptions.fortranForm_ == FortranForm::FreeForm) {
+fortranOptions.isFixedForm = false;
+  }

Hm, wouldn't this work:
```
fortranOptions.isFixedForm = (frontendOptions.fortranForm_ == 
FortranForm::FixedForm) ? true : false;
 ```



Comment at: flang/test/Flang-Driver/Inputs/fixed-line-length-test.f:4
+  end
\ No newline at end of file


FIXME



Comment at: flang/test/Flang-Driver/fixed-free-flag.f90:8-9
+!--
+! RUN: not %flang-new -fsyntax-only -ffree-form %S/Inputs/fixed-form-test.f 
%S/Inputs/free-form-test.f90  2>&1 | FileCheck %s --check-prefix=FREEFORM
+! RUN: %flang-new -fsyntax-only -ffixed-form %S/Inputs/free-form-test.f90 
%S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

2021-01-26 Thread Faris via Phabricator via cfe-commits

FarisRehman created this revision.
Herald added a reviewer: sscalpone.
Herald added a subscriber: dang.
Herald added a reviewer: awarzynski.
Herald added a reviewer: jansvoboda11.
FarisRehman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add support for the following layout options:

- -ffree-form
- -ffixed-form
- -ffixed-line-length-n

The default fixed line length is 72, based off the current default.

This patch does not add `-ffree-line-length-n` as Fortran::parser::Options does 
not have a variable for free form columns.
Whilst `fixedFormColumns` variable is used in f18 for `-ffree-line-length-n`, 
f18 only allows `-ffree-line-length-none`/`-ffree-line-length-0` and not a 
user-specified value. `fixedFormcolumns` cannot be used in the new driver as it 
is ignored in the frontend when dealing with free form files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95460

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/Inputs/fixed-form-test.f
  flang/test/Flang-Driver/Inputs/fixed-line-length-test.f
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fixed-free-flag.f90
  flang/test/Flang-Driver/fixed-line-length.f90

Index: flang/test/Flang-Driver/fixed-line-length.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-line-length.f90
@@ -0,0 +1,43 @@
+! Ensure argument -ffixed-line-length-n works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -E -ffixed-line-length-3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: %flang-new -fc1 -E %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=DEFAULTLENGTH
+! RUN: not %flang-new -fc1 -E -ffixed-line-length-3 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=INVALIDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-none %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-0 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=UNLIMITEDLENGTH
+! RUN: %flang-new -fc1 -E -ffixed-line-length-13 %S/Inputs/fixed-line-length-test.f  2>&1 | FileCheck %s --check-prefix=LENGTH13
+
+!-
+! EXPECTED OUTPUT WITH DEFAULT LENGTH
+!-
+! The line should be trimmed to 72 characters when reading based on the default value of fixed line length.
+! DEFAULTLENGTH: programaa
+
+!-
+! EXPECTED OUTPUT WITH LENGTH LESS THAN 7
+!-
+! INVALIDLENGTH: invalid value '{{(-)?[0-9]+}}'
+
+!---
+! EXPECTED OUTPUT WITH UNLIMITED LENGTH
+!---
+! The line should not be trimmed and so 73 characters (including spaces) should be read.
+! UNLIMITEDLENGTH: programaaa
+
+!
+! EXPECTED OUTPUT WITH LENGTH 13
+!
+! LENGTH13: program
Index: flang/test/Flang-Driver/fixed-free-flag.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fixed-free-flag.f90
@@ -0,0 +1,25 @@
+! Ensure arguments -ffree-form and -ffixed-form work as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fsyntax-only -ffree-form %S/Inputs/fixed-form-test.f %S/Inputs/free-form-test.f90  2>&1 | FileCheck %s --check-prefix=FREEFORM
+! RUN: %flang-new -fsyntax-only -ffixed-form %S/Inputs/free-form-test.f90 %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s --check-prefix=FIXEDFORM
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

15 matches

Site Navigation

Mail list logo

Footer information