[PATCH] D92363: [HIP] Warn no --offload-arch option

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 3 12:17:48 PST 2020


tra added a comment.

In D92363#2431812 <https://reviews.llvm.org/D92363#2431812>, @yaxunl wrote:

> 



> There are still valid use cases for not providing GPU arch. For example, users would like to test syntax of a HIP program for which GPU arch does not matter, or users would like to get PCH file where GPU arch does not matter. Another example is cmake may test whether a compiler can compile HIP program without options. Make it a warning allows such uses cases.

Issuing no warning allows such use cases even better. :-)

Also, how do I silence that warning, if I'm compiling with -Werror? 
If we provide a knob to silence the warning, how would that be better than asking the user to specify a GPU arch? 
If we do ask users to specify GPU arch in order to silence the warning => we effectively make the GPU arch a mandatory option.

I believe that no warning or a mandatory GPU arch would be a more principled way. With the warning we effectively flag the default value as 'it's not a very useful default'. If it's not useful by default, then it should not be the default. If it is useful, there should be no warning.

In this case, I think making GPU arch mandatory would have a stronger case, as the default we use right now is indeed quite often wrong. However, we probably have existing users who have valid use cases that work without the explicitly specified GPU arch, so requiring the GPU arch would break them. That constrains us to the no-warning scenario in practice.

Introducing a warning would break existing users, too -- compiling with `-Werror` is not that uncommon. If we're willing to break users, we may as well break them by requiring a GPU arch.
I just don't see a compelling argument where adding a warning would provide a net benefit over no-warning or a mandatory-GPU-arch scenarios.


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

https://reviews.llvm.org/D92363



More information about the cfe-commits mailing list