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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
