[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-05 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: lld/ELF/LTO.cpp:80
   // Check if basic block sections must be used.
   // Allowed values for --lto-basicblock-sections are "all", "labels",
   // "", or none.  This is the equivalent

Maybe "--lto-basicblock-sections" should be changed to 
"--lto-basic-block-sections" too?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68049/new/

https://reviews.llvm.org/D68049



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-07-31 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment.

Ping? Can we make a decision on this? 
I've this simple one D35849: [UBSan] Provide default blacklist filename for 
UBSan  which, depending on this, shall be 
discarded or move forward.
If this CL stalls, I'll seek to proceed with https://reviews.llvm.org/D35849. 
Any how, https://reviews.llvm.org/D35849 gets wiped out whence this CL 
(https://reviews.llvm.org/D32842) is submitted.


https://reviews.llvm.org/D32842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment.

In D85408#2253055 , @MaskRay wrote:

> I am still reading the patch, but I have noticed one thing worth discussing. 
> `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the 
> memory image). An absolute relocation type (`.quad .Lfunc_begin0`) works but 
> the value is a link-time address, not taking account of the load base 
> (PIE/shared object)). (If .bb_addr_map has the SHF_ALLOC flag, linkers will 
> report a text relocation) How do you intend to use `.bb_addr_map` at runtime?

Regarding "not taking account of the load base (PIE/shared object)" - the 
profile conversion tool uses addresses in .bb_addr_map section and MMAP entries 
in perf.data file to construct runtime addresses, so these runtime addresses 
can be matched with the profile addresses recorded in perf.data file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85408/new/

https://reviews.llvm.org/D85408

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Han Shen via Phabricator via cfe-commits
shenhan created this revision.
shenhan added reviewers: xur, snehasish.
Herald added subscribers: mattd, asavonic, pengfei, hiraditya.
Herald added a project: All.
shenhan requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

When building a fatbinary, the driver invokes the compiler multiple times with 
different "--target". (For example, with "-x cuda --cuda-gpu-arch=sm_70" flags, 
clang will be invoded twice, once with --target=x86_64_, once with 
--target=sm_70) If we use -fsplit-machine-functions or 
-fno-split-machine-functions for such invocation, the driver reports an error.

This CL changes the behavior so:

1. "-fsplit-machine-functions" is now passed to all targets, for non-X86 
targets, the flag is a NOOP and causes a warning.
2. "-fno-split-machine-functions" now negates -fsplit-machine-functions (if 
-fno-split-machine-functions appears after any -fsplit-machine-functions) for 
any target triple, previously, it causes an error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions2.c
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/test/CodeGen/X86/mfs-triple.ll

Index: llvm/test/CodeGen/X86/mfs-triple.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/mfs-triple.ll
@@ -0,0 +1,38 @@
+; 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_WARN
+
+; MFS_ON: Machine Function Splitter Transformation
+; MFS_WARN: warning: -fsplit-machine-functions is only valid for X86.
+; MFS_WARN_NO: Machine Function Splitter Transformation
+
+define void @foo4(i1 zeroext %0, i1 zeroext %1) nounwind {
+  br i1 %0, label %3, label %7
+
+3:
+  %4 = call i32 @bar()
+  br label %7
+
+5:
+  %6 = call i32 @baz()
+  br label %7
+
+7:
+  br i1 %1, label %8, label %10
+
+8:
+  %9 = call i32 @bam()
+  br label %12
+
+10:
+  %11 = call i32 @baz()
+  br label %12
+
+12:
+  %13 = tail call i32 @qux()
+  ret void
+}
+
+declare i32 @bar()
+declare i32 @baz()
+declare i32 @bam()
+declare i32 @qux()
Index: llvm/lib/CodeGen/TargetPassConfig.cpp
===
--- llvm/lib/CodeGen/TargetPassConfig.cpp
+++ llvm/lib/CodeGen/TargetPassConfig.cpp
@@ -1275,7 +1275,11 @@
"performance.";
   }
 }
-addPass(createMachineFunctionSplitterPass());
+if (TM->getTargetTriple().isX86())
+  addPass(createMachineFunctionSplitterPass());
+else
+  WithColor::warning()
+  << "-fsplit-machine-functions is only valid for X86.\n";
   }
 
   addPostBBSections();
Index: clang/test/Driver/fsplit-machine-functions2.c
===
--- clang/test/Driver/fsplit-machine-functions2.c
+++ clang/test/Driver/fsplit-machine-functions2.c
@@ -7,6 +7,9 @@
 // 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 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 that for non-X86, passing no-split-machine-functions does not cause error.
+// RUN: %clang -### -target aarch64-unknown-linux -flto -fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS2
 
 // CHECK-PASS:  "-plugin-opt=-split-machine-functions"
 // CHECK-NOPASS-NOT:"-plugin-opt=-split-machine-functions"
+// CHECK-NOPASS2-NOT:   "-plugin-opt=-split-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,8 +1,8 @@
 // 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-functio

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9
+
+// Check that -fsplit-machine-functions is passed to both x86 and cuda 
compilation and does not cause driver error.
+// MFS2: -fsplit-machine-functions

tra wrote:
> We will still see a warning, right? So, for someone compiling with `-Werror` 
> that's going to be a problem.
> 
> Also, if the warning is issued from the top-level driver, we may not even be 
> able to suppress it when we disable splitting on GPU side with `-Xarch_device 
> -fno-split-machine-functions`.
> 
> 
> We will still see a warning, right?
Yes, there still will be a warning. We've discussed it and we think that pass 
-fsplit-machine-functions in this case is not a proper usage and a warning is 
warranted, and it is not good that skip doing split silently while uses 
explicitly ask for it.

> Also, if the warning is issued from the top-level driver
The warning will not be issued from the top-level driver, it will be issued 
when configuring optimization passes.
So:


  - -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
Will enable MFS for host, disable MFS for gpus and without any warnings.

  - -Xarch_host -fsplit-machine-functions
The same as the above

  - -Xarch_host -fsplit-machine-functions -Xarch_device 
-fno-split-machine-functions
The same as the above



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1278
 }
-addPass(createMachineFunctionSplitterPass());
+if (TM->getTargetTriple().isX86())
+  addPass(createMachineFunctionSplitterPass());

snehasish wrote:
> Can you coordinate with @dhoekwater ? He has some patches in flight for 
> AArch64. 
> 
> I think D157157 is the one which modifies the same logic.
Thanks. Yes, I'll coordinate with @dhoekwater before resolving this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-14 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9
+
+// Check that -fsplit-machine-functions is passed to both x86 and cuda 
compilation and does not cause driver error.
+// MFS2: -fsplit-machine-functions

tra wrote:
> shenhan wrote:
> > tra wrote:
> > > We will still see a warning, right? So, for someone compiling with 
> > > `-Werror` that's going to be a problem.
> > > 
> > > Also, if the warning is issued from the top-level driver, we may not even 
> > > be able to suppress it when we disable splitting on GPU side with 
> > > `-Xarch_device -fno-split-machine-functions`.
> > > 
> > > 
> > > We will still see a warning, right?
> > Yes, there still will be a warning. We've discussed it and we think that 
> > pass -fsplit-machine-functions in this case is not a proper usage and a 
> > warning is warranted, and it is not good that skip doing split silently 
> > while uses explicitly ask for it.
> > 
> > > Also, if the warning is issued from the top-level driver
> > The warning will not be issued from the top-level driver, it will be issued 
> > when configuring optimization passes.
> > So:
> > 
> > 
> >   - -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
> > Will enable MFS for host, disable MFS for gpus and without any warnings.
> > 
> >   - -Xarch_host -fsplit-machine-functions
> > The same as the above
> > 
> >   - -Xarch_host -fsplit-machine-functions -Xarch_device 
> > -fno-split-machine-functions
> > The same as the above
> > 
> > We've discussed it and we think that pass -fsplit-machine-functions in this 
> > case is not a proper usage and a warning is warranted, and it is not good 
> > that skip doing split silently while uses explicitly ask for it.
> 
> I would agree with that assertion if we were talking exclusively about CUDA 
> compilation.
> However, a common real world use pattern is that the flags are set globally 
> for all C++ compilations, and then CUDA compilations within the project need 
> to do whatever they need to to keep things working. The original user intent 
> was for the option to affect the host compilation. There's no inherent 
> assumption that it will do anything useful for the GPU.
> 
> In number of similar cases in the past we did settle on silently ignoring 
> some top-level flags that we do expect to encounter in real projects, but 
> which made no sense for the GPU. E.g. sanitizers. If the project is built w/ 
> sanitizer enabled, the idea is to sanitize the host code, The GPU code 
> continues to be built w/o sanitizer enabled. 
> 
> Anyways, as long as we have a way to deal with it it's not a big deal one way 
> or another.
> 
> > -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
> > Will enable MFS for host, disable MFS for gpus and without any warnings.
> 
> OK. This will work.
> 
> 
> In number of similar cases in the past we did settle on silently ignoring 
> some top-level flags that we do expect to encounter in real projects, but 
> which made no sense for the GPU. E.g. sanitizers. If the project is built w/ 
> sanitizer enabled, the idea is to sanitize the host code, The GPU code 
> continues to be built w/o sanitizer enabled.

Can I understand it this way - if the compiler is **only** building for CPUs, 
then silently ignore any optimization flags is not a good behavior. If the 
compiler is building CPUs and GPUs, it is still not a good behavior to silently 
ignore optimization flags for CPUs, but it is probably ok to silently ignore 
optimization flags for GPUs.

> OK. This will work.
Thanks for confirming.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1281-1282
+else
+  WithColor::warning()
+  << "-fsplit-machine-functions is only valid for X86.\n";
   }

arsenm wrote:
> You cannot spam warnings here. The other instance of printing here looks like 
> a new addition and should be removed
Thanks. Do you suggest moving the warnings to the underlying pass? (Although 
that means we create passes that only issue warnings.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-14 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 550137.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/CodeGen/MachineFunctionSplitter.cpp
  llvm/lib/IR/DiagnosticInfo.cpp

Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -449,3 +449,7 @@
   if (!getNote().empty())
 DP << ": " << getNote();
 }
+
+void DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter &DP) const {
+  DP << "-fsplit-machine-functions is not valid for " << TargetTriple;
+}
Index: llvm/lib/CodeGen/MachineFunctionSplitter.cpp
===
--- llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,9 +35,11 @@
 #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 
 
 using namespace llvm;
@@ -82,6 +84,11 @@
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
   bool runOnMachineFunction(MachineFunction &F) override;
+
+  bool doInitialization(Module &) override;
+
+private:
+  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -127,7 +134,20 @@
   return (*Count < ColdCountThreshold);
 }
 
+bool MachineFunctionSplitter::doInitialization(Module &M) {
+  StringRef TripleStr = M.getTargetTriple();
+  if (!Triple(TripleStr).isX86()) {
+UnsupportedTriple = true;
+M.getContext().diagnose(
+DiagnosticInfoMachineFunctionSplit(TripleStr, 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.
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -86,6 +86,7 @@
   DK_SrcMgr,
   DK_DontCall,
   DK_MisExpect,
+  DK_MachineFunctionSplit,
   DK_FirstPluginKind // Must be last value to work with
  // getNextAvailablePluginDiagnosticKind
 };
@@ -1117,6 +1118,20 @@
   }
 };
 
+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
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,8 +1,8 @@
 // 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
+// RUN: %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:error: unsupported option '-fsplit-machine-functions' for target
+// CHECK-TRIPLE:warning: -fsplit-machine-functions is only valid for X86.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5859,14 +5859,15 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This co

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-14 Thread Han Shen via Phabricator via cfe-commits
shenhan marked an inline comment as done.
shenhan added inline comments.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1281-1282
+else
+  WithColor::warning()
+  << "-fsplit-machine-functions is only valid for X86.\n";
   }

arsenm wrote:
> shenhan wrote:
> > arsenm wrote:
> > > You cannot spam warnings here. The other instance of printing here looks 
> > > like a new addition and should be removed
> > Thanks. Do you suggest moving the warnings to the underlying pass? 
> > (Although that means we create passes that only issue warnings.)
> Move it to the pass, and use a backend remark, not directly print to the 
> console (e.g. DiagnosticInfoUnsupported)
Thanks, created DiagnosticInfoMachineFunctionSplit and moved the warning to MFS 
pass.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-15 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 550423.
shenhan marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  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/X86/machine-function-splitter.ll

Index: llvm/test/CodeGen/X86/machine-function-splitter.ll
===
--- llvm/test/CodeGen/X86/machine-function-splitter.ll
+++ llvm/test/CodeGen/X86/machine-function-splitter.ll
@@ -5,6 +5,15 @@
 ; 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.
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -449,3 +449,7 @@
   if (!getNote().empty())
 DP << ": " << getNote();
 }
+
+void DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter &DP) const {
+  DP << "-fsplit-machine-functions is not valid for " << TargetTriple;
+}
Index: llvm/lib/CodeGen/MachineFunctionSplitter.cpp
===
--- llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,9 +35,11 @@
 #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 
 
 using namespace llvm;
@@ -82,6 +84,13 @@
   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
 
@@ -127,7 +136,20 @@
   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.
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -86,6 +86,7 @@
   DK_SrcMgr,
   DK_DontCall,
   DK_MisExpect,
+  DK_MachineFunctionSplit,
   DK_FirstPluginKind // Must be last value to work with
  // getNextAvailablePluginDiagnosticKind
 };
@@ -1117,6 +1118,20 @@
   }
 };
 
+class DiagnosticInfoMachineFunctionSplit : public DiagnosticInfo {
+  StringRef TargetTriple;
+
+public:
+  DiagnosticInfoMachineFunctionSplit(StringRef TargetTriple,
+ DiagnosticSeverity DS)
+  : DiagnosticInfo(DK_MachineFunctionSplit, DS),
+TargetTriple(TargetTriple) {}
+  void print(Diagnost

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-15 Thread Han Shen via Phabricator via cfe-commits
shenhan marked 3 inline comments as done.
shenhan added inline comments.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1278
 }
-addPass(createMachineFunctionSplitterPass());
+if (TM->getTargetTriple().isX86())
+  addPass(createMachineFunctionSplitterPass());

shenhan wrote:
> snehasish wrote:
> > Can you coordinate with @dhoekwater ? He has some patches in flight for 
> > AArch64. 
> > 
> > I think D157157 is the one which modifies the same logic.
> Thanks. Yes, I'll coordinate with @dhoekwater before resolving this.
@dhoekwater will rebase D157157 on top of this.



Comment at: llvm/test/CodeGen/X86/mfs-triple.ll:8
+
+define void @foo4(i1 zeroext %0, i1 zeroext %1) nounwind {
+  br i1 %0, label %3, label %7

snehasish wrote:
> Any reason why we can't use the bitcode already in 
> test/CodeGen/machine-function-splitter.ll? (Going to be moved to 
> test/Generic/machine-function-splitter.ll in D157563)
> 
> IMO we can just reuse the basic test and add these run and check lines.
Moved the tests into machine-function-splitter.ll. Either this CL or D157563 
can be submitted first, and the other will rebase on top of that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-16 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 550834.
shenhan marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  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/Inputs/fsloader-mfs.afdo
  llvm/test/CodeGen/Generic/machine-function-splitter.ll
  llvm/test/CodeGen/X86/Inputs/fsloader-mfs.afdo

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -10,6 +10,15 @@
 ; 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.
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -449,3 +449,7 @@
   if (!getNote().empty())
 DP << ": " << getNote();
 }
+
+void DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter &DP) const {
+  DP << "-fsplit-machine-functions is not valid for " << TargetTriple;
+}
Index: llvm/lib/CodeGen/MachineFunctionSplitter.cpp
===
--- llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,9 +35,11 @@
 #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 
 
 using namespace llvm;
@@ -82,6 +84,13 @@
   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
 
@@ -127,7 +136,20 @@
   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.
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -86,6 +86,7 @@
   DK_SrcMgr,
   DK_DontCall,
   DK_MisExpect,
+  DK_MachineFunctionSplit,
   DK_FirstPluginKind // Must be last value to work with
  // getNextAvailablePluginDiagnosticKind
 };
@@ -1117,6 +1118,20 @@
   }
 };
 
+class DiagnosticInfoMachineFunctionSplit : public DiagnosticInfo {
+  StringRef TargetTriple;
+
+public:
+  DiagnosticInfoMachineFunctionSplit(StringRef TargetTriple,
+ DiagnosticSeverity

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-16 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: llvm/test/CodeGen/X86/mfs-triple.ll:8
+
+define void @foo4(i1 zeroext %0, i1 zeroext %1) nounwind {
+  br i1 %0, label %3, label %7

shenhan wrote:
> snehasish wrote:
> > Any reason why we can't use the bitcode already in 
> > test/CodeGen/machine-function-splitter.ll? (Going to be moved to 
> > test/Generic/machine-function-splitter.ll in D157563)
> > 
> > IMO we can just reuse the basic test and add these run and check lines.
> Moved the tests into machine-function-splitter.ll. Either this CL or D157563 
> can be submitted first, and the other will rebase on top of that.
Rebased on D157563.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-16 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 550885.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  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/Inputs/fsloader-mfs.afdo
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -10,6 +10,15 @@
 ; 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.
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -449,3 +449,7 @@
   if (!getNote().empty())
 DP << ": " << getNote();
 }
+
+void DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter &DP) const {
+  DP << "-fsplit-machine-functions is not valid for " << TargetTriple;
+}
Index: llvm/lib/CodeGen/MachineFunctionSplitter.cpp
===
--- llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,9 +35,11 @@
 #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 
 
 using namespace llvm;
@@ -82,6 +84,13 @@
   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
 
@@ -127,7 +136,20 @@
   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.
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -86,6 +86,7 @@
   DK_SrcMgr,
   DK_DontCall,
   DK_MisExpect,
+  DK_MachineFunctionSplit,
   DK_FirstPluginKind // Must be last value to work with
  // getNextAvailablePluginDiagnosticKind
 };
@@ -1117,6 +1118,20 @@
   }
 };
 
+class DiagnosticInfoMachineFunctionSplit : public DiagnosticInfo {
+  StringRef TargetTriple;
+
+public:
+  DiagnosticInfoMachineFunctionSplit(StringRef TargetTriple,
+ DiagnosticSeverity DS)
+  : DiagnosticInfo(DK_MachineFunctionSplit, DS),
+TargetTriple(TargetTrip

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-16 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 550913.
shenhan added a comment.

Refined the test case a little bit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  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/Inputs/fsloader-mfs.afdo
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -10,6 +10,15 @@
 ; 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.
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -449,3 +449,7 @@
   if (!getNote().empty())
 DP << ": " << getNote();
 }
+
+void DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter &DP) const {
+  DP << "-fsplit-machine-functions is not valid for " << TargetTriple;
+}
Index: llvm/lib/CodeGen/MachineFunctionSplitter.cpp
===
--- llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,9 +35,11 @@
 #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 
 
 using namespace llvm;
@@ -82,6 +84,13 @@
   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
 
@@ -127,7 +136,20 @@
   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.
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -86,6 +86,7 @@
   DK_SrcMgr,
   DK_DontCall,
   DK_MisExpect,
+  DK_MachineFunctionSplit,
   DK_FirstPluginKind // Must be last value to work with
  // getNextAvailablePluginDiagnosticKind
 };
@@ -1117,6 +1118,20 @@
   }
 };
 
+class DiagnosticInfoMachineFunctionSplit : public DiagnosticInfo {
+  StringRef TargetTriple;
+
+public:
+  DiagnosticInfoMachineFunctionSplit(StringRef TargetTriple,
+ DiagnosticSeverity DS)
+  : DiagnosticInfo

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-16 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment.

In D157750#4593582 , @dhoekwater 
wrote:

> This patch will make it difficult to write tests for MFS on AArch64 before it 
> is officially enabled. Currently, because clang performs the Triple check, we 
> can use `-enable-split-machine-functions` to run tests with MFS on Arm, but 
> after this patch the flag won't do anything. Is there a way that we can land 
> this patch while still making MFS testable on AArch64?

Discussed with @dhoekwater offline, we decide to use a temporary hidden flag, 
something like "enable-mfs-for-debugging/testing", to force enable MFS for any 
triple during the period when MFS for arm is progressively rolled out, and when 
all is done, we will remove that temporary flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-16 Thread Han Shen via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG317a0fe5bd71: [Driver][CodeGen] Properly handle 
-fsplit-machine-functions for fatbinary… (authored by shenhan).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  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/Inputs/fsloader-mfs.afdo
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -10,6 +10,15 @@
 ; 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.
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -449,3 +449,7 @@
   if (!getNote().empty())
 DP << ": " << getNote();
 }
+
+void DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter &DP) const {
+  DP << "-fsplit-machine-functions is not valid for " << TargetTriple;
+}
Index: llvm/lib/CodeGen/MachineFunctionSplitter.cpp
===
--- llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,9 +35,11 @@
 #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 
 
 using namespace llvm;
@@ -82,6 +84,13 @@
   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
 
@@ -127,7 +136,20 @@
   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.
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -86,6 +86,7 @@
   DK_SrcMgr,
   DK_DontCall,
   DK_MisExpect,
+  DK_MachineFunctionSplit,
   DK_FirstPluginKind // Must be last value to work with
  // getNextAvailablePluginDiagnosticKind
 };
@@ -1117,6 +1118,20 @@
   }
 };
 
+class DiagnosticInfoMachineFunctionSplit : public DiagnosticInfo {
+  StringRef TargetTriple;
+
+p

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-17 Thread Han Shen via Phabricator via cfe-commits
shenhan created this revision.
shenhan added reviewers: MaskRay, Hahnfeld, steelannelida.
Herald added subscribers: mattd, asavonic.
Herald added a project: All.
shenhan requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This CL includes two changes:

1. move clang backend-warnings test cases from Driver/ to CodeGen/.
2. Replace multiple `cd "$(dirname "%t")"` with a single `rm -fr "%t"; mkdir -p 
"%t"; cd "%t"`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158231

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,10 @@
 
 ;; 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
+; MFS_ON-NOT: 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,9 @@
-// REQUIRES: arm-registered-target
+// REQUIRES: x86-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-OPT1 %s
+// RUN: %clang -### -target x86_64 -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT2 %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
 
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK-OPT1:   "-fsplit-machine-functions"
+// CHECK-OPT2:   "-fsplit-machine-functions"
+// CHECK-NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,29 @@
 // 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: %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: %clang -### --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
 // RUN: --cuda-gpu-arch=sm_7

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-17 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment.

Thanks all. Created D158231  to address the 
post-submit comments and added all as reviewers.  @MaskRay @Hahnfeld 
@steelannelida


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157750/new/

https://reviews.llvm.org/D157750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-17 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 551332.
shenhan marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,11 @@
 
 ;; 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
+; MFS_ON-NOT: warning: -fsplit-machine-functions is not valid for
+; MFS_ON-NOT: warning:
 ;; 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,7 @@
-// REQUIRES: arm-registered-target
+// REQUIRES: x86-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 -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
 
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK_OPT:"-fsplit-machine-functions"
+// CHECK_NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,29 @@
 // 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: %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: %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_ho

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-17 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: clang/test/CodeGen/fsplit-machine-functions.c:41
+
+// RUN: %clang -c -target arm-unknown-linux-gnueabi -fsplit-machine-functions 
%s \
+// RUN: 2>&1 | FileCheck -check-prefix=MFS5 %s

MaskRay wrote:
> `-target ` has been deprecated since Clang 3.4. Use `--target=`
> 
> 
> The warning test should be moved to `Driver/`.
> Whenever a warning can be placed either in Driver or in Frontend, we prefer 
> Driver.
> -target  has been deprecated since Clang 3.4. Use --target=

Replaced with --target=arm-unknown-linux-gnueabi

> Whenever a warning can be placed either in Driver or in Frontend, we prefer 
> Driver.

Just a little bit confused, you mentioned in [the other 
comment](https://reviews.llvm.org/D157750#inline-1530250)  that `Driver tests 
should not run the backend. Most driver commands should use -###.`
And the warning to be tested here is  issued from backend, so -### does not 
trigger it at all, so I moved the tests from Driver/ to here . Do you suggest 
move these back to Driver/?



Comment at: clang/test/Driver/fsplit-machine-functions.c:7
 
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK-OPT1:   "-fsplit-machine-functions"
+// CHECK-OPT2:   "-fsplit-machine-functions"

MaskRay wrote:
> if OPT1 is identical OPT2, we should use the same prefix.
> 
> If you want to test `-fprofile-use=`, test the extra CC1 option from the 
> driver `-fprofile-use=`.
After a second thought, test of "fprofile-use" is irrelevant to 
fsplit-machine-functions flag, removed the test lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 551583.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,11 @@
 
 ;; 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
+; MFS_ON-NOT: warning: -fsplit-machine-functions is not valid for
+; MFS_ON-NOT: warning:
 ;; 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,7 @@
-// REQUIRES: arm-registered-target
+// REQUIRES: x86-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 -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
 
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK_OPT:"-fsplit-machine-functions"
+// CHECK_NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,29 @@
 // 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: %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: %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-function

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
shenhan marked an inline comment as done.
shenhan added inline comments.



Comment at: clang/test/CodeGen/fsplit-machine-functions.c:41
+
+// RUN: %clang -c -target arm-unknown-linux-gnueabi -fsplit-machine-functions 
%s \
+// RUN: 2>&1 | FileCheck -check-prefix=MFS5 %s

MaskRay wrote:
> shenhan wrote:
> > MaskRay wrote:
> > > `-target ` has been deprecated since Clang 3.4. Use `--target=`
> > > 
> > > 
> > > The warning test should be moved to `Driver/`.
> > > Whenever a warning can be placed either in Driver or in Frontend, we 
> > > prefer Driver.
> > > -target  has been deprecated since Clang 3.4. Use --target=
> > 
> > Replaced with --target=arm-unknown-linux-gnueabi
> > 
> > > Whenever a warning can be placed either in Driver or in Frontend, we 
> > > prefer Driver.
> > 
> > Just a little bit confused, you mentioned in [the other 
> > comment](https://reviews.llvm.org/D157750#inline-1530250)  that `Driver 
> > tests should not run the backend. Most driver commands should use -###.`
> > And the warning to be tested here is  issued from backend, so -### does not 
> > trigger it at all, so I moved the tests from Driver/ to here . Do you 
> > suggest move these back to Driver/?
> `test/Driver` tests `%clang -###` and driver options.
> 
> Other directories (e.g. `test/CodeGen`) test `%clang_cc1` and CC1 options. 
> `%clang ` is discouraged.
Thanks. Now test/CodeGen/ tests are using %clang_cc1 and test/Driver/ tests are 
using %clang -###.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 551630.
shenhan marked 3 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,11 @@
 
 ;; 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
+; MFS_ON-NOT: warning: -fsplit-machine-functions is not valid for
+; MFS_ON-NOT: warning:
 ;; 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,5 @@
-// REQUIRES: arm-registered-target
+// 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: %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
-
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK_OPT:"-fsplit-machine-functions"
+// CHECK_NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,29 @@
 // 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: %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: %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-

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 551632.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,11 @@
 
 ;; 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
+; MFS_ON-NOT: warning: -fsplit-machine-functions is not valid for
+; MFS_ON-NOT: warning:
 ;; 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,5 @@
-// REQUIRES: arm-registered-target
+// 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: %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
-
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK_OPT:"-fsplit-machine-functions"
+// CHECK_NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,28 @@
-// 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: %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: %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  "

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: clang/test/CodeGen/fsplit-machine-functions.c:1
+// REQUIRES: system-linux
+// REQUIRES: x86-registered-target

MaskRay wrote:
> Any reason `system-linux` is needed?
Deleted. Also deleted "REQUIRES: system-linux" from 
"fsplit-machine-functions-with-cuda-nvptx.c".



Comment at: clang/test/CodeGen/fsplit-machine-functions.c:2
+// REQUIRES: system-linux
+// REQUIRES: x86-registered-target
+// REQUIRES: arm-registered-target

MaskRay wrote:
> We usually place x86 tests under CodeGen/X86 and arm tests under CodeGen/ARM
I can moved the X86 cases to clang/test/CodeGen/X86, but there is no 
clang/test/CodeGen/Arm or similar, shall I create "clang/test/CodeGen/Arm" 
instead? 



Comment at: clang/test/Driver/fsplit-machine-functions.c:1
-// REQUIRES: arm-registered-target
+// REQUIRES: x86-registered-target
 

MaskRay wrote:
> If you only use `-###`, `// REQUIRES: x86-registered-target` is unneeded.
Ah, yes. Deleted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 551634.
shenhan marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,10 @@
 
 ;; 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
+; MFS_ON-NOT: warning:
 ;; 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,5 @@
-// REQUIRES: arm-registered-target
+// 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: %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
-
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK_OPT:"-fsplit-machine-functions"
+// CHECK_NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,28 @@
-// 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: %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: %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-

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 551680.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158231/new/

https://reviews.llvm.org/D158231

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,10 @@
 
 ;; 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
+; MFS_ON-NOT: warning:
 ;; 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,5 @@
-// REQUIRES: arm-registered-target
+// 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: %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
-
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK_OPT:"-fsplit-machine-functions"
+// CHECK_NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,28 @@
-// 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: %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: %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 

[PATCH] D157565: [CodeGen] Add AArch64 behavior to existing MFS tests

2023-08-18 Thread Han Shen via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9d079d6188b: [clang][test] Refine clang 
machine-function-split tests. (authored by shenhan).
Herald added subscribers: cfe-commits, mattd, asavonic.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D157565?vs=551617&id=551688#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157565/new/

https://reviews.llvm.org/D157565

Files:
  clang/test/CodeGen/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  llvm/test/CodeGen/Generic/machine-function-splitter.ll

Index: llvm/test/CodeGen/Generic/machine-function-splitter.ll
===
--- llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -16,10 +16,10 @@
 
 ;; 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
+; MFS_ON-NOT: warning:
 ;; 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
+; MFS_OFF-NOT: 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.
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,10 +1,5 @@
-// REQUIRES: arm-registered-target
+// 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: %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
-
-// CHECK-OPT:   "-fsplit-machine-functions"
-// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
-// CHECK-TRIPLE:warning: -fsplit-machine-functions is not valid for arm
+// CHECK_OPT:"-fsplit-machine-functions"
+// CHECK_NOOPT-NOT:  "-fsplit-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
===
--- clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
+++ clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
@@ -1,68 +1,28 @@
-// 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: %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: %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 -

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-21 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9
+
+// Check that -fsplit-machine-functions is passed to both x86 and cuda 
compilation and does not cause driver error.
+// MFS2: -fsplit-machine-functions

MaskRay wrote:
> MaskRay wrote:
> > tra wrote:
> > > shenhan wrote:
> > > > tra wrote:
> > > > > shenhan wrote:
> > > > > > tra wrote:
> > > > > > > We will still see a warning, right? So, for someone compiling 
> > > > > > > with `-Werror` that's going to be a problem.
> > > > > > > 
> > > > > > > Also, if the warning is issued from the top-level driver, we may 
> > > > > > > not even be able to suppress it when we disable splitting on GPU 
> > > > > > > side with `-Xarch_device -fno-split-machine-functions`.
> > > > > > > 
> > > > > > > 
> > > > > > > We will still see a warning, right?
> > > > > > Yes, there still will be a warning. We've discussed it and we think 
> > > > > > that pass -fsplit-machine-functions in this case is not a proper 
> > > > > > usage and a warning is warranted, and it is not good that skip 
> > > > > > doing split silently while uses explicitly ask for it.
> > > > > > 
> > > > > > > Also, if the warning is issued from the top-level driver
> > > > > > The warning will not be issued from the top-level driver, it will 
> > > > > > be issued when configuring optimization passes.
> > > > > > So:
> > > > > > 
> > > > > > 
> > > > > >   - -fsplit-machine-functions -Xarch_device 
> > > > > > -fno-split-machine-functions
> > > > > > Will enable MFS for host, disable MFS for gpus and without any 
> > > > > > warnings.
> > > > > > 
> > > > > >   - -Xarch_host -fsplit-machine-functions
> > > > > > The same as the above
> > > > > > 
> > > > > >   - -Xarch_host -fsplit-machine-functions -Xarch_device 
> > > > > > -fno-split-machine-functions
> > > > > > The same as the above
> > > > > > 
> > > > > > We've discussed it and we think that pass -fsplit-machine-functions 
> > > > > > in this case is not a proper usage and a warning is warranted, and 
> > > > > > it is not good that skip doing split silently while uses explicitly 
> > > > > > ask for it.
> > > > > 
> > > > > I would agree with that assertion if we were talking exclusively 
> > > > > about CUDA compilation.
> > > > > However, a common real world use pattern is that the flags are set 
> > > > > globally for all C++ compilations, and then CUDA compilations within 
> > > > > the project need to do whatever they need to to keep things working. 
> > > > > The original user intent was for the option to affect the host 
> > > > > compilation. There's no inherent assumption that it will do anything 
> > > > > useful for the GPU.
> > > > > 
> > > > > In number of similar cases in the past we did settle on silently 
> > > > > ignoring some top-level flags that we do expect to encounter in real 
> > > > > projects, but which made no sense for the GPU. E.g. sanitizers. If 
> > > > > the project is built w/ sanitizer enabled, the idea is to sanitize 
> > > > > the host code, The GPU code continues to be built w/o sanitizer 
> > > > > enabled. 
> > > > > 
> > > > > Anyways, as long as we have a way to deal with it it's not a big deal 
> > > > > one way or another.
> > > > > 
> > > > > > -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
> > > > > > Will enable MFS for host, disable MFS for gpus and without any 
> > > > > > warnings.
> > > > > 
> > > > > OK. This will work.
> > > > > 
> > > > > 
> > > > > In number of similar cases in the past we did settle on silently 
> > > > > ignoring some top-level flags that we do expect to encounter in real 
> > > > > projects, but which made no sense for the GPU. E.g. sanitizers. If 
> > > > > the project is built w/ sanitizer enabled, the idea is to sanitize 
> > > > > the host code, The GPU code continues to be built w/o sanitizer 
> > > > > enabled.
> > > > 
> > > > Can I understand it this way - if the compiler is **only** building for 
> > > > CPUs, then silently ignore any optimization flags is not a good 
> > > > behavior. If the compiler is building CPUs and GPUs, it is still not a 
> > > > good behavior to silently ignore optimization flags for CPUs, but it is 
> > > > probably ok to silently ignore optimization flags for GPUs.
> > > > 
> > > > > OK. This will work.
> > > > Thanks for confirming.
> > > >  it is probably ok to silently ignore optimization flags for GPUs.
> > > 
> > > In this case, yes. 
> > > 
> > > I think the most consistent way to handle the situation is to keep the 
> > > warning in place at cc1 compiler level, but change the driver behavior 
> > > (and document it) so that it does not pass the splitting options to 
> > > offloading sub-compilations. This way we'll do the sensible thing for the 
> > > most common use case, yet would still warn if the user tries to enable 
> > > the splitting where they should not (e.g. by using `-Xclang 
> > > -fsplit-machine-functio

[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Han Shen via Phabricator via cfe-commits
shenhan created this revision.
shenhan added a reviewer: MaskRay.
Herald added a subscriber: pengfei.
Herald added a project: All.
shenhan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, when clang reports an error when -fno-split-machine-functions is 
used for non-X86 archs.

  

However, in some cases, users may specify flags as "-fsplit-machine-functions 
-fother-flags -fno-split-machine-functions", the first one is from a global 
flag set, the last one is used to negate the global flag, we think this is a 
valid usage mode.

Another cases is when clang is used to invoke multiple workloads, like "-x cuda 
-fsplit-machine-functions -Xarch_device -fno-split-machine-functions", the 
latter is used to negate the global -fsplit-machine-functions when invoke 
workloads for GPU."

  

This change makes this work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158755

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 553219.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158755/new/

https://reviews.llvm.org/D158755

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions.c


Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -15,6 +15,10 @@
 // RUN: not %clang -### -c --target=arm-unknown-linux 
-fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR
 // ERR: error: unsupported option '-fsplit-machine-functions' for target
 
-/// FIXME
-// RUN: not %clang -### -c --target=arm-unknown-linux 
-fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s 
--check-prefix=ERR2
-// ERR2: error: unsupported option '-fno-split-machine-functions' for target
+// RUN: not %clang -### -x cuda -S -nocudainc -nocudalib 
-fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR2
+// ERR2: clang: error: unsupported option '-fsplit-machine-functions' for 
target
+
+// The following all run successfully.
+// RUN: %clang -### -x cuda -S -nocudainc -nocudalib -fsplit-machine-functions 
-Xarch_device -fno-split-machine-functions %s
+// RUN: %clang -### -x cuda -S -nocudainc -nocudalib -Xarch_host 
-fsplit-machine-functions %s
+// RUN: %clang -### --target=arm-unknown-linux -fsplit-machine-functions 
-fno-split-machine-functions %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 


Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -15,6 +15,10 @@
 // RUN: not %clang -### -c --target=arm-unknown-linux -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR
 // ERR: error: unsupported option '-fsplit-machine-functions' for target
 
-/// FIXME
-// RUN: not %clang -### -c --target=arm-unknown-linux -fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR2
-// ERR2: error: unsupported option '-fno-split-machine-functions' for target
+// RUN: not %clang -### -x cuda -S -nocudainc -nocudalib -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR2
+// ERR2: clang: error: unsupported option '-fsplit-machine-functions' for target
+
+// The following all run successfully.
+// RUN: %clang -### -x cuda -S -nocudainc -nocudalib -fsplit-machine-functions -Xarch_device -fno-split-machine-functions %s
+// RUN: %clang -### -x cuda -S -nocudainc -nocudalib -Xarch_host -fsplit-machine-functions %s
+// RUN: %clang -### --target=arm-unknown-linux -fsplit-machine-functions -fno-split-machine-functions %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@l

[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment.

In D158755#4614566 , @MaskRay wrote:

> Thanks for the patch. We need a test. I cleaned up 
> `fsplit-machine-functions.c` a bit. You can rebase and modify the `not %clang 
> -### -c --target=arm-unknown-linux -fsplit-machine-functions 
> -fno-split-machine-functions` RUN line in 
> `clang/test/Driver/fsplit-machine-function.c`

Thanks. Fixed the case tagged with "FIXME" and added some new cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158755/new/

https://reviews.llvm.org/D158755

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Han Shen via Phabricator via cfe-commits
shenhan updated this revision to Diff 553256.
shenhan marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158755/new/

https://reviews.llvm.org/D158755

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions.c


Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -15,6 +15,4 @@
 // RUN: not %clang -### -c --target=arm-unknown-linux 
-fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR
 // ERR: error: unsupported option '-fsplit-machine-functions' for target
 
-/// FIXME
-// RUN: not %clang -### -c --target=arm-unknown-linux 
-fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s 
--check-prefix=ERR2
-// ERR2: error: unsupported option '-fno-split-machine-functions' for target
+// RUN: %clang -### --target=arm-unknown-linux -fsplit-machine-functions 
-fno-split-machine-functions %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 


Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -15,6 +15,4 @@
 // RUN: not %clang -### -c --target=arm-unknown-linux -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR
 // ERR: error: unsupported option '-fsplit-machine-functions' for target
 
-/// FIXME
-// RUN: not %clang -### -c --target=arm-unknown-linux -fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR2
-// ERR2: error: unsupported option '-fno-split-machine-functions' for target
+// RUN: %clang -### --target=arm-unknown-linux -fsplit-machine-functions -fno-split-machine-functions %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158755: Make "-fno-split-machine-functions" a valid flag for all archs.

2023-08-24 Thread Han Shen via Phabricator via cfe-commits
shenhan added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions.c:19
+// RUN: not %clang -### -x cuda -S -nocudainc -nocudalib 
-fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR2
+// ERR2: clang: error: unsupported option '-fsplit-machine-functions' for 
target
+

MaskRay wrote:
> We should not test `clang: ` before `error:`. It may be a different string in 
> certain builds where `%clang` is a symlink to a file not named `clang`. This 
> would also fail on Windows. Rule of thumb: don't test the exact string before 
> `error:`
Acked.



Comment at: clang/test/Driver/fsplit-machine-functions.c:21
+
+// The following all run successfully.
+// RUN: %clang -### -x cuda -S -nocudainc -nocudalib -fsplit-machine-functions 
-Xarch_device -fno-split-machine-functions %s

MaskRay wrote:
> We don't really need new tests. The existing `--target=arm-unknown-linux` one 
> I added is sufficient.
Done, removed newly added cases.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158755/new/

https://reviews.llvm.org/D158755

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158755: [Driver] Make "-fno-split-machine-functions" a valid flag for all archs

2023-08-24 Thread Han Shen via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b27167a57c5: [Driver] Make 
"-fno-split-machine-functions" a valid flag for all archs (authored 
by shenhan).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158755/new/

https://reviews.llvm.org/D158755

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions.c


Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -15,6 +15,4 @@
 // RUN: not %clang -### -c --target=arm-unknown-linux 
-fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR
 // ERR: error: unsupported option '-fsplit-machine-functions' for target
 
-/// FIXME
-// RUN: not %clang -### -c --target=arm-unknown-linux 
-fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s 
--check-prefix=ERR2
-// ERR2: error: unsupported option '-fno-split-machine-functions' for target
+// RUN: %clang -### --target=arm-unknown-linux -fsplit-machine-functions 
-fno-split-machine-functions %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 


Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -15,6 +15,4 @@
 // RUN: not %clang -### -c --target=arm-unknown-linux -fsplit-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR
 // ERR: error: unsupported option '-fsplit-machine-functions' for target
 
-/// FIXME
-// RUN: not %clang -### -c --target=arm-unknown-linux -fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s --check-prefix=ERR2
-// ERR2: error: unsupported option '-fno-split-machine-functions' for target
+// RUN: %clang -### --target=arm-unknown-linux -fsplit-machine-functions -fno-split-machine-functions %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5872,13 +5872,13 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
options::OPT_fno_split_machine_functions)) {
-// This codegen pass is only available on x86-elf targets.
-if (Triple.isX86() && Triple.isOSBinFormatELF()) {
-  if (A->getOption().matches(options::OPT_fsplit_machine_functions))
+if (!A->getOption().matches(options::OPT_fno_split_machine_functions)) {
+  // This codegen pass is only available on x86-elf targets.
+  if (Triple.isX86() && Triple.isOSBinFormatELF())
 A->render(Args, CmdArgs);
-} else {
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << A->getAsString(Args) << TripleStr;
+  else
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35849: [UBSan] Provide default blacklist filename for UBSan

2017-07-25 Thread Han Shen via Phabricator via cfe-commits
shenhan created this revision.

This is to provide a default blacklist filename for UBSan.

While UBSan is turned on, it's better that clang pick up a blacklist file (when 
exists), just as what ASan / MSan does, so we do not end up adding the 
"-fsanitize-blacklist" option to every command line.


https://reviews.llvm.org/D35849

Files:
  lib/Driver/SanitizerArgs.cpp


Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -100,6 +100,8 @@
 BlacklistFile = "dfsan_abilist.txt";
   else if (Kinds & CFI)
 BlacklistFile = "cfi_blacklist.txt";
+  else if (Kinds & Undefined)
+BlacklistFile = "ubsan_blacklist.txt";
 
   if (BlacklistFile) {
 clang::SmallString<64> Path(D.ResourceDir);


Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -100,6 +100,8 @@
 BlacklistFile = "dfsan_abilist.txt";
   else if (Kinds & CFI)
 BlacklistFile = "cfi_blacklist.txt";
+  else if (Kinds & Undefined)
+BlacklistFile = "ubsan_blacklist.txt";
 
   if (BlacklistFile) {
 clang::SmallString<64> Path(D.ResourceDir);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-07-25 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment.

Thanks. Can you update "SanitizerArgs::collectDefaultBlacklists" to include 
"ubsan_blacklist.txt"?


https://reviews.llvm.org/D32842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35849: [UBSan] Provide default blacklist filename for UBSan

2017-07-25 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment.

Thanks for pointing out the relevant CLs. I agree that's the clearer and better 
solution.

(The only concern is that it has been sitting there for 2 months. I'll check 
back.)

In https://reviews.llvm.org/D35849#820473, @vsk wrote:

> This won't do the right thing if more than one sanitizer with a default 
> blacklist is enabled. It's also problematic that a default blacklist for one 
> sanitizer can blacklist code for a different sanitizer. See:
>
> https://reviews.llvm.org/D32043
>  https://reviews.llvm.org/D32047
>  https://reviews.llvm.org/D32842
>
> IMO I think https://reviews.llvm.org/D32842 is the right path forward. I 
> don't have the time to get back to it this week, but if you'd like to pick it 
> up and respond to review feedback from @eugenis, please do (I can certainly 
> help with the review).





https://reviews.llvm.org/D35849



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits