Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port Classic Flang changes to LLVM 16 #159

Merged
merged 51 commits into from
Jul 6, 2023

Conversation

bryanpkc
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@pawosm-arm pawosm-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the checks will stop failing after the classic flang part is merged.

@bryanpkc
Copy link
Collaborator Author

bryanpkc commented Jun 26, 2023

I suppose the checks will stop failing after the classic flang part is merged.

Yes and no. The Flang test failures will be fixed by flang-compiler/flang#1380 and flang-compiler/flang#1381, but right now the CI jobs are failing during the OpenMP build. I wasn't able to reproduce this locally, and I'm still trying to figure out why the CI jobs are failing.

Update: I have reproduced the problem.

@bryanpkc
Copy link
Collaborator Author

bryanpkc commented Jun 26, 2023

Upstream release/16.x (llvm/llvm-project@7cbf1a2) also fails with the same compile-time failure in OpenMP:

/home/b00375952/src/release_16x/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:740:14: error: call to deleted constructor of 'llvm::Error'
      return Err;
             ^~~
/home/b00375952/src/release_16x/llvm/include/llvm/Support/Error.h:185:3: note: 'Error' has been explicitly marked deleted here
  Error(const Error &Other) = delete;
  ^
/home/b00375952/src/release_16x/llvm/include/llvm/Support/Error.h:492:18: note: passing argument to parameter 'Err' here
  Expected(Error Err)
                 ^
1 error generated.

The CMake command I used on Ubuntu 22.04 x86_64 was:

cmake -DCMAKE_INSTALL_PREFIX=$PWD/install -DCMAKE_BUILD_TYPE=Release \
      -DCMAKE_C_COMPILER=/usr/bin/clang-11 -DCMAKE_CXX_COMPILER=/usr/bin/clang++-11 \
      -DLLVM_TARGETS_TO_BUILD=X86 \
      -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
      -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_PROJECTS="clang;openmp" \
      ../llvm

Update: This should now be fixed.

@pawosm-arm pawosm-arm self-requested a review June 27, 2023 11:51
@bryanpkc
Copy link
Collaborator Author

bryanpkc commented Jul 2, 2023

Added several libomptarget patches that should help with the check-openmp failures.

schweitzpgi and others added 21 commits July 4, 2023 17:33
This commit cherry-picks 427c0e8 from the
release_14x branch, which is a combination of the following legacy patches:

Commit 82b38d2:

    [ClassicFlang] Port release_90 changes from flang-compiler/llvm

    Cherry-picked commit 2085211cfcca70411dc63f0d08763facc8a02090 by Eric Schweitz,
    resolved merge conflicts, fixed build failures (e.g. adapted CGDebugInfo.cpp to
    the new API), and fixed the DIGlobalVariable unit tests, which have been broken
    since commit edfad65eebdf045b050f37380b6b61d673513982.

Commit 885dd87e5fdc:

    [DebugInfo] Removal of DIFortranArrayType and DIFortranSubrange

    These extensions are no more required after merge of below PR.

Commit 5c9b2e0867d5:

    Modification to incorporate comments from @bryanpkc.

Commit 822de2c:

    [AsmPrinter] Fix redundant names and types

    A bug was introduced in 82b38d2 while
    cherry-picking a DIGlobalVariable-related patch.

Commit 45a70a8:

    Port driver changes from release/11x

    Cherry-picked c51f89679135f84675f492d560ec5535c2000cfe by Varun Jayathirtha
    from release_90, and resolved merge conflicts.

    To avoid conflicts with the new Flang, lib/Driver/ToolChains/Flang.{cpp,h}
    have been renamed to ClassicFlang.{cpp,h}, and the ENABLE_CLASSIC_FLANG macro
    is introduced to select which incarnation of Flang to build. The macro is set
    by running CMake with -DLLVM_ENABLE_CLASSIC_FLANG.

    After merge with LLVM 11: Move flang options to the end of the definitions list.

Commit a9a8036:

    Port Classic Flang to LLVM 13

    File changes to TargetLibraryInfo and DebugLocEntry to adapt to the code from
    release/13.x and make it work. Comment out the changes due a segmentation
    fault, code need to be reviewed properly once all commits are in

Commit fe989b7:

    Fix -fveclib=PGMATH

    The use of #ifdef in include/clang/Driver/Options.td was incorrect and
    unsupported. As a result -fveclib=PGMATH was silently ignored, and
    in LLVM 12, it causes the invocation to fail. This patch unguards the
    option so that it is parsed correctly, but lets the FLANG_LLVM_EXTENSIONS
    macro continue to toggle the feature.

Commit 7c224ae:

    Fix use of classic Flang as preprocessor

Commit 8403c83:

    Merge FLANG_LLVM_EXTENSIONS macro into ENABLE_CLASSIC_FLANG

Commit 486741e:

    Fix test failures when in classic Flang mode

    Add a new lit feature tag "classic_flang" to select which tests can or cannot be
    run when the driver is built for classic Flang. Handle LLVM_ENABLE_CLASSIC_FLANG
    in llvm/cmake/modules/HandleLLVMOptions.cmake instead of clang/CMakeLists.txt so
    that macro works in both clang and llvm.

Commit a10f592:

    Port Classic Flang to LLVM 13

    LLVM port from release_12x to release_13x, changes done in order to
    make project able to be build.

Commit d385321 (partial):

    Change to Options.td in order to add the correct invocation for
    ffixed_line_length_VALUE.

This commit also includes the following legacy patch:

Commit 5da2c11:

    Add DebugInfo tests

Commit c379c8d:

    [ClassicFlang][Driver] Correct the LLVM version passed by the Driver
Make TableGen respect the ENABLE_CLASSIC_FLANG macro and better separate the
driver options that are mutually exclusive. When the macro is defined, do not
build LLVM Flang at all.
…lang-compiler#92)

* Support for -gdwarf-5 option in Flang driver.

Summary:
  FLANG driver doesnt pass -gdwarf-4/5 to flang1 in form of xbits,
while it passes for -gdwarf-2/3
   -gdwarf-2 => -x 120 0x200
   -gdwarf-3 => -x 120 0x4000

Due to this -gdwarf-5 is never honored and default option -gdwarf-4
is taken.
    # flang -gdwarf-5 test.f90
    # llvm-dwarfdump a.out | grep version
    0x00000000: Compile Unit: length = 0x0000008e version = 0x0004

  Now 0x1000000/0x2000000 will be passed for -gdwarf-4/5
   -gdwarf-4 => -x 120 0x1000000
   -gdwarf-5 => -x 120 0x2000000

Testing:
  - GNU gdb fortran testsuite
  - check-llvm
  - check-debuginfo

* Flang doenst choose correct dwarf version when multiple -g/-gdwarfN mentioned

 Summary:
When multiple -g/-gdwarfN options are passed together at compile time,
flang chooses the least one. Clang/gfortran etc choose the last one.

-gdwarf-5 -gdwarf-3 => flang chooses 5 while clang/gfortran choose 3
-gdwarf-5 -g => flang choses the default while clang/gfortran choose 5

  Testing:
- check-llvm
- check-debuginfo

* Default dwarf version should be 4 for -g with flang

Currently flang dumps dwarf version 2 for -g and 4 for absence of -g

-------------------------
$ flang my.f90
$ llvm-dwarfdump a.out | grep version
0x00000000: Compile Unit: length = 0x0000003d version = 0x0004 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x00000041)

$ flang -g my.f90
$ llvm-dwarfdump a.out | grep version
0x00000000: Compile Unit: length = 0x00000047 version = 0x0002 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x0000004b)
-------------------------

It should be 4 for -g as it is the case with clang.
This commit is motivated by reducing the merge burden by shrinking the
diff between llvm upstream and classic-flang-llvm-project.

Outside of Flang, Fortran code is fed through the Compile phase, and
the appropriate tooling is picked up through ToolChain::SelectTool.

Classic Flang introduced a FortranFrontend, but these days this seems
unnecessary. Fortran can go through the same machinery as everything else.

* Use the Preprocess phase to preprocess Fortran code. This phase is
  always combined with the Compile phase.

* Use the Compile phase to lower Fortran code to LLVM IR, and use the
  Backend phase to compile and optimize the IR. These phases are never
  combined.

* Remove FortranFrontendJobClass.

* Remove FortranFrontend tool (instead it's just the Flang tool, which
  in Classic Flang mode is Classic Flang).

* Update tests which inspect the output of the Classic Flang tooling,
  and ensures that the driver does the right thing for various types of
  inputs.

Based on a patch from Peter Waller <[email protected]>.
This commit squashes the following CI-related patches from the release_14x
branch.

  commit 54a96d0
  Author: Kiran Chandramohan <[email protected]
  Date:   Wed Oct 28 17:08:22 2020 +0000

      [workflows] Removing a few CI pipelines

  commit 8bf9551
  Author: Bryan Chan <[email protected]>
  Date:   Mon May 2 21:16:56 2022 -0400

      [workflows] Stop using upstream actions and enable OpenMP build

  commit b921dd2
  Author: michalpasztamobica <[email protected]>
  Date:   Fri Jun 4 13:55:52 2021 +0200

      [workflows] Allow pre-compile jobs to be trigerred manually.

  commit 9f828fd
  Author: michalpasztamobica <[email protected]>
  Date:   Fri Jun 4 22:09:44 2021 +0200

      [workflows] Add build scripts and make CI use them.

  commit a976b96
  Author: Abraham Tovar <[email protected]>
  Date:   Wed Oct 13 11:40:30 2021 +0200

      [workflows] LLVM release_13x CI changes

  commit 7a5ee22
  Author: Abraham Tovar <[email protected]>
  Date:   Wed Nov 17 10:14:21 2021 +0000

      [workflows] Removing CI test-base for release_100

  commit a39a0c6
  Author: pwisniewskimobica <[email protected]>
  Date:   Thu Jan 27 15:06:19 2022 +0100

      [workflows] Use GCC 11 in CI builds

  commit 198a6b2
  Author: pwisniewskimobica <[email protected]>
  Date:   Thu Jan 13 10:11:43 2022 +0100

      [workflows] Modify CI scripts to add more test variants

  commit 84ed34d
  Author: Bryan Chan <[email protected]>
  Date:   Sun Apr 24 19:13:25 2022 -0400

      [workflows] support GitHub actions for release_14x branch

  commit dd852a6
  Author: Bryan Chan <[email protected]>
  Date:   Fri Jul 15 21:50:21 2022 -0400

      [workflows] Fix CI for release_14x

  commit b52bcc0
  Author: Bryan Chan <[email protected]>
  Date:   Wed Jul 27 22:28:43 2022 -0400

      [build] Stop caching AArch64 Docker image layers
* -Wsuggest-override for ClassicFlangMacroBuilder::defineMacro
* -Wrange-loop-construct when iterating over llvm::opt::Arg::getValues

Signed-off-by: Itay Bookstein <[email protected]>
-emit-flang-llvm instructs flang to stop after flang2 and dump the LLVM IR.

Can be useful for debugging and also would be a useful option for testing flang
output more accurately.

Signed-off-by: Richard Barton <[email protected]>
Add %flang as a tool substitution available in lit tests. This apes the way
%clang is defined and adds a $FLANG override in a similar vein.

To avoid this being dead code, add a single test to check the flang driver is
reporting the correct phases when running under various phase control options.

Signed-off-by: Richard Barton <[email protected]>
This patch partially reverts the following commit from an early version of
Classic Flang LLVM:

flang-compiler/llvm@6866909

When the LoopVectorize.cpp change in that commit is ported to LLVM 14, it causes
a failure in the following test:

llvm/test/Transforms/LoopVectorize/AArch64/smallest-and-widest-types.ll

Reverting the change allows all tests to run cleanly.
Port commit 64bf2a6 to LLVM 16 and resolve merge conflicts. The original
commit message follows:

Pass LLVM target features (-mattr strings) to flang to embed in generated
.ll files. For normal compilation this won't make much difference as the
attributes are passed to clang after flang2 and can be applied there but this
is crucial to enable LTO with flang as clang will not apply the attributes
when building the flang2 output. libLTO will need to read these out of the
object files to apply them.

Signed-off-by: Richard Barton <[email protected]>
Use existing clang support for uniquifying target feature lists to pass
a unique list through to flang.
Version screen has been erroneously reporting as flang-new since LLVM 12
These CMake variables are used to set python variables in the lit
config. As such, they need to bet set with a valid CMake boolean value
that is also a valid python boolean value (1, True, etc.) and not one
that is not (ON, NO, etc.) to work. This is fragile.

Call the LLVM cmake helper function on these two downstream CMake
bryanpkc and others added 25 commits July 4, 2023 17:33
In LLVM 15, the Fortran and OpenMP runtime libraries are added to the linker
command line using common methods (addFortranRuntime* and addOpenMPRuntime*).
This commit adds Classic Flang awareness to addFortranRuntime*, so that
Classic Flang doesn't attempt to link with LLVM Flang libraries. Re-using
the same methods as Clang and LLVM Flang also helps reduce downstream delta.
A Classic Flang test is added to ensure that the linker command is constructed
correctly.
LLVM 15 added 'ignore-forks: true' to the GitHub action triggers, which
prevents the actions from running in the classic-flang-llvm-project repo.
This patch undoes the setting.
… branches

By changing the branch filter from an explicit list to a glob, this commit
hopes to reduce future maintenance burden (as long as we continue to use
the same branch naming convention).
Unlike their flang counterparts, classic-flang-llvm-project PRs did not have
any AArch64 workflows. So problems with a PR may remain hidden until another
PR is submitted to the flang repo and fails the AArch64 workflow there.
This patch adds the missing build job.
Delete some snippets that had been commented out since commit a10f592
in release_13x.
Upstream llvm-project repository does not accept pull requests, but our fork does.
The following Flang commit has changed the default location of the Flang build
directory, resulting in CI failures in the classic-flang-llvm-project:

flang-compiler/flang@054bde3

This patch updates the path name in the CI workflows.
This patch adds the -x option which lets the user specify additional CMake
options when configuring classic-flang-llvm-project.
This patch adds code to generate the driver flags so that llvm bridge in
flang2 can generate the nsz, reassoc attributes to arithmetic instructions.
Also included is a testcase.
This patch fixes link-time issues experienced while compiling some
more obscure Fortran workloads. For those programs, calls to the
libpgmath functions not defined for AArch64 were generated, e.g.
`__pg_log_4` or `__fd_log_4`. This patch eliminates such possibility.

Signed-off-by: Paul Osmialowski <[email protected]>
As noted in flang-compiler#11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes flang-compiler#11.

Originally By: Keno Fischer <[email protected]>
The 'append' method should be used instead of extend,
since the 'extend' method treats the arguments as an iterable object,
so extends the current list with list of characters.
The 'llvm_build_project' build script installs llvm after the build by default.
It is not necessary, if the user doesn't want.
…-none

These three options are added in release_12. When compiling programs
with them using Classic Flang, there are only warnings generated. They
are removed now so that the errors of "unknown argument" are reported.
Add these options in Driver for compatibility reason. Please note that
these options are not implemented yet in Classic Flang.
LLVM IR input should not be acceptable for Classic Flang tools. This bug was
caused by an incorrect merge conflict resolution when upgrading to LLVM 15.
* Extend pre-compile_llvm workflow with a Windows job which
defines build strategy for X64 and ARM64 architectures
to produce prebuilt LLVM binaries for both platform.
Currently Windows on ARM job is disabled, until
an official will be available.

* Also enable Classic Flang LLVM testing on Windows x64 only.
Some tests are failing on Windows on ARM, so we don't run
tests on WoA until they are fixed.

* Added Windows support to 'classic-flang.f95' driver test.
Modified FileChecker regex pattern to match Windows path
separator and binary suffix too.
* Native Ninja is already available for Windows on ARM,
so we can use it on both Windows platform.

* Introduced a new '-i' build option to trigger whether
the user wants to install LLVM or not. Previously the script relies
on install_prefix option which was wrong approach.
In this case we still could not defer the install for a later time
if install directory was set.
With this change the python build script will be consistent
with official bash script.
Summary:
Some older compilers, which we still support, have problems handling the
copy elision that allows us to directly move an `Error` to an
`Expected`. This patch adds explicit moves to remove the error.

(cherry picked from commit edc0355)
…he linker

Summary:
Clang doesn't warn on `-B` options passed to it. This one is not
forwarded to the linker which results in some tests failing when
offloading to x86_64 with the `bfd` linker.

(cherry picked from f983987)
Summary:
This code passed the value of `-rpath` directly to the clang invocation.
If we're using the linker then it'll be fine. However, if the linker is
`gcc` as is the case when doing `-fopenmp-targets=x86_64` then this will
cause problems.  This patch adds the `-Wl,-rpath,` to feed it to the
linker correctly.

(cherry picked from 51ff548)
Summary:
The placement of input files can change the result of the linker. We
should put the input files earlier to avoid this.

(cherry picked from fa5209c)
Otherwise Classic Flang's CMakeLists.txt will be unable to find llvm-lit and
cannot generate test-related goals, e.g. `check-all`.
@bryanpkc
Copy link
Collaborator Author

bryanpkc commented Jul 5, 2023

Fixed a couple of test-related problems. All CI checks are passing now.

@bryanpkc bryanpkc merged commit b88498b into flang-compiler:release_16x Jul 6, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.