Hi,
On Fri, Feb 26, 2016 at 06:51:34PM +0100, Jakub Jelinek wrote:
> On Fri, Feb 26, 2016 at 06:18:13PM +0100, Martin Jambor wrote:
> > > I'm a proponent of enabling as many useful warnings by default, or if not
> > > by default, then with -Wall. -Whsa is enabled by default, and has thus
> > > set a precedent of doing that.
> >
> > I am not sure I'd go as far as "as many as possible," but in the case
> > of -Whsa, the warnings get emitted only if HSA offloading is
> > configured and especially only if the user used OMP and its target
> > construct. This means that it is relevant only for a rather small
> > class of users and it's not a "your code looks weird" kind of warning
> > but a "the compiler is not doing what you clearly asked for" warning.
> > So that is why we decided to warn unconditionally.
> >
> > But as far as I understand, gcc does not give any promises about
> > warnings, so I believe decisions like a defaultness of a warning can
> > be revisited at any point in the future, for example if people learn
> > not to expect some constructs to be offloaded to GPUs. Moreover, the
> > conventions regarding offloading are still being settled and still
> > will for quite some time so nobody should really expect such details
> > to be set in stone.
>
> The thing is, most of the tests in the libgomp.{c,c++,fortran}/ testsuite
> are (meant to be) valid OpenMP testcases, having them full of dozens of
> dg-warning lines where every of the 10+ different offloading target warns
> about something would be a maintainance nightmare.
Agreed, having such dg-warnings would definitely be an overkill. I
only intended to mark the whole test with an option. I am willing to
be looking for new hsa warnings and examine them myself, adding the
option if necessary. I would not expect the originator of the
testcase or anybody who does not care for HSA to do it.
> E.g. when adding
> new OpenMP tests, one would need to configure all the offloaders
> (individually?), for some you need hw not every committer has,
No special hardware is necessary to see the warning (though you need
at least https://github.com/HSAFoundation/HSA-Runtime-Reference-Source
to build the libgomp plugin). Once gcc decides to emit HSAIL, it of
course has to work as expected. There should be no need to configure
hsa individually either.
> for others
> there are other issues (e.g., is the required amdkfd going to be submitted
> for upstream kernel? I might have hard time convincing our kernel
> maintainers to use that instead of what is in upstream kernel others).
I am being repeatedly told it will be and very soon, but apparently it
takes longer than AMD anticipated. I don't think anybody expects any
distribution to pick it up on their own (...but you know, Red Hat is
actually the company that now employs the upstream kernel kfd
maintainer ;-).
> So, IMHO if you want to check for warnings, do that as Martin has added a
> new subdir with only hsa OpenMP tests, if you want test warnings on tests we
> already have elsewhere, #include them in the other dir, dg-do link instead
> of run (so that it is not run multiple times), and check for the warnings;
> you could also use -foffload=hsa in there to make sure you only have to care
> about hsa warnings, and not NVPTX, or whatever other offloader.
>
Just to be clear, I never wanted to be testing for presence of
warnings, I see no value in that.
All in all, I am willing to add -Wno-hsa to default options and only
have these warnings on in dedicated HSA directories. I will amend the
posted patches once testsuite maintainers look at my initial proposal
for the first such directory.
Martin
