Rewrite parts of stdlib where optval could be used

[Carltoffel]

While exploring the stdlib I discovered the optval function and shortly after a routine which could have made use of it but it doesn't. Should we search for such optional arguments and replace the if-else-blocks with the optval-function?
If there are no other plans which would interfere with this, I think this should be done because it shortens the code which makes the code easier to understand.

If I find time to build the stdlib I could do the job and open a PR. Please tell me if I have to keep something in mind, otherwise I will just try it.

[gareth-nx]

Does anyone have a good understanding of the likely performance implications of using a function-call, rather than a 2-line

x = default
if(present(x_in)) x = x_in

While we would hope there was no difference, I would not be shocked if there was (just because function or subroutine calls can have overhead). So I would be cautious about these changes.

Apologies to raise this question without any evidence whatsoever. But I wonder if anyone has experience on the issue?

[leonfoks]

I thought there was always a small performance hit with things like present() because it's checked at runtime, not compile time?

[gareth-nx]

I recall seeing Steve Lionel comment at one stage that present was cheap -- whereas I could imagine that optval is more of an unknown for the compiler.

But this is all speculation.

However, putting this style of coding all throughout stdlib will (IMO) be seen as encouraging this style of programming. So it would be nice to be confident that it doesn't lead to performance problems. Or, if there are known cases where it's an issue, then we could just warn about them in the optval documentation (but perhaps not use it everywhere in stdlib).

Again, sorry I don't have the answers here.

[gareth-nx]

Ok so I've made a test here, that might help with the discussion.

It suggests there can be performance issues (factor of 2 difference), assuming there's nothing too wrong with how I've written this example. (Disclaimer: Written quickly -- there could be problems !!).

I've made a simple subroutine that either squares or cubes a number, depending on the value of an optional argument, using both optval, and present.

!
! square_or_cube_mod.f90
!
module square_or_cube_mod
    !
    ! This module contains two versions of a subroutine that square or cube a number
    ! One version uses optval, one uses if(present(...))
    !
    use stdlib_optval, only: optval
    implicit none

contains

    subroutine square_or_cube_with_optval(x, use_square)
        real, intent(inout) :: x
        logical, optional, intent(in) :: use_square
        logical :: local_use_square

        local_use_square = optval(use_square, .true.)

        if(local_use_square) then
            x = x*x
        else
            x = x*x*x
        end if

    end subroutine

    subroutine square_or_cube_with_present(x, use_square)
        real, intent(inout) :: x
        logical, optional, intent(in) :: use_square
        logical :: local_use_square

        local_use_square = .true. ! default
        if(present(use_square)) local_use_square = use_square

        if(local_use_square) then
            x = x*x
        else
            x = x*x*x
        end if

    end subroutine

end module

Here is the program that times each version

!
! run_test.f90
!
program speed_test_optval_vs_present
    use square_or_cube_mod, only: square_or_cube_with_present, square_or_cube_with_optval
    implicit none
    
    integer, parameter :: N = 1e+07, reps = 10
    real, allocatable :: x(:), x_orig(:)
    integer :: i, j, t0, t1, t_present, t_optval

    allocate(x(N), x_orig(N))

    call random_number(x_orig)

    t_present = 0
    t_optval = 0

    do j = 1, reps

        ! Case with present
        x = x_orig
        call system_clock(t0)
        do i = 1, N
            call square_or_cube_with_present(x(i), .true.)
        end do
        call system_clock(t1)
        t_present = t_present + (t1 - t0)

        ! Case with optval
        x = x_orig
        call system_clock(t0)
        do i = 1, N
            call square_or_cube_with_optval(x(i), .true.)
        end do
        call system_clock(t1)
        t_optval = t_optval + (t1 - t0)

    end do

    print*, 'Time ratio (present / optval) :',  t_present * 1.0 / t_optval

end program

Here is how I compiled it

# I define stdlib variables STDLIB_CFLAGS and STDLIB_LIBS
# source ~/setup_fortran_stdlib.sh
    
# Compile
gfortran -O3 -cpp -c $STDLIB_CFLAGS square_or_cube_mod.f90
gfortran -O3 -c $STDLIB_CFLAGS run_test.f90
# Link
gfortran -O3 -o run_test square_or_cube_mod.o run_test.o $STDLIB_LIBS

When running this I get a factor-of-2 difference in favour of the version with present

./run_test 
 Time ratio (present / optval) :  0.467796624    

Let me stress that I'm not trying to say people should never use optval, even if the above results hold. Obviously in this context the subroutine is not doing lots of work, so there's more opportunity for issues. But such things do happen, and IMO are worth considering before promoting overly uniform usage of one or other programming style.

[Carltoffel]

Could you repeat the test with the additional flag -flto, please?

[gareth-nx]

Done -- there was no change. [Edit -- this was a mistake, see thread below -- actually the performance advantage of of present was increased]

Here is the modified build script:

# I define stdlib variables STDLIB_CFLAGS and STDLIB_LIBS
# source ~/setup_fortran_stdlib.sh

# Compile
gfortran -O3 -cpp -c -flto $STDLIB_CFLAGS square_or_cube_mod.f90
gfortran -O3 -c -flto $STDLIB_CFLAGS run_test.f90
# Link
gfortran -O3 -o -flto run_test square_or_cube_mod.o run_test.o $STDLIB_LIBS 

Compiled and ran with

$ source build.sh 
$ ./run_test 
 Time ratio (present / optval) :  0.458193988    

[Carltoffel]

Are you sure the calculation is doing anything? The compiler might skip the whole loop, because you are not using x in any way. You could print (parts of) x to verify both routines are doing the same.

[gareth-nx]

Good point. Also, while checking this, I realised there was an error in my use of the -flto option.

When I do it correctly the advantages of present are even greater (about 4 or 5 times faster). I guess this is because lto cannot see inside the pre-compiled stdlib library (?).

I get the same relative performance when adding some code to try to prevent optimizations, as you suggested. For simplicity I only show that version here:

The new code is:

program speed_test_optval_vs_present
    use square_or_cube_mod, only: square_or_cube_with_present, square_or_cube_with_optval
    implicit none

    integer, parameter :: N = 1e+07, reps = 10
    real, allocatable :: x(:), x_orig(:)
    real :: random_val
    integer :: i, j, t0, t1, t_present, t_optval, random_index
    logical :: all_passed = .true.

    allocate(x(N), x_orig(N))

    call random_number(x_orig)

    t_present = 0
    t_optval = 0

    do j = 1, reps

        ! Make a random index
        call random_number(random_val)
        random_index = max(1, min(int(random_val * N), N))

        ! Case with present
        x = x_orig
        call system_clock(t0)
        do i = 1, N
            call square_or_cube_with_present(x(i), .true.)
        end do
        call system_clock(t1)
        t_present = t_present + (t1 - t0)

        ! Use the value of x in some way.
        ! Compare this with the subsequent call, and ensure they are the same.
        random_val = x(random_index)

        ! Case with optval
        x = x_orig
        call system_clock(t0)
        do i = 1, N
            call square_or_cube_with_optval(x(i), .true.)
        end do
        call system_clock(t1)
        t_optval = t_optval + (t1 - t0)

        ! Comparison with previous (to try to inhibit compiler optimization)
        if(x(random_index) /= random_val) all_passed = .false.

    end do

    print*, 'Time ratio (present / optval) :',  t_present * 1.0 / t_optval
    print*, merge('PASS', 'FAIL', all_passed)

end program

And here is the corrected build script

# I define stdlib variables STDLIB_CFLAGS and STDLIB_LIBS
# source ~/setup_fortran_stdlib.sh

# Compile
gfortran -O3 -cpp -c -flto $STDLIB_CFLAGS square_or_cube_mod.f90
gfortran -O3 -c -flto $STDLIB_CFLAGS run_test.f90
# Link
gfortran -O3 -flto -o run_test square_or_cube_mod.o run_test.o $STDLIB_LIBS 

Compiled and ran with

$ source build.sh 
$ ./run_test 
 Time ratio (present / optval) :  0.209183678    
 PASS

[jvdp1]

Are you sure the calculation is doing anything? The compiler might skip the whole loop, because you are not using x in any way. You could print (parts of) x to verify both routines are doing the same.

Using the first code provided by @gareth-nx and adding a print*,x(i) after each loop, I got a ratio around 1 when compiling with -flto -fPIC BOTH stdlib AND the test code.

When using the new code provided by @gareth-nx, the ratio is almost always 1.0000 when using the options flto -fPIC for both stdlib and @gareth-nx code.

Removing '-flto -fPIC' from the compilation of stdlib resulted in ratios similar to the ones obtained by @gareth-nx

@gareth-nx could you check if you compile stdlib with the options -lto and/or -fPIC too?

From my understanding, present() just checks if the address of the arg is "0" or not.

[awvwgk]

From my understanding, present() just checks if the address of the arg is "0" or not.

Unless the dummy argument has the value attribute, in this case it is most likely an additional descriptor passed which is checked.

[gareth-nx]

@jvdp1 I am quite ignorant of CMake and so on, but tried to change the compilation of stdlib by adding the line add_compile_options(-flto -fPIC) to CMakeLists.txt in stdlib, near where a bunch of similar options are defined.

I'm not sure if that's the right thing to do. Probably not -- now I'm getting even worse performance for optval.

$ ./run_test 
Time ratio (present / optval) :  0.152777776    
PASS

[jvdp1]

Some additions to the tests:

• stdlib is compiled with the options -O3 or -O3 -lto -fPIC.

• Compilation of the test code as follows (always):

gfortran -O3 -flto -fPIC  -fopt-info -I ~/stdlib/build/src/mod_files/ modtest.f90 newtest.f90 ~/stdlib/build/src/libfortran_stdlib.a 

• Output of gfortran when stdlib is compiled without -flto -fPIC (so only -O3):

modtest.f90:29:42: optimized: Semantic equality hit:__square_or_cube_mod_MOD_square_or_cube_with_present.part.0/3->__square_or_cube_mod_MOD_square_or_cube_with_optval.part.0/4
modtest.f90:29:42: optimized: Assembler symbol names:__square_or_cube_mod_MOD_square_or_cube_with_present.part.0/3->__square_or_cube_mod_MOD_square_or_cube_with_optval.part.0/4
optimized:  Inlined __square_or_cube_mod_MOD_square_or_cube_with_present.part.0.constprop/44 into square_or_cube_with_present.constprop/42 which now has time 7.100000 and size 12, net change
 of +0.
optimized:  Inlined __square_or_cube_mod_MOD_square_or_cube_with_present.part.0.constprop/43 into square_or_cube_with_optval.constprop/41 which now has time 20.000000 and size 15, net change
 of -7.
optimized:  Inlined square_or_cube_with_present.constprop/42 into speed_test_optval_vs_present/8 which now has time 25375.085144 and size 222, net change of -7 (cross module).
optimized:  Inlined square_or_cube_with_optval.constprop/41 into speed_test_optval_vs_present/8 which now has time 30816.329254 and size 230, net change of -7 (cross module).
newtest.f90:38:18: optimized: loop vectorized using 16 byte vectors
newtest.f90:38:18: optimized:  loop versioned for vectorization because of possible aliasing
newtest.f90:27:14: optimized: loop vectorized using 16 byte vectors
newtest.f90:25:18: optimized: loop vectorized using 16 byte vectors
newtest.f90:25:18: optimized:  loop versioned for vectorization because of possible aliasing
newtest.f90:38:18: optimized: loop with 2 iterations completely unrolled (header execution count 4541736)
newtest.f90:25:18: optimized: loop with 2 iterations completely unrolled (header execution count 4541735)
newtest.f90:6:40: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:11:29: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:11:29: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:51:74: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:52:45: optimized: basic block part vectorized using 16 byte vectors

And the ratio:

 Time ratio (present / optval) :  0.333333343    
 PASS

• Output of gfortran when stdlib is compiled with -O3 -flto -fPIC

modtest.f90:29:42: optimized: Semantic equality hit:__square_or_cube_mod_MOD_square_or_cube_with_present.part.0/3->__square_or_cube_mod_MOD_square_or_cube_with_optval.part.0/4
modtest.f90:29:42: optimized: Assembler symbol names:__square_or_cube_mod_MOD_square_or_cube_with_present.part.0/3->__square_or_cube_mod_MOD_square_or_cube_with_optval.part.0/4
optimized:  Inlined __square_or_cube_mod_MOD_square_or_cube_with_present.part.0.constprop/63 into square_or_cube_with_present.constprop/61 which now has time 7.100000 and size 12, net change
 of +0.
optimized:  Inlined __square_or_cube_mod_MOD_square_or_cube_with_present.part.0.constprop/62 into square_or_cube_with_optval.constprop/59 which now has time 20.000000 and size 15, net change
 of -7.
optimized:  Inlined optval_ll1.constprop/60 into square_or_cube_with_optval.constprop/59 which now has time 7.350000 and size 12, net change of -8 (cross module).
optimized:  Inlined square_or_cube_with_present.constprop/61 into speed_test_optval_vs_present/8 which now has time 25375.085144 and size 222, net change of -7 (cross module).
optimized:  Inlined square_or_cube_with_optval.constprop/59 into speed_test_optval_vs_present/8 which now has time 17955.206909 and size 227, net change of -7 (cross module).
newtest.f90:40:14: optimized: loop vectorized using 16 byte vectors
newtest.f90:38:18: optimized: loop vectorized using 16 byte vectors
newtest.f90:38:18: optimized:  loop versioned for vectorization because of possible aliasing
newtest.f90:27:14: optimized: loop vectorized using 16 byte vectors
newtest.f90:25:18: optimized: loop vectorized using 16 byte vectors
newtest.f90:25:18: optimized:  loop versioned for vectorization because of possible aliasing
newtest.f90:38:18: optimized: loop with 2 iterations completely unrolled (header execution count 4541736)
newtest.f90:25:18: optimized: loop with 2 iterations completely unrolled (header execution count 4541735)
newtest.f90:6:40: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:11:29: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:11:29: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:51:74: optimized: basic block part vectorized using 16 byte vectors
newtest.f90:52:45: optimized: basic block part vectorized using 16 byte vectors

and the ratio:

Time ratio (present / optval) :  0.980000019    
 PASS

With these outputs, it is clear that using -flto -fPIC when compiling stdlib allows the inlining of optval() in the test code, which result in an expected ratio of ~1.

[jvdp1]

@jvdp1 I am quite ignorant of CMake and so on, but tried to change the compilation of stdlib by adding the line add_compile_options(-flto -fPIC) to CMakeLists.txt in stdlib, near where a bunch of similar options are defined.

I am also quite ignorant of CMake. @awvwgk could probably help for a better use of CMake
I did similary as you, except that I commented all other (debug) options, because that may cancel the benefit of -flto -fPIC

[gareth-nx]

@jvdp1 @awvwgk @Carltoffel So I followed the approach of @jvdp1 and hacked the CMakelists.txt file in stdlib to remove a number of compilation options and enforce others:

  #add_compile_options(-fimplicit-none)
  #add_compile_options(-ffree-line-length-132)
  #add_compile_options(-Wall)
  #add_compile_options(-Wextra)
  #add_compile_options(-Wimplicit-procedure)
  #add_compile_options(-Wconversion-extra)
  add_compile_options(-O3 -flto -fPIC)

Now, I am getting relative speeds very close to 1, as reported by @jvdp1 .

So it seems there are a bunch of interacting issues here:

• How stdlib is built
• Compiler options used by codes that use stdlib

This might be tricky to deal with in the general case? For instance I recall having some codes that I could not compile with "lto" (I think they used pre-compiled netcdf or similar).