[clang] [flang] [flang] Preserve fixed form in fc1 -x value (PR #117563)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 26 06:47:56 PST 2024
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
More information about the cfe-commits
mailing list