[flang][driver] Fix output filename generation in `flang`

[awarzynski]

Summary

In the flang bash script, we need to be careful _when_ to use <output>
from flang -c -o <output> <input> when generating the relocatable
output file name.

In particular, we should use it in this case:

compilation only
flang -c -o <output> <input>

but leave it for the final executable here:

compile, assemble and link
flang  -o <output> <input>

This change is implemented in get_relocatable_name.

I've also taken the liberty to fix how errors from sub-commands are
reported (without this change, flang always returns 0 on failure).
This is implemented in main.

[DavidSpickett]

The specifics of the script are a bit beyond me but:

(without this change, flang always returns 0 on failure)

Definitely will help the test suite be clearer about what's failed where.

flang/tools/f18/flang.in

343–349

I'm surprised that this works with:

set -euo pipefail

(but then again bash is always surprising)

[awarzynski]

flang/tools/f18/flang.in

343–349

That's a great catch, thank you!

I don't see a better way to work around it other than surrounding such statements with set +e and set -e. That's also the suggested solution here (it's the best intro into set -euo pipefail that I am aware of). This is not great, but we don't consider this script a long-term solution, so perhaps that's fine?

As for if ! <command>, that sets $? to 0 when <command> fails. That's because there is ! in ! <command>. This is not what we want here. This is documented in help if. Hence the need to update this.

[awarzynski]

Work around set -e and commands that might fail

[DavidSpickett]

The failure handling looks good. I'm still confused about the output naming. Can you clarify what you mean by:

In particular, we should use it in this case:

compilation only
flang -c -o <output> <input>
but leave it for the final executable here:

compile, assemble and link
flang  -o <output> <input>

Do you mean "but leave it for the final executable" as in, only use <output> to name the final executable. Instead of using it to name the intermediate sources as well?

flang/tools/f18/flang.in

343–349

This is not great

My thoughts on most bash but this looks good. Pretty obvious what's going on.

[awarzynski]

In D105896#2879350, @DavidSpickett wrote:

The failure handling looks good. I'm still confused about the output naming. Can you clarify what you mean by:

In particular, we should use it in this case:

compilation only
flang -c -o <output> <input>
but leave it for the final executable here:

compile, assemble and link
flang  -o <output> <input>

Do you mean "but leave it for the final executable" as in, only use <output> to name the final executable. Instead of using it to name the intermediate sources as well?

Let's consider 2 examples. This is basically what the script does.
CASE 1: compilation only

flang -c hello.f90 -o hello

This script will translate this into:

# Genreate temporary unaprsed Fortran file
flang-new -fdebug-unparse hello.f90 -o flang_unparsed_source_file_0.f90
# Genreate the output object file - do use `-o hello`
gfortran -c -o hello flang_unparsed_source_file_0.f90

CASE 2: compilation + assembly + linking

flang  hello.f90 -o hello

This script will translate this into:

# Genreate temporary unaprsed Fortran file
flang-new -fdebug-unparse hello.f90 -o flang_unparsed_source_file_0.f90
# Generate a temporary object file - don't use `-o hello`
gfortran -c  flang_unparsed_source_file_0.f90  -o flang_unparsed_source_file_0.o
# Generate the final outupt - do use `-o hello`
gfortran  flang_unparsed_source_file_0.o -o hello

BOTTOM LINE
The script needs to be careful _when_ to use -o <output> and _what_ to use as <output>. Sadly, it's tricky to convey that with comments or code :/

[DavidSpickett]

I don't think I can word it well either but it makes sense seeing the examples.

[awarzynski]

Rebase on top of main

flang/tools/f18/flang.in

for idx in "${!other_srcs[@]}"; do
     then status=$?
          echo flang: in "$PWD", flang-new failed with exit status $status: "$wd/bin/flang-new" "${opts[@]}" "$@" >&2
          exit $status
     fi
   done
 }
 
 # === get_relocatable_name ======================================================
-#
-# Generate a name for the output object file based on the source file, e.g.
+# This method generates the name of the output file for the compilation phase
+# (triggered with `-c`). If the user of this script is only interested in
+# compilation (`flang -c`), use $OUTPUT_FILE provided that it was defined.
+# Otherwise, use the usual heuristics:
 #   * file.f --> file.o
 #   * file.c --> file.o
-# If OUTPUT_FILE is defined, use that instead.
 #
 # INPUTS:
 #   $1 - input source file for which to generate the output name
 # =============================================================================
 get_relocatable_name() {
   local -r src_file=$1
 
-  if [[ ! -z ${OUTPUT_FILE:+x} ]]; then
+  if [[ $COMPILE_ONLY == "True" ]] && [[ ! -z ${OUTPUT_FILE:+x} ]]; then
     out_file="$OUTPUT_FILE"
   else
     current_ext=${src_file##*.}
     new_ext="o"
 
     out_file=$(basename "${src_file}" "$current_ext")${new_ext}
   fi
 
main() {
   flang_options+=("-module-suffix")
   flang_options+=(".f18.mod")
   flang_options+=("-fdebug-unparse")
   flang_options+=("-fno-analyzed-objects-for-unparse")
 
   [[ ! -z ${MODULE_DIR} ]] && flang_options+=("-module-dir ${MODULE_DIR}")
   [[ ! -z ${INTRINSICS_MOD_DIR} ]] && flang_options+=("-intrinsics-module-directory ${INTRINSICS_MOD_DIR}")
   for idx in "${!fortran_source_files[@]}"; do
-    if ! "$wd/bin/@FLANG_DEFAULT_DRIVER@" "${flang_options[@]}" "${fortran_source_files[$idx]}" -o "${unparsed_file_base}_${idx}.f90"
-    then status=$?
-         echo flang: in "$PWD", @FLANG_DEFAULT_DRIVER@ failed with exit status $status: "$wd/bin/@FLANG_DEFAULT_DRIVER@" "${flang_options[@]}" "$@" >&2
-         exit $status
+    set +e
+    "$wd/bin/@FLANG_DEFAULT_DRIVER@" "${flang_options[@]}" "${fortran_source_files[$idx]}" -o "${unparsed_file_base}_${idx}.f90"
+    ret_status=$?
+    set -e
+    if [[ $ret_status != 0 ]]; then
+         echo flang: in "$PWD", @FLANG_DEFAULT_DRIVER@ failed with exit status "$ret_status": "$wd/bin/@FLANG_DEFAULT_DRIVER@" "${flang_options[@]}" "$@" >&2
+         exit "$ret_status"
     fi
   done
 
   # STEP 2: Compile Fortran Source Files
   readonly ext_fc="${F18_FC:-gfortran}"
   for idx in "${!fortran_source_files[@]}"; do
     # We always have to specify the output name with `-o <out_obj_file>`. This
     # is because we are using the unparsed rather than the original source file
     # below. As a result, we cannot rely on the compiler-generated output name.
     out_obj_file=$(get_relocatable_name "${fortran_source_files[$idx]}")
 
-    if ! $ext_fc "-c" "${ext_fc_options[@]}" "${unparsed_file_base}_${idx}.f90" "-o" "${out_obj_file}"
-    then status=$?
-      echo flang: in "$PWD", "$ext_fc" failed with exit status $status: "$ext_fc" "${ext_fc_options[@]}" "$@" >&2
-         exit $status
+    set +e
+    $ext_fc "-c" "${ext_fc_options[@]}" "${unparsed_file_base}_${idx}.f90" "-o" "${out_obj_file}"
+    ret_status=$?
+    set -e
+    if [[ $ret_status != 0 ]]; then
+      echo flang: in "$PWD", "$ext_fc" failed with exit status "$ret_status": "$ext_fc" "${ext_fc_options[@]}" "$@" >&2
+         exit "$ret_status"
     fi
     object_files+=(${out_obj_file})
   done
 
   # Delete the unparsed files
   for idx in "${!fortran_source_files[@]}"; do
     rm "${unparsed_file_base}_${idx}.f90"
   done
 
   # STEP 3: Compile Other Source Files
   for idx in "${!other_source_files[@]}"; do
     # We always specify the output name with `-o <out_obj_file>`. The user
     # might have used `-o`, but we never add it to $OPTIONS (or
     # $ext_fc_options). Hence we need to use `get_relocatable_name`.
     out_obj_file=$(get_relocatable_name "${other_source_files[$idx]}")
 
-    if ! $ext_fc "-c" "${ext_fc_options[@]}" "${other_source_files[${idx}]}" "-o" "${out_obj_file}"
-    then status=$?
-      echo flang: in "$PWD", "$ext_fc" failed with exit status $status: "$ext_fc" "${ext_fc_options[@]}" "$@" >&2
-         exit $status
+    set +e
+    $ext_fc "-c" "${ext_fc_options[@]}" "${other_source_files[${idx}]}" "-o" "${out_obj_file}"
+    ret_status=$?
+    set -e
+    if [[ $ret_status != 0 ]]; then
+      echo flang: in "$PWD", "$ext_fc" failed with exit status "$ret_status": "$ext_fc" "${ext_fc_options[@]}" "$@" >&2
+         exit "$ret_status"
     fi
     object_files+=(${out_obj_file})
   done
 
   # STEP 4: Link
   if [[ $COMPILE_ONLY == "True" ]]; then
     exit 0;
   fi
 
   if [[ ${#object_files[@]} -ge 1 ]]; then
     # If $OUTPUT_FILE was specified, use it for the output name.
     if [[ ! -z ${OUTPUT_FILE:+x} ]]; then
       output_definition="-o $OUTPUT_FILE"
     else
       output_definition=""
     fi
 
-    if ! $ext_fc "${ext_fc_options[@]}" "${object_files[@]}" "${lib_files[@]}" ${output_definition:+$output_definition}
-    then status=$?
-         echo flang: in "$PWD", "$ext_fc" failed with exit status $status: "$ext_fc" "${ext_fc_options[@]}" "$@" >&2
-         exit $status
+    set +e
+    $ext_fc "${ext_fc_options[@]}" "${object_files[@]}" "${lib_files[@]}" ${output_definition:+$output_definition}
+    ret_status=$?
+    set -e
+    if [[ $ret_status != 0 ]]; then
+         echo flang: in "$PWD", "$ext_fc" failed with exit status "$ret_status": "$ext_fc" "${ext_fc_options[@]}" "$@" >&2
+         exit "$ret_status"
     fi
   fi
 
   # Delete intermediate object files
   for idx in "${!fortran_source_files[@]}"; do
     rm "${object_files[$idx]}"
   done
 }
 
 main "${@}"