[flang-commits] [clang] [flang] [flang] Preserve fixed form in fc1 -x value (PR #117563)

[via flang-commits]

macurtis-amd wrote:

Thanks @banach-space and @DavidTruby for looking at this.

> banach-space: Adding new types to [Types.def](https://github.com/llvm/llvm-project/pull/117563/files#diff-b2abf750cadedc6109158e6f82b66abfaabd7c7c86c650d2a5163dc3e5fc44a7) is a fairly huge design update. I'm not saying this is approach is incorrect, but it definitely deserves a bit of discussion. Perhaps under a GitHub issue? I don't really mind.

Created issue [117712](https://github.com/llvm/llvm-project/issues/117712). We can continue discussion there if that is the right place.

> banach-space: Could you quickly summarise what's not working

>From that issue:

Here is the problematic fixed form fortran source `reduced.f`:                                                                         
```
      subroutine foo(a, b)
      if ( (a  .eq. 0) .and.
     >     (b . eq. 1) ) then
         print *, "foo"
      end if
      end subroutine
```
Prior to [9fb2db1](https://github.com/llvm/llvm-project/commit/9fb2db1e1f42ae10a9d8c1d9410b5f4e719fdac0) `flang -save-temps -c` produced `reduced.i`:
```
      subroutine foo(a, b)
      if ( (a  .eq. 0) .and.(b.eq.1))then

         print *, "foo"
      end if
      end subroutine
```
which compiles without error.

With [9fb2db1](https://github.com/llvm/llvm-project/commit/9fb2db1e1f42ae10a9d8c1d9410b5f4e719fdac0), `flang -save-temps -c` produces `reduced.i`:
```
      subroutine foo(a, b)
      if ( (a  .eq. 0) .and.(b. eq. 1)) then

         print *, "foo"
      end if
      end subroutine
```

Which produces:
```
error: Could not parse reduced.i
./reduced.f:2:31: error: expected ')'
        if ( (a  .eq. 0) .and.(b. eq. 1)) then
                                ^
...
```

In either case the commands produced by the driver look like:
```
flang-new -fc1 -E -o reduced.i -x f95-cpp-input reduced.f
flangnew -fc1 -emit-llvm-bc ... -o reduced.bc -x f95 reduced.i
...
```

>  banach-space: ... and why adding new types to Types.def solves that?

With this change, the driver produces:
```
flang-new -fc1 -E ... -o reduced.i -x f95-fixed-cpp-input reduced.f
flang-new -fc1 -emit-llvm-bc -o reduced.bc -x f95-fixed reduced.i
```
Which preserves the fact that reduced.i contains fixed form fortran.

> banach-space: Is that the only solution?

No. I initially looked at modifying [Flang::ConstructJob](https://github.com/llvm/llvm-project/blob/65c36179be68dda0d1cc5d7e5c5b312a6b52cc0e/clang/lib/Driver/ToolChains/Flang.cpp#L709) to add `-ffixed-form` to the compilation command. But I found myself having to walk back through the compilation graph to get the original filename extension so I could determine if it was fixed or free.

It worked, but seemed brittle. It does have the benefit of being more surgical.

Another solution I considered, was propagating the fixed vs free information in some way other than the type. I didn't go down this path because it seemed more natural to consider it part of the type.

FWIW, I don't have strong feelings here. I'm happy to implement the fix however you all think is best.

> DavidTruby: what is the difference between the proposed flang -fc1 -x f95-fixed and the existing flang -fc1 -ffixed-form?

> DavidTruby: In the test case in here I already don't see an error with flang -fc1 -ffixed-form, is there an example you have where this fails to compile but -x f95-fixed works?

Sorry. I should have put this in the commit description, but the issue is specific to the use of `-save-temps`. If manually invoking flang -fc1 there is no benefit to -x f95-fixed. The problem is the driver needs to know to add `-ffixed-form` when building the compilation command. As noted above, I tried doing this, but was not happy with the resulting code.


https://github.com/llvm/llvm-project/pull/117563