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

[Artem Belevich via Phabricator via cfe-commits]

tra 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:
> shenhan wrote:
> > 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 

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay 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

shenhan wrote:
> 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. 

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

[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 
> > > 

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay 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:
> 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-functions` 
> > during CUDA compilation)
> > 
> > 
> > 
> > 
> There are excessive spaces before `%clang`. We should keep just one space: 
> `RUN: %clang`
I agree with @tra's analysis. Either do nothing on Clang side and requiring 

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Sorry, but I think this change should be reverted.

(a) `-fsplit-machine-functions` on an unsupported target now emits a warning 
instead of an error. This diverges from the regular expectation for 
target-specific features.

  % fclang --target=riscv64 -fsplit-machine-functions -c a.c
  warning: -fsplit-machine-functions is not valid for riscv64 [-Wbackend-plugin]

`warn_drv_for_elf_only` is not necessary. We typically just use 
`err_drv_unsupported_opt_for_target`.

(b) the test  needs substantial change (D158231 
)

(c) The error reporting should be on the driver side, not in backend. `void 
DiagnosticInfoMachineFunctionSplit::print(DiagnosticPrinter ) const` is not 
necessary. 
At the very least, it should not be in the generic `IR/DiagnosticInfo.cpp`


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

[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] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:16
+// causes a warning.
+// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
+// RUN: --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s 2>&1 
\

tra wrote:
> Hahnfeld wrote:
> > steelannelida wrote:
> > > Unfortunately these commands fail in our sandbox due to writing files to 
> > > readonly directories:
> > > 
> > >  `unable to open output file 
> > > 'fsplit-machine-functions-with-cuda-nvptx.s': 'Permission denied'`
> > > 
> > > Could you please specify the output files via `%t` substitutions? I'm not 
> > > sure how to do this for cuda compilation.
> > IIRC the file names are generated based on what you specify with `-o`. Did 
> > you try this already?
> The problem is that in this case we didn't pass any -o at all here, so the 
> compiler tries to write into the current directory.
> 
> We need `-o %t.s` or `-o /dev/null` here.
Driver tests should not run the backend. Most driver commands should use `-###`.

`REQUIRES: shell` disables the internal shell, which essentially disables the 
test on Windows. This should generally be avoided.

An idiom is `// RUN: rm -rf %t && mkdir %t && cd %t` (see `ftime-trace.cpp`) if 
the driver places output files in the current working directory.



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:
> > > 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 

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

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:16
+// causes a warning.
+// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
+// RUN: --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s 2>&1 
\

Hahnfeld wrote:
> steelannelida wrote:
> > Unfortunately these commands fail in our sandbox due to writing files to 
> > readonly directories:
> > 
> >  `unable to open output file 'fsplit-machine-functions-with-cuda-nvptx.s': 
> > 'Permission denied'`
> > 
> > Could you please specify the output files via `%t` substitutions? I'm not 
> > sure how to do this for cuda compilation.
> IIRC the file names are generated based on what you specify with `-o`. Did 
> you try this already?
The problem is that in this case we didn't pass any -o at all here, so the 
compiler tries to write into the current directory.

We need `-o %t.s` or `-o /dev/null` here.


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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:16
+// causes a warning.
+// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
+// RUN: --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s 2>&1 
\

steelannelida wrote:
> Unfortunately these commands fail in our sandbox due to writing files to 
> readonly directories:
> 
>  `unable to open output file 'fsplit-machine-functions-with-cuda-nvptx.s': 
> 'Permission denied'`
> 
> Could you please specify the output files via `%t` substitutions? I'm not 
> sure how to do this for cuda compilation.
IIRC the file names are generated based on what you specify with `-o`. Did you 
try this already?


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

[Dmitry Chernenkov via Phabricator via cfe-commits]

steelannelida added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:16
+// causes a warning.
+// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
+// RUN: --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s 2>&1 
\

Unfortunately these commands fail in our sandbox due to writing files to 
readonly directories:

 `unable to open output file 'fsplit-machine-functions-with-cuda-nvptx.s': 
'Permission denied'`

Could you please specify the output files via `%t` substitutions? I'm not sure 
how to do this for cuda compilation.


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

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

I dsabled two tests without `{arm,aarch64}-registered-target` in 
rGeeac4321c517ee8afc30ebe62c5b1778efc1173d 
; two 
post-commit comments inline




Comment at: llvm/test/CodeGen/Generic/machine-function-splitter.ll:18
+; 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.

shouldn't this be `MFS_ON-NOT`?



Comment at: llvm/test/CodeGen/Generic/machine-function-splitter.ll:21
+; MFS_OFF: warning: -fsplit-machine-functions is not valid for
+; MFS_OFF_NO: Machine Function Splitter Transformation
 




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

[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 ) 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 ) const override;
 
   bool runOnMachineFunction(MachineFunction ) override;
+
+  bool doInitialization(Module &) override;
+
+  static bool isSupportedTriple(const Triple ) { return T.isX86(); }
+
+private:
+  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -127,7 +136,20 @@
   return (*Count < ColdCountThreshold);
 }
 
+bool MachineFunctionSplitter::doInitialization(Module ) {
+  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 ) {
+  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:
+  

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

[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

[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 ) 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 ) const override;
 
   bool runOnMachineFunction(MachineFunction ) override;
+
+  bool doInitialization(Module &) override;
+
+  static bool isSupportedTriple(const Triple ) { return T.isX86(); }
+
+private:
+  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -127,7 +136,20 @@
   return (*Count < ColdCountThreshold);
 }
 
+bool MachineFunctionSplitter::doInitialization(Module ) {
+  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 ) {
+  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)
+  : 

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

[Daniel Hoekwater via Phabricator via cfe-commits]

dhoekwater added a comment.

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.


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

[Rong Xu via Phabricator via cfe-commits]

xur accepted this revision.
xur added a comment.

lgtm


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

[Daniel Hoekwater via Phabricator via cfe-commits]

dhoekwater accepted this revision.
dhoekwater added a comment.
This revision is now accepted and ready to land.

lgtm.


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

[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 ) 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 ) const override;
 
   bool runOnMachineFunction(MachineFunction ) override;
+
+  bool doInitialization(Module &) override;
+
+  static bool isSupportedTriple(const Triple ) { return T.isX86(); }
+
+private:
+  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -127,7 +136,20 @@
   return (*Count < ColdCountThreshold);
 }
 
+bool MachineFunctionSplitter::doInitialization(Module ) {
+  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 ) {
+  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 

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

[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

[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 ) 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 ) const override;
 
   bool runOnMachineFunction(MachineFunction ) override;
+
+  bool doInitialization(Module &) override;
+
+  static bool isSupportedTriple(const Triple ) { return T.isX86(); }
+
+private:
+  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -127,7 +136,20 @@
   return (*Count < ColdCountThreshold);
 }
 
+bool MachineFunctionSplitter::doInitialization(Module ) {
+  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 ) {
+  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)
+  : 

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

[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

[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 ) 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 ) const override;
 
   bool runOnMachineFunction(MachineFunction ) override;
+
+  bool doInitialization(Module &) override;
+
+  static bool isSupportedTriple(const Triple ) { return T.isX86(); }
+
+private:
+  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -127,7 +136,20 @@
   return (*Count < ColdCountThreshold);
 }
 
+bool MachineFunctionSplitter::doInitialization(Module ) {
+  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 ) {
+  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 ) 

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

[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

[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 ) 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 ) const override;
 
   bool runOnMachineFunction(MachineFunction ) override;
+
+  bool doInitialization(Module &) override;
+
+private:
+  bool UnsupportedTriple = false;
 };
 } // end anonymous namespace
 
@@ -127,7 +134,20 @@
   return (*Count < ColdCountThreshold);
 }
 
+bool MachineFunctionSplitter::doInitialization(Module ) {
+  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 ) {
+  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 ) 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 codegen pass is 

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

[Artem Belevich via Phabricator via cfe-commits]

tra 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

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-functions` during CUDA 
compilation)






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

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



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

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)


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

[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

[Artem Belevich via Phabricator via cfe-commits]

tra 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

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.




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

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



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

You cannot spam warnings here. The other instance of printing here looks like a 
new addition and should be removed


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

[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

[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

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.






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

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`.




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

[Snehasish Kumar via Phabricator via cfe-commits]

snehasish added a subscriber: dhoekwater.
snehasish added inline comments.



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

Can you coordinate with @dhoekwater ? He has some patches in flight for 
AArch64. 

I think D157157 is the one which modifies the same logic.



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

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.


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

[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