peixin added a comment.
Currently, I cannot test the option `-fconvert=big-endian` with open statement
with convert argument using the following code since lowering is not supported.
I am not sure if it can be tested in runtime.
$ cat fconvert_option_openfile.f90
module fconvert_option_openfile
contains
subroutine open_for_read(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='old')
end
subroutine open_for_read_BE(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='old',
convert="BIG_ENDIAN")
end
subroutine open_for_read_LE(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='old',
convert="LITTLE_ENDIAN")
end
subroutine open_for_read_native(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='old',
convert="NATIVE")
end
subroutine open_for_write(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='new')
end
subroutine open_for_write_BE(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='new',
convert="BIG_ENDIAN")
end
subroutine open_for_write_LE(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='new',
convert="LITTLE_ENDIAN")
end
subroutine open_for_write_native(fid, file_name)
integer :: fid
character(:), allocatable :: file_name
open (fid, file=file_name, form='UNFORMATTED', status='new',
convert="NATIVE")
end
end
$ cat fconvert_option_readfile.f90
module fconvert_option_readfile
contains
subroutine readfile(fid, del_flag, expect, n, t)
integer :: n, t
integer :: fid, del_flag
real :: expect(n)
real :: buf(n)
read (fid) buf
if (del_flag .eq. 0) then
close (fid)
else
close (fid, status='delete')
end if
do i = 1, n
if (buf(i) .ne. expect(i)) stop (i+t)
enddo
end
end
$ cat fconvert_option_main_1.f90
program fconvert_option_main_1
use fconvert_option_openfile
use fconvert_option_readfile
integer, parameter :: n = 4
integer :: del_flag = 0 ! 1 for deleting data file after read
real :: arr(n) = [1.0, 2.0, 3.0, 4.0]
character(:), allocatable :: filename
integer :: arglen
call get_command_argument(1, length = arglen)
allocate(character(arglen) :: filename)
call get_command_argument(1, value = filename)
call open_for_read(10, filename)
call readfile(10, del_flag, arr, n, 0)
call open_for_read_LE(11, filename)
call readfile(11, del_flag, arr, n, 4)
call open_for_read_native(12, filename)
call readfile(12, del_flag, arr, n, 8)
print *, 'PASS'
end
BTW, can you continue working on the lowering of the convert argument of open
statement?
================
Comment at: clang/include/clang/Driver/Options.td:4897
def ffixed_line_length_VALUE : Joined<["-"], "ffixed-line-length-">,
Group<f_Group>, Alias<ffixed_line_length_EQ>;
+def fconvert_EQ : Joined<["-"], "fconvert=">, Group<f_Group>,
+ HelpText<"Set endian conversion of data for unformatted files">;
----------------
awarzynski wrote:
> jpenix-quic wrote:
> > peixin wrote:
> > > Why do you move it here? Maybe it is not implemented now, clang may need
> > > this option eventually. @MaskRay
> > I was using the fixed line length options as a reference for how to handle
> > this--based on the discussion in the review here
> > (https://reviews.llvm.org/D95460) about forwarding options to gfortran, I
> > was thinking that it would also be safe to handle fconvert similarly, but
> > I'm not 100% sure and definitely might be misunderstanding something!
> GFortran support has been effectively broken since LLVM 12 (see e.g. this [[
> https://github.com/llvm/llvm-project/commit/6a75496836ea14bcfd2f4b59d35a1cad4ac58cee
> | change ]]). We would do ourselves a favor if we just removed it altogether
> (I'm not aware of anyone requiring it).
>
> And if Clang ever needs this option, we can always update this definition
> accordingly. No need to optimize for hypothetical scenarios that may or may
> not happen :) To me, this change makes perfect sense.
OK. This sounds reasonable to me.
================
Comment at: flang/lib/Lower/Bridge.cpp:2757
+ if (funit.isMainProgram()) {
+ auto conversion = bridge.getConversion();
----------------
I think this should be moved into `void
lowerFunc(Fortran::lower::pft::FunctionLikeUnit &funit)`.
================
Comment at: flang/runtime/main.cpp:55
+
+ if (auto convert{Fortran::runtime::GetConvertFromInt(i)}) {
+ Fortran::runtime::executionEnvironment.conversion = *convert;
----------------
Nit: Is it better to to check the range of i before getting it from
`GetConvertFromInt`?
```
if (i < static_cast<int>(Convert::Unknown) || i >
static_cast<int>(Convert::Swap)) {
crash
} else {
Fortran::runtime::executionEnvironment.conversion = static_cast<Convert>(i);
}
```
================
Comment at: flang/runtime/main.cpp:58
+ } else {
+ Fortran::runtime::Terminator{__FILE__, __LINE__}.Crash(
+ "Invalid convert option passed to ConvertOption");
----------------
Should `__FILE__, __LINE__` be passed as argument in lowering to point to the
file name and line of the source file? Or is this (__FILE__, __LINE__) pointing
the the file name and line of the source file?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130513/new/
https://reviews.llvm.org/D130513
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
