[clang] 77596e6 - Revert D157750 "[Driver][CodeGen] Properly handle -fsplit-machine-functions for fatbinary compilation."

Fangrui Song via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 21 13:54:21 PDT 2023


Author: Fangrui Song
Date: 2023-08-21T13:54:15-07:00
New Revision: 77596e6b167bf0a5efa790597d6b75ac5e685b55

URL: https://github.com/llvm/llvm-project/commit/77596e6b167bf0a5efa790597d6b75ac5e685b55
DIFF: https://github.com/llvm/llvm-project/commit/77596e6b167bf0a5efa790597d6b75ac5e685b55.diff

LOG: Revert D157750 "[Driver][CodeGen] Properly handle -fsplit-machine-functions for fatbinary compilation."

This reverts commit 317a0fe5bd7113c0ac9d30b2de58ca409e5ff754.
This reverts commit 30c4b97aec60895a6905816670f493cdd1d7c546.

See post-commit discussions on https://reviews.llvm.org/D157750 that
we should use a different mechanism to handle the error with --cuda-gpu-arch=

The IR/DiagnosticInfo.cpp, warn_drv_for_elf_only, codegne tests in
clang/test/Driver, and the following driver behavior (downgrading error
to warning) changes are undesired.
```
% clang --target=riscv64 -fsplit-machine-functions -c a.c
warning: -fsplit-machine-functions is not valid for riscv64 [-Wbackend-plugin]
```

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticDriverKinds.td
    clang/lib/Driver/ToolChains/Clang.cpp
    clang/test/Driver/fsplit-machine-functions.c
    clang/test/Driver/fsplit-machine-functions2.c
    llvm/include/llvm/IR/DiagnosticInfo.h
    llvm/lib/CodeGen/MachineFunctionSplitter.cpp
    llvm/lib/IR/DiagnosticInfo.cpp
    llvm/test/CodeGen/Generic/machine-function-splitter.ll

Removed: 
    clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
    llvm/test/CodeGen/Generic/Inputs/fsloader-mfs.afdo


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 19294c50543317..73264181ca1c5c 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -693,10 +693,6 @@ def warn_drv_fjmc_for_elf_only : Warning<
   "-fjmc works only for ELF; option ignored">,
   InGroup<OptionIgnored>;
 
-def warn_drv_for_elf_only : Warning<
-  "'%0' works only for ELF; option ignored">,
-  InGroup<OptionIgnored>;
-
 def warn_target_override_arm64ec : Warning<
   "/arm64EC has been overridden by specified target: %0; option ignored">,
   InGroup<OptionIgnored>;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 8ac68df3b6db73..39bc21b9678ba5 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,15 +5872,14 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
                                options::OPT_fno_split_machine_functions)) {
-    if (A->getOption().matches(options::OPT_fsplit_machine_functions)) {
-      // This codegen pass is only available on elf targets.
-      if (Triple.isOSBinFormatELF())
+    // This codegen pass is only available on x86-elf targets.
+    if (Triple.isX86() && Triple.isOSBinFormatELF()) {
+      if (A->getOption().matches(options::OPT_fsplit_machine_functions))
         A->render(Args, CmdArgs);
-      else
-        D.Diag(diag::warn_drv_for_elf_only) << A->getAsString(Args);
+    } else {
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << A->getAsString(Args) << TripleStr;
     }
-    // Do not issue warnings for -fno-split-machine-functions even it is not
-    // on ELF.
   }
 
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,

diff  --git a/clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c b/clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
deleted file mode 100644
index f2b09e13d80b68..00000000000000
--- a/clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ /dev/null
@@ -1,68 +0,0 @@
-// REQUIRES: system-linux
-// REQUIRES: x86-registered-target
-// REQUIRES: nvptx-registered-target
-// REQUIRES: shell
-
-// Check that -fsplit-machine-functions is passed to both x86 and cuda
-// compilation and does not cause driver error.
-// RUN:   cd "$(dirname "%t")" ; \
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
-// RUN:     --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s \
-// RUN:     2>&1 | FileCheck %s --check-prefix=MFS1
-// MFS1: "-target-cpu" "x86-64"{{.*}}"-fsplit-machine-functions"
-// MFS1: "-target-cpu" "sm_70"{{.*}}"-fsplit-machine-functions"
-
-// Check that -fsplit-machine-functions is passed to cuda and it
-// causes a warning.
-// RUN:   cd "$(dirname "%t")" ; \
-// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
-// RUN:     --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s \
-// RUN:     2>&1 | FileCheck %s --check-prefix=MFS2
-// MFS2: warning: -fsplit-machine-functions is not valid for nvptx
-
-// Check that -Xarch_host -fsplit-machine-functions is passed only to
-// native compilation.
-// RUN:   cd "$(dirname "%t")" ; \
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
-// RUN:     --cuda-gpu-arch=sm_70 -x cuda -Xarch_host \
-// RUN:     -fsplit-machine-functions -S %s \
-// RUN:     2>&1 | FileCheck %s --check-prefix=MFS3
-// MFS3:     "-target-cpu" "x86-64"{{.*}}"-fsplit-machine-functions"
-// MFS3-NOT: "-target-cpu" "sm_70"{{.*}}"-fsplit-machine-functions"
-
-// Check that -Xarch_host -fsplit-machine-functions does not cause any warning.
-// RUN:   cd "$(dirname "%t")" ; \
-// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
-// RUN      --cuda-gpu-arch=sm_70 -x cuda -Xarch_host \
-// RUN      -fsplit-machine-functions -S %s || { echo \
-// RUN      "warning: -fsplit-machine-functions is not valid for" ; } \
-// RUN      2>&1 | FileCheck %s --check-prefix=MFS4
-// MFS4-NOT: warning: -fsplit-machine-functions is not valid for
-
-// Check that -Xarch_device -fsplit-machine-functions does cause the warning.
-// RUN:   cd "$(dirname "%t")" ; \
-// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
-// RUN:     --cuda-gpu-arch=sm_70 -x cuda -Xarch_device \
-// RUN:     -fsplit-machine-functions -S %s 2>&1 | \
-// RUN:     FileCheck %s --check-prefix=MFS5
-// MFS5: warning: -fsplit-machine-functions is not valid for
-
-// Check that -fsplit-machine-functions -Xarch_device
-// -fno-split-machine-functions only passes MFS to x86
-// RUN:   cd "$(dirname "%t")" ; \
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
-// RUN:     --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions \
-// RUN:     -Xarch_device -fno-split-machine-functions -S %s \
-// RUN:     2>&1 | FileCheck %s --check-prefix=MFS6
-// MFS6:     "-target-cpu" "x86-64"{{.*}}"-fsplit-machine-functions"
-// MFS6-NOT: "-target-cpu" "sm_70"{{.*}}"-fsplit-machine-functions"
-
-// Check that -fsplit-machine-functions -Xarch_device
-// -fno-split-machine-functions has no warnings
-// RUN:   cd "$(dirname "%t")" ; \
-// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
-// RUN:     --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions \
-// RUN:     -Xarch_device -fno-split-machine-functions -S %s \
-// RUN:     || { echo "warning: -fsplit-machine-functions is not valid for"; } \
-// RUN:     2>&1 | FileCheck %s --check-prefix=MFS7
-// MFS7-NOT: warning: -fsplit-machine-functions is not valid for

diff  --git a/clang/test/Driver/fsplit-machine-functions.c b/clang/test/Driver/fsplit-machine-functions.c
index 4ba4e52fb79484..577c87abd129aa 100644
--- a/clang/test/Driver/fsplit-machine-functions.c
+++ b/clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,10 @@
 // REQUIRES: arm-registered-target
 
-// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions %s -c -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
-// RUN: %clang -### -target x86_64 -fsplit-machine-functions %s -c -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
-// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions -fno-split-machine-functions %s -c -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
-// RUN: %clang -c -target arm-unknown-linux-gnueabi -fsplit-machine-functions %s -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
+// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -target x86_64 -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions -fno-split-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
+// RUN: not %clang -c -target arm-unknown-linux -fsplit-machine-functions %s 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
 
 // CHECK-OPT:       "-fsplit-machine-functions"
 // CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:    warning: -fsplit-machine-functions is not valid for arm
+// CHECK-TRIPLE:    error: unsupported option '-fsplit-machine-functions' for target

diff  --git a/clang/test/Driver/fsplit-machine-functions2.c b/clang/test/Driver/fsplit-machine-functions2.c
index 45bf71b6eb9ad9..1b81be084eff93 100644
--- a/clang/test/Driver/fsplit-machine-functions2.c
+++ b/clang/test/Driver/fsplit-machine-functions2.c
@@ -1,15 +1,12 @@
 // Test -fsplit-machine-functions option pass-through with lto
-// RUN: %clang -### -target x86_64-unknown-linux -flto -fsplit-machine-functions %s -o %t.o 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -### -target x86_64-unknown-linux -flto -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
 
 // Test no pass-through to ld without lto
-// RUN: %clang -### -target x86_64-unknown-linux -fsplit-machine-functions %s -o %t.o 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
+// RUN: %clang -### -target x86_64-unknown-linux -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
 
 // Test the mix of -fsplit-machine-functions and -fno-split-machine-functions
-// RUN: %clang -### -target x86_64-unknown-linux -flto -fsplit-machine-functions -fno-split-machine-functions %s -o %t.o 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
-// RUN: %clang -### -target x86_64-unknown-linux -flto -fno-split-machine-functions -fsplit-machine-functions %s -o %t.o 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
-// Check that for non-X86, passing no-split-machine-functions does not cause error.
-// RUN: %clang -### -target aarch64-unknown-linux-gnu -flto -fsplit-machine-functions -fno-split-machine-functions %s -o %t.o 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS2
+// RUN: %clang -### -target x86_64-unknown-linux -flto -fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
+// RUN: %clang -### -target x86_64-unknown-linux -flto -fno-split-machine-functions -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
 
 // CHECK-PASS:          "-plugin-opt=-split-machine-functions"
 // CHECK-NOPASS-NOT:    "-plugin-opt=-split-machine-functions"
-// CHECK-NOPASS2-NOT:   "-plugin-opt=-split-machine-functions"

diff  --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index 541bbf8a779b8a..628445fe9fb2cc 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -86,7 +86,6 @@ enum DiagnosticKind {
   DK_SrcMgr,
   DK_DontCall,
   DK_MisExpect,
-  DK_MachineFunctionSplit,
   DK_FirstPluginKind // Must be last value to work with
                      // getNextAvailablePluginDiagnosticKind
 };
@@ -1118,20 +1117,6 @@ class DiagnosticInfoDontCall : public DiagnosticInfo {
   }
 };
 
-class DiagnosticInfoMachineFunctionSplit : public DiagnosticInfo {
-  StringRef TargetTriple;
-
-public:
-  DiagnosticInfoMachineFunctionSplit(StringRef TargetTriple,
-                                     DiagnosticSeverity DS)
-      : DiagnosticInfo(DK_MachineFunctionSplit, DS),
-        TargetTriple(TargetTriple) {}
-  void print(DiagnosticPrinter &DP) const override;
-  static bool classof(const DiagnosticInfo *DI) {
-    return DI->getKind() == DK_MachineFunctionSplit;
-  }
-};
-
 } // end namespace llvm
 
 #endif // LLVM_IR_DIAGNOSTICINFO_H

diff  --git a/llvm/lib/CodeGen/MachineFunctionSplitter.cpp b/llvm/lib/CodeGen/MachineFunctionSplitter.cpp
index 6b1b9520972e36..fbc071536d221a 100644
--- a/llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ b/llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,11 +35,9 @@
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/Passes.h"
-#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/TargetParser/Triple.h"
 #include <optional>
 
 using namespace llvm;
@@ -84,13 +82,6 @@ class MachineFunctionSplitter : public MachineFunctionPass {
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
   bool runOnMachineFunction(MachineFunction &F) override;
-
-  bool doInitialization(Module &) override;
-
-  static bool isSupportedTriple(const Triple &T) { return T.isX86(); }
-
-private:
-  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -136,20 +127,7 @@ static bool isColdBlock(const MachineBasicBlock &MBB,
   return (*Count < ColdCountThreshold);
 }
 
-bool MachineFunctionSplitter::doInitialization(Module &M) {
-  StringRef T = M.getTargetTriple();
-  if (!isSupportedTriple(Triple(T))) {
-    UnsupportedTriple = true;
-    M.getContext().diagnose(
-        DiagnosticInfoMachineFunctionSplit(T, DS_Warning));
-    return false;
-  }
-  return MachineFunctionPass::doInitialization(M);
-}
-
 bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) {
-  if (UnsupportedTriple)
-    return false;
   // We target functions with profile data. Static information in the form
   // of exception handling code may be split to cold if user passes the
   // mfs-split-ehcode flag.

diff  --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp
index 30a15664ede15f..342c4cbbc39d65 100644
--- a/llvm/lib/IR/DiagnosticInfo.cpp
+++ b/llvm/lib/IR/DiagnosticInfo.cpp
@@ -449,7 +449,3 @@ void DiagnosticInfoDontCall::print(DiagnosticPrinter &DP) const {
   if (!getNote().empty())
     DP << ": " << getNote();
 }
-
-void DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter &DP) const {
-  DP << "-fsplit-machine-functions is not valid for " << TargetTriple;
-}

diff  --git a/llvm/test/CodeGen/Generic/Inputs/fsloader-mfs.afdo b/llvm/test/CodeGen/Generic/Inputs/fsloader-mfs.afdo
deleted file mode 100644
index 1f9faf5ddfa686..00000000000000
Binary files a/llvm/test/CodeGen/Generic/Inputs/fsloader-mfs.afdo and /dev/null 
diff er

diff  --git a/llvm/test/CodeGen/Generic/machine-function-splitter.ll b/llvm/test/CodeGen/Generic/machine-function-splitter.ll
index 03934434f3ceb7..a3c2a7c7b31a91 100644
--- a/llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ b/llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -11,15 +11,6 @@
 ; RUN: sed 's/InstrProf/SampleProfile/g' %s > %t.ll
 ; RUN: llc < %t.ll -mtriple=x86_64-unknown-linux-gnu -split-machine-functions | FileCheck %s --check-prefix=FSAFDO-MFS
 ; RUN: llc < %t.ll -mtriple=x86_64-unknown-linux-gnu -split-machine-functions | FileCheck %s --check-prefix=FSAFDO-MFS2
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -debug-pass=Structure -fs-profile-file=%S/Inputs/fsloader-mfs.afdo -enable-fs-discriminator=true -improved-fs-discriminator=true -split-machine-functions 2>&1 | FileCheck %s --check-prefix=MFS_ON
-; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -debug-pass=Structure -fs-profile-file=%S/Inputs/fsloader-mfs.afdo -enable-fs-discriminator=true -improved-fs-discriminator=true -split-machine-functions 2>&1 | FileCheck %s --check-prefix=MFS_OFF
-
-;; Check that MFS is on for X86 targets.
-; MFS_ON: Machine Function Splitter Transformation
-; MFS_ON_NO: warning: -fsplit-machine-functions is not valid for
-;; Check that MFS is not on for non-X86 targets.
-; MFS_OFF: warning: -fsplit-machine-functions is not valid for
-; MFS_OFF_NO: Machine Function Splitter Transformation
 
 define void @foo1(i1 zeroext %0) nounwind !prof !14 !section_prefix !15 {
 ;; Check that cold block is moved to .text.split.


        


More information about the cfe-commits mailing list