[RFC] Flang - improve debug information, user error messages and fix OpenMP function mismatch for -save-temps flag

Subprojects Flang
1

Hi,
I want to improve debug information, user error messages and fix a bug for OpenMP offloading for flang with attached -save-temps flag, I describe the motivation, root cause in the next paragraphs and I provide three possible solutions. Please choose the best option.

Motivation:
I would like to point out that Flang behaves differently when -save-temps flag is specified by the user. I have observed 3 issues which are visible when -save-flag is set.

  • Different information about error location when -save-temps flag is set
    Information about error location is dependent on -save-temps flag. If the user sets -save-temps then the error is reported in the preprocessed file. If -save-temps flag is not set then error is reported in source file. Clang always reports an error in the source file.

    Example:

    Source code:

subroutine test
  implicit none
  integer :: i
  ij=10
  print *,i
end subroutine

Output of flang-new -save-temps -c test.f95 :

       error: Semantic errors in test.i
      ./test.i:5:7: error: No explicit type declared for 'ij'

Output of flang-new -c test.f95:

    error: Semantic errors in test.f95
    ./test.f95:4:3: error: No explicit type declared for 'ij'

As you can see the source location of the error is different (different file, line and column)

  • Different debug info for -save-temps flag:
    If you attach -save-temps flag then the debug info wil point to the location inside preprocessed file not the original source code. If we do not attach -save-temps then debug info will point out to original source code. Clang always points debug info to the original source code.

  • OpenMP offloading function mismatch for -save-temps flag
    The -save-temps flag breaks code generation for OpenMP offloading. OpenMP requires separate preprocessing for host and the device code.
    That’s why we generate two preprocessed files for OpenMP offloading: test_host.i and test_device.i . We use the name of the source file as the parameter for generation of an unique name of the kernel function which corresponds to given pragma omp target . Clang always takes as the parameter the name of the source file. Flang uses two different files for -save-temps flag (test_host.i and test_device.i) as the input when it generates the host call and target function. In consequence we have a function names mismatch:
    on the host side:
    call omp_kernel_1111 ()
    whereas the device kernel has different name:
    omp_kernel_2222 () { ... }
    We have no function name mismatch for Flang when we do not set -save-temps flag because the function names is generated on the basis of the source file not the preprocessed files.

Root cause:
The flang driver provides different set of flang-new -fc1 commands. When -save-temps flag is set we have separate preprocessor step:

For flang-new -save-temps -v -c test.f95 we see the following flang-new -fc1 commands:

flang-new -fc1 -E /* other flags */ -o test.i 
flang-new -fc1 /*other flags */ -o test.bc -x f95 test.i

If flag -save-temps is not set then we have no separated preprocessor step:
flang-new -fc1 /*other flags */ -o test.o -x f95-cpp-input test.f95

Discussion about possible solutions:

A) Merge preprocessor step with LLVM IR code generation.

If user specifies -save temps then flang will launch one command:
flang-new -fc1 /*other flags */ -o test.bc -x f95 test.f95
instead of:

flang-new -fc1 -E /* other flags */ -o test.i 
flang-new -fc1 /*other flags */ -o test.bc -x f95 test.i

Advantages:

  • No modification of Flang Preprocessor.

Disadvantages:

  • Possible code duplication with Clang. IIRC Flang uses parts of Clang driver to control compilation phases.

B) Modify flang preprocessor so that it will use source location from original source file not the preprocessed file.

Advantages:
a) No modification of Flang/Clang driver

Disadvantages:
a) Flang preprocessor attaches limited information about origins of the preprocessed code.
b) User can disable information about the source file in preprocessor output. If user specifies flang-new -P -save-temps then we have no information about original code loacation and all problems mentioned in previous section are back.

C) Provide information about original source file by additional -fc1 flag -main-file-name

Advantages:

a) Allow to bypass issues mentioned in solutions 1 and 2.
b) This solution allows me to sollve an OpenMP offloading function name mismatch without large modification of Flang source code.

Disadvantages:

a) It’s a bypass solution and it complicates compiler logic. We still could have inaccurate information about line/column of the code symbol if the parser modifies the layout of the source code (Actually flang-new -E generates reformatted code so the line number/column can be inaccurate with -save-temps flag).

Thank you for reading so far. Please share your feedback.

1 Like
2

Hi Dominik, thanks for working on this and for this comprehensive summary!

My first reaction is that that’s a few different goals in one RFC :slight_smile: In particular, to me these seem like two separate issues:

  • Different debug info for -save-temps flag
  • OpenMP offloading function mismatch for -save-temps flag

So far this is identical to what Clang does, right? So the fact that clangDriver creates these different jobs doesn’t feel like the actual root cause (based on the fact that everything works fine for Clang).

+1

It sounds like your issues could be fixed with some rather non-intrusive updates to the preprocessor. And that’s what I would attempt in the first instance

You may discover that a lot of the logic in Clang (and projects that use it) relies on the current behaviour. It’s not something that I’ve researched myself (i.e. job creation logic in clangDriver), but feels like a pretty fundamental part of the driver logic. I would prioritise alternative solutions.

This feels like the quickest route to solve your problems and you could probably prototype it quite quickly? How would the driver decide when to use this flag? But then …

… brings us back to the preprocessor.

+1 for consistency with Clang.

-Andrzej

1 Like
3

What do you think about âš™ D151445 [Flang] Add main-file-name flag to flang -fc1 ? This is short-term fix for OpenMP function mismatch issue. This flag is automatically attached to flang -fc1 if the input code comes from the file. I use the value of this flag to restore the name of the original file for OpenMP MLIR to LLVM IR translation.

4

I think this option is probably the best for us if the frontend team agrees. It will also help the CMake flow which invokes the preprocessor separately and help produce better error or location information. Is the -P option commonly used?

In flang, it is the frontend that is the canonical source for File and Location information. This flag bypasses the frontend of flang which might not be desirable.

5

I agree this is the best option. It seems that flang already does something similar to clang, by inserting information about the original file in the preprocessed file.

Using the previous example, saved to test.f90 file, this source

subroutine test
  implicit none
  integer :: i
  ij=10
  print *,i
end subroutine

is preprocessed to

#line "./test.f90" 1
      subroutine test
      implicit none
      integer :: i
      ij=10
      print *,i
      end subroutine

So, at least in this case, the needed information is already present, but it seems the frontend is not using it, because it indeed reports the file name and line of the preprocessed file in the error message.

1 Like
6

Hi @kiranchandramohan , @banach-space , @luporl
thank you for your meaningful feedback. I managed to use Flang preprocessor "line" directive to get original tokens instead of preprocessed ones. The âš™ D153329 [PoC][Flang] Use preprocessor line directive to add more accurate token localization for -save-temps flag contains proposition how we can improve token localization when -save-temps flag is attached.

What is missing:

  1. Handling information about the starting line (I only use information about the file origins and I use the whole file)
  2. Extensive testing (I only run LLVM Flang unit tests). Do you know test cases/Fortran benchmarks which extensively use Fortran preprocessor?
1 Like
7

The CMake flow will preprocess separately (flang-new -E) and then invoke the frontend on the preprocessed output. Effectively, you can run files with flang-new -E and then pipe it to flang to test this.

It will be good if you change the title of the patch to be to process line directives. And mention in the summary of the patch the various use cases.

8

Note also that the OpenMP construct location is not correct in some places regardless of the above mentioned problem unless this issue ([Flang][OpenMP] Incorrect location information for OpenMP constructs/clauses · Issue #57215 · llvm/llvm-project · GitHub) was addressed.

9

@clementval I was not aware of this bug. It needs to be addressed as well. IMO the reported bug is independent from observed function mismatch for OpenMP target offloading for -save-temps flag. Please look at the detailed description below for more information:

We generate two preprocessed files for OpenMP offloading for test.f90 and -save-temps flag: test_host.i and test_device.i . For OpenMP we use the name of the source file as the parameter for generation of the unique name of the kernel function which corresponds to given pragma omp target . Clang always takes as the parameter the name of the source file. Flang uses two different files for -save-temps flag (test_host.i and test_device.i) as the input when it generates the host call and target function. In consequence we have a function names mismatch:
on the host side:
call omp_kernel_1111 ()
whereas the device kernel has different name:
omp_kernel_2222 () {
}

10

Could you share an example? Ideally, Clang and Flang should behave consistently. Better still, share this logic. Is that what your patch for -main-file-name strives to achieve?

11

OpenMP background:
Currently OpenMP for Clang is being heavily refactored. The functions, which can be reused by Flang are being moved from Clang Codegen into OpenMPIRBuilder which is used both by Clang and Flang. Both Clang and Flang translates input code twice (once to generate the device code and the second one to generate the host code). Clang calls OpenMPIRBuilder mainly from CGOpenMPRuntime.cpp file.

Example:
I base on Clang built from main branch. (commit id: 25f8b1a0a812717ecf095761dd6c7d6601408a1c)

Input code:

int foo(int *A)
{
#pragma omp  target
   *A = 10;
  return 0;
}

Compilation command:

clang -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx908 -c test.c -save-temps

Generated device code for OpenMP target pragma is placed in file test-openmp-amdgcn-amd-amdhsa.bc:

define weak_odr protected amdgpu_kernel void @__omp_offloading_fd00_e443d9_foo_l3(ptr noundef %A) #0 {

The host call is in file test–host-x86_64-unknown-linux-gnu.bc:

  %19 = call i32 @__tgt_target_kernel(ptr @1, i64 -1, i32 -1, i32 0, ptr @.__omp_offloading_fd00_e443d9_foo_l3.region_id, ptr %kernel_args)

The internal Clang logic:
The name of the function is created by the function getTargetRegionEntryFnName . Clang uses a wrapper function which takes const TargetRegionEntryInfo &EntryInfo as the input parameter.

Clang firstly generates TargetRegionEntryInfo struct. The function which generates TargetRegionEntryInfo struct is defined here. It takes as the parameter BeginLoc of type SourceLocation and it extracts the name of the source code file. When the struct is ready, then the function getTargetRegionEntryFnName from OpenMPIRBuilder is called.

As you can see Clang has information about proper file name in SourceLocation objects. My patch with -main-file-name is workaround because information about original source file cannot be extracted from MLIR module when -save-temps flag is attached.

12

Wouldn’t it make more sense to start your two workflows from the parse tree instead of the preprocessed file?

13

Wouldn’t it make more sense to start your two workflows from the parse tree instead of the preprocessed file?

No, both Clang and Flang start with separate preprocessing for OpenMP offloading. We do it to handle code which depends on target specific preprocessor directives. For example:

#ifdef __SSE2
const int VF = 4
#else
const int VF = 1
endif

You can find detailed explanation here.

14

I understand that but why do you need to start from a preprocessed file. You could do your compilation without going through temp files.

15

Can you start the process with the original file so each pass is a full compilation (but managed as a joined-pair by the driver)?

16

I cannot do compilation without going through temp files if I attach -save-temps flag, because the Flang compiler driver creates different compiler jobs for -save-temps flag.

For -save-temps flag Flang compiler driver process each file separately.

  • User command: flang-new test.f95 -v -c -save-temps
    • Fortran compilation jobs:
      flang-new -fc1 -triple x86_64-unknown-linux-gnu -E -fcolor-diagnostics -mrelocation-model static -target-cpu x86-64 -o test.i -save-temps=cwd -x f95-cpp-input test.f95
      flang-new -fc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -fcolor-diagnostics -mrelocation-model static -target-cpu x86-64 -o test.bc -save-temps=cwd -x f95 test.i
      Other compilation steps which converts *.bc file to *.s and then to *.o file

Whereas without -save-temps Flang generates the object file directly from the source file:

  • User command: flang-new test.f95 -v -c
    • Fortran compilation job: flang-new -fc1 -triple x86_64-unknown-linux-gnu -emit-obj -fcolor-diagnostics -mrelocation-model static -target-cpu x86-64 -o test.o -x f95-cpp-input test.f95

IMO the -save-temps flag is useful for compiler debugging. I can extract the code (MLIR / LLVM IR / assembly) and I can easily verify if given compilation phase is correct.

I cannot do it for -save-temps flag because the set of compilation jobs is managed by Flang driver which uses Clang’s driver library. I was convinced that I should not modify the Flang driver (see this post)

17

https://reviews.llvm.org/D153910

1 Like
18

Fix merged.