[all-commits] [llvm/llvm-project] d0ee8b: [LTO] Fix -fwhole-program-vtables handling after H...

Teresa Johnson via All-commits all-commits at lists.llvm.org
Thu Jun 3 14:25:26 PDT 2021


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: d0ee8b64ecf359737ce550d8f47f465ab6657be7
      https://github.com/llvm/llvm-project/commit/d0ee8b64ecf359737ce550d8f47f465ab6657be7
  Author: Teresa Johnson <tejohnson at google.com>
  Date:   2021-06-03 (Thu, 03 Jun 2021)

  Changed paths:
    M clang/lib/Driver/ToolChains/Clang.cpp
    M clang/test/Driver/cuda-options.cu
    M clang/test/Driver/hip-options.hip

  Log Message:
  -----------
  [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

A recent change (D99683) to support ThinLTO for HIP caused a regression
when compiling cuda code with -flto=thin -fwhole-program-vtables.
Specifically, we now get an error:
error: invalid argument '-fwhole-program-vtables' only allowed with '-flto'

This error is coming from the device offload cc1 action being set up for
the cuda compile, for which -flto=thin doesn't apply and gets dropped.
This is a regression, but points to a potential issue that was silently
occurring before the patch, details below.

Before D99683, the check for fwhole-program-vtables in the driver looked
like:

  if (WholeProgramVTables) {
    if (!D.isUsingLTO())
      D.Diag(diag::err_drv_argument_only_allowed_with)
          << "-fwhole-program-vtables"
          << "-flto";
    CmdArgs.push_back("-fwhole-program-vtables");
  }

And D.isUsingLTO() returned true since we have -flto=thin. However,
because the cuda cc1 compile is doing device offloading, which didn't
support any LTO, there was other code that suppressed -flto* options
from being passed to the cc1 invocation. So the cc1 invocation silently
had -fwhole-program-vtables without any -flto*. This seems potentially
problematic, since if we had any virtual calls we would get type test
assume sequences without the corresponding LTO pass that handles them.

However, with the patch, which adds support for device offloading LTO
option -foffload-lto=thin, the code has changed so that we set a bool
IsUsingLTO based on either -flto* or -foffload-lto*, depending on
whether this is the device offloading action. For the device offload
action in our compile, since we don't have -foffload-lto, IsUsingLTO is
false, and the check for LTO with -fwhole-program-vtables now fails.

What we should do is only pass through -fwhole-program-vtables to the
cc1 invocation that has LTO enabled (either the device offload action
with -foffload-lto, or the non-device offload action with -flto), and
otherwise drop the -fwhole-program-vtables for the non-LTO action.
Then we should error only if we have -fwhole-program-vtables without any
-f*lto* options.

Differential Revision: https://reviews.llvm.org/D103579




More information about the All-commits mailing list