Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

Hi,

I added some content to gccbrig.texi in r254820 as below. If you have something
that I could describe further there, please just let me know.

Index: gcc/brig/gccbrig.texi
===
--- gcc/brig/gccbrig.texi (revision 254819)
+++ gcc/brig/gccbrig.texi (revision 254820)
@@ -1,5 +1,153 @@
 \input texinfo @c -*-texinfo-*-
 @setfilename gccbrig.info
-@settitle The GNU BRIG Compiler
+@settitle The GNU BRIG (HSAIL) Compiler
+@set copyrights-brig 2017

+@c Merge the standard indexes into a single one.
+@syncodeindex fn cp
+@syncodeindex vr cp
+@syncodeindex ky cp
+@syncodeindex pg cp
+@syncodeindex tp cp
+
+@include gcc-common.texi
+
+@copying
+@c man begin COPYRIGHT
+Copyright @copyright{} @value{copyrights-brig} Free Software Foundation, Inc.
+
+Permission is granted to copy, distribute and/or modify this document
+under the terms of the GNU Free Documentation License, Version 1.3 or
+any later version published by the Free Software Foundation; with no
+Invariant Sections, the Front-Cover Texts being (a) (see below), and
+with the Back-Cover Texts being (b) (see below).
+A copy of the license is included in the
+@c man end
+section entitled ``GNU Free Documentation License''.
+@ignore
+@c man begin COPYRIGHT
+man page gfdl(7).
+@c man end
+@end ignore
+
+@c man begin COPYRIGHT
+
+(a) The FSF's Front-Cover Text is:
+
+ A GNU Manual
+
+(b) The FSF's Back-Cover Text is:
+
+ You have freedom to copy and modify this GNU Manual, like GNU
+ software.  Copies published by the Free Software Foundation raise
+ funds for GNU development.
+@c man end
+@end copying
+
+@ifinfo
+@format
+@dircategory Software development
+@direntry
+* Gccbrig: (gccbrig).   A GCC-based compiler for BRIG/HSAIL
finalization
+@end direntry
+@end format
+
+@insertcopying
+@end ifinfo
+
+@titlepage
+@title The GNU BRIG (HSAIL) Compiler
+@versionsubtitle
+@author Pekka Jääskeläinen
+
+@page
+@vskip 0pt plus 1filll
+Published by the Free Software Foundation @*
+51 Franklin Street, Fifth Floor@*
+Boston, MA 02110-1301, USA@*
+@sp 1
+@insertcopying
+@end titlepage
+@contents
+@page
+
+@node Top
+@top Introduction
+
+This manual describes how to use @command{gccbrig}, the GNU compiler for
+the binary representation (BRIG) of the HSA Intermediate Language (HSAIL).
+For more information about the Heterogeneous System Architecture (HSA)
+Foundation's standards in general, see @uref{http://www.hsafoundation.com/}.
+
+@menu
+* Copying:: The GNU General Public License.
+* GNU Free Documentation License::
+How you can share and copy this manual.
+* Using Gccbrig::   How to use Gccbrig.
+* Index::   Index.
+@end menu
+
+@include gpl_v3.texi
+
+@include fdl.texi
+
+
+@node Using Gccbrig
+@chapter Using Gccbrig
+
+@c man title gccbrig A GCC-based compiler for HSAIL
+
+@ignore
+@c man begin SYNOPSIS gccbrig
+gccbrig [@option{-c}|@option{-S}]
+[@option{-O}@var{level}] [@option{-L}@var{dir}@dots{}]
+[@option{-o} @var{outfile}] @var{infile}@dots{}
+
+Gccbrig is typically not invoked from the command line, but
+through an HSA finalizer implementation.
+@c man end
+@c man begin SEEALSO
+The Info entry for @file{gccbrig} and
+@uref{https://github.com/HSAFoundation/phsa}
+@c man end
+@end ignore
+
+@c man begin DESCRIPTION gccbrig
+
+The BRIG frontend (@command{gccbrig}) differs from the
+other frontends in GCC on how it's typically used.  It's a translator
+for an intermediate language that is not meant to be written directly
+by programmers.  Its input format BRIG is a binary representation of
+HSAIL, which is a textual assembly format for an imaginary machine
+of which instruction set is defined in HSA Programmer Reference Manual
+(PRM) Specification.  Gccbrig currently implements the Base profile
+of the PRM version 1.0.
+
+HSA Runtime Specification defines an API which includes means
+to build and launch ``kernels'' from a host program running on a CPU
+to one or more heterogeneous ``kernel agents''. A kernel Agent
+is typically a GPU or a DSP device controlled by the CPU.
+The build phase is called ``finalization'', which means translation of
+one or more target-independent BRIG files describing the program that
+one wants to run in the Agent to the Agent's instruction set.  Gccbrig
+implements the translation process by generating GENERIC, which is
+translated to the ISA of any supported GCC target by the GCC's backend
+framework, thus enabling potentially any GCC target to act as an HSA agent.
+
+As the kernel finalization process can be only launched from the host API,
+@command{gccbrig} is not typically used directly from the command line by
+the end user, but through an HSA runtime implementation that implements
+the finalizer API running on the host CPU.  Gccbrig is
+designed to work with an open source HSA runtime implementation
+called ``phsa-runtime'', which can be 

Re: [PATCH] BRIG frontend: request for a global review

[Martin Jambor]

Hi,

On Sun, Sep 17, 2017 at 02:13:34PM +0200, Thomas Schwinge wrote:
> Hi!
> 
> On Tue, 24 Jan 2017 15:30:34 -0500, David Malcolm  wrote:
> > On Tue, 2017-01-24 at 13:52 +0100, Martin Jambor wrote:
> > > [...] I have just
> > > committed the BRIG FE as revision 244867.
> 
> In a build with that enabled, I just happened to "make html" in "gcc/",
> and ran into:
> 
> [...]
> makeinfo --split-size=500 --html -I [...]/source-gcc/gcc/doc -I 
> [...]/source-gcc/gcc/doc/include \
> -I [...]/source-gcc/gcc/brig -o 
> [...]/build-gcc/gcc/HTML/gcc-8.0.0/brig
> makeinfo: missing file argument.
> Try `makeinfo --help' for more information.
> [...]/source-gcc/gcc/brig/Make-lang.in:117: recipe for target 
> '[...]/build-gcc/gcc/HTML/gcc-8.0.0/brig/index.html' failed
> make: *** [[...]/build-gcc/gcc/HTML/gcc-8.0.0/brig/index.html] Error 255
> 
> > A deps issue for the docs I noticed when glancing through the commit:
> > 
> > diff --git a/gcc/brig/Make-lang.in b/gcc/brig/Make-lang.in
> > new file mode 100644 (file)
> > index 000..b85b1b0
> > --- /dev/null
> > +++ b/gcc/brig/Make-lang.in
> > 
> > [...snip...]
> > 
> > +# Documentation.
> > +
> > +GO_TEXI_FILES = \
> > +   brig/gccbrig.texi \
> > +   $(gcc_docdir)/include/fdl.texi \
> > +   $(gcc_docdir)/include/gpl_v3.texi \
> > +   $(gcc_docdir)/include/gcc-common.texi \
> > +   gcc-vers.texi
> > 
> > Presumably this should be BRIG_TEXI_FILES, rather than GO_TEXI_FILES?
> > 
> > +# doc/gccbrig.info: $(BRIG_TEXI_FILES)
> > +#  if test "x$(BUILD_INFO)" = xinfo; then \
> > +#rm -f doc/gccbrig.info*; \
> > +#$(MAKEINFO) $(MAKEINFOFLAGS) -I $(gcc_docdir) \
> > +#  -I $(gcc_docdir)/include -o $@ $<; \
> > +#  else true; fi
> > +
> > +# doc/gccbrig.dvi: $(BRIG_TEXI_FILES)
> > +#  $(TEXI2DVI) -I $(abs_docdir) -I $(abs_docdir)/include -o $@ $<
> > +
> > +# doc/gccbrig.pdf: $(BRIG_TEXI_FILES)
> > +#  $(TEXI2PDF) -I $(abs_docdir) -I $(abs_docdir)/include -o $@ $<
> > +
> > +$(build_htmldir)/brig/index.html: $(BRIG_TEXI_FILES)
> > +   $(mkinstalldirs) $(@D)
> > +   rm -f $(@D)/*
> > +   $(TEXI2HTML) -I $(gcc_docdir) -I $(gcc_docdir)/include \
> > +   -I $(srcdir)/brig -o $(@D) $<
> > 
> > ...for use in describing the deps of the above.
> 
> ..., so that still needs to be fixed.  Alas, that won't help: the
> "gccbrig.texi" file doesn't actually exist.  ;-)
> 

I see, I always only check "make info" when verifying documentation
changes and so missed this.  Thanks for providing the interim fix, me
and/or Pekka will add some basic content by the time next gcc 7 is
released (IIRC, it is supposed to come out at the end of this or the
beginning of next year).

Martin

Re: [PATCH] BRIG frontend: request for a global review

[Thomas Schwinge]

Hi!

On Tue, 24 Jan 2017 15:30:34 -0500, David Malcolm  wrote:
> On Tue, 2017-01-24 at 13:52 +0100, Martin Jambor wrote:
> > [...] I have just
> > committed the BRIG FE as revision 244867.

In a build with that enabled, I just happened to "make html" in "gcc/",
and ran into:

[...]
makeinfo --split-size=500 --html -I [...]/source-gcc/gcc/doc -I 
[...]/source-gcc/gcc/doc/include \
-I [...]/source-gcc/gcc/brig -o 
[...]/build-gcc/gcc/HTML/gcc-8.0.0/brig
makeinfo: missing file argument.
Try `makeinfo --help' for more information.
[...]/source-gcc/gcc/brig/Make-lang.in:117: recipe for target 
'[...]/build-gcc/gcc/HTML/gcc-8.0.0/brig/index.html' failed
make: *** [[...]/build-gcc/gcc/HTML/gcc-8.0.0/brig/index.html] Error 255

> A deps issue for the docs I noticed when glancing through the commit:
> 
> diff --git a/gcc/brig/Make-lang.in b/gcc/brig/Make-lang.in
> new file mode 100644 (file)
> index 000..b85b1b0
> --- /dev/null
> +++ b/gcc/brig/Make-lang.in
> 
> [...snip...]
> 
> +# Documentation.
> +
> +GO_TEXI_FILES = \
> +   brig/gccbrig.texi \
> +   $(gcc_docdir)/include/fdl.texi \
> +   $(gcc_docdir)/include/gpl_v3.texi \
> +   $(gcc_docdir)/include/gcc-common.texi \
> +   gcc-vers.texi
> 
> Presumably this should be BRIG_TEXI_FILES, rather than GO_TEXI_FILES?
> 
> +# doc/gccbrig.info: $(BRIG_TEXI_FILES)
> +#  if test "x$(BUILD_INFO)" = xinfo; then \
> +#rm -f doc/gccbrig.info*; \
> +#$(MAKEINFO) $(MAKEINFOFLAGS) -I $(gcc_docdir) \
> +#  -I $(gcc_docdir)/include -o $@ $<; \
> +#  else true; fi
> +
> +# doc/gccbrig.dvi: $(BRIG_TEXI_FILES)
> +#  $(TEXI2DVI) -I $(abs_docdir) -I $(abs_docdir)/include -o $@ $<
> +
> +# doc/gccbrig.pdf: $(BRIG_TEXI_FILES)
> +#  $(TEXI2PDF) -I $(abs_docdir) -I $(abs_docdir)/include -o $@ $<
> +
> +$(build_htmldir)/brig/index.html: $(BRIG_TEXI_FILES)
> +   $(mkinstalldirs) $(@D)
> +   rm -f $(@D)/*
> +   $(TEXI2HTML) -I $(gcc_docdir) -I $(gcc_docdir)/include \
> +   -I $(srcdir)/brig -o $(@D) $<
> 
> ...for use in describing the deps of the above.

..., so that still needs to be fixed.  Alas, that won't help: the
"gccbrig.texi" file doesn't actually exist.  ;-)

As the intention surely must be to provide some kind of documentation
eventually, and to let me move forward, I "fixed" that (dummy file), and
also enabled some other documentation bits.  As obvious, committed to
trunk in r252893:

commit 61cfebc8aabe6bd1c2b045982943f5758ed81b8b
Author: tschwinge 
Date:   Sun Sep 17 12:11:40 2017 +

Fix gccbrig documentation build

gcc/brig/
* Make-lang.in (GO_TEXI_FILES): Rename to...
(BRIG_TEXI_FILES): ... this.
(doc/gccbrig.info, doc/gccbrig.dvi, doc/gccbrig.pdf, brig.info)
(brig.srcinfo, brig.man, brig.srcman, brig.install-man)
($(DESTDIR)$(man1dir)/$(GCCBRIG_INSTALL_NAME)$(man1ext)):
Uncomment/enable targets.
(gccbrig.pod): New target.
* gccbrig.texi: New file.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@252893 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/brig/ChangeLog| 11 +++
 gcc/brig/Make-lang.in | 52 +++
 gcc/brig/gccbrig.texi |  5 +
 3 files changed, 44 insertions(+), 24 deletions(-)

diff --git gcc/brig/ChangeLog gcc/brig/ChangeLog
index 0804532..ebf31c4 100644
--- gcc/brig/ChangeLog
+++ gcc/brig/ChangeLog
@@ -1,3 +1,14 @@
+2017-09-17  Thomas Schwinge  
+
+   * Make-lang.in (GO_TEXI_FILES): Rename to...
+   (BRIG_TEXI_FILES): ... this.
+   (doc/gccbrig.info, doc/gccbrig.dvi, doc/gccbrig.pdf, brig.info)
+   (brig.srcinfo, brig.man, brig.srcman, brig.install-man)
+   ($(DESTDIR)$(man1dir)/$(GCCBRIG_INSTALL_NAME)$(man1ext)):
+   Uncomment/enable targets.
+   (gccbrig.pod): New target.
+   * gccbrig.texi: New file.
+
 2017-08-04  Henry Linjamäki  
 
Fix PR 81713
diff --git gcc/brig/Make-lang.in gcc/brig/Make-lang.in
index d7566bf..23ddc03 100644
--- gcc/brig/Make-lang.in
+++ gcc/brig/Make-lang.in
@@ -93,25 +93,25 @@ brig1$(exeext): $(BRIG_OBJS) attribs.o $(BACKEND) $(LIBDEPS)
 
 # Documentation.
 
-GO_TEXI_FILES = \
+BRIG_TEXI_FILES = \
brig/gccbrig.texi \
$(gcc_docdir)/include/fdl.texi \
$(gcc_docdir)/include/gpl_v3.texi \
$(gcc_docdir)/include/gcc-common.texi \
gcc-vers.texi
 
-# doc/gccbrig.info: $(BRIG_TEXI_FILES)
-#  if test "x$(BUILD_INFO)" = xinfo; then \
-#rm -f doc/gccbrig.info*; \
-#$(MAKEINFO) $(MAKEINFOFLAGS) -I $(gcc_docdir) \
-#  -I $(gcc_docdir)/include -o $@ $<; \
-#  else true; fi
+doc/gccbrig.info: $(BRIG_TEXI_FILES)
+   if test "x$(BUILD_INFO)" = xinfo; then \
+ rm -f doc/gccbrig.info*; \
+ 

Re: [PATCH] BRIG frontend: request for a global review

[Martin Jambor]

Hi,

I have just committed the patch, as it is, except that a couple of
two-spaces-after Pekka's name in Changelogs had already been corrected
(sorry for that mistake) and I have also

On Fri, Jan 27, 2017 at 10:31:34AM +0200, Pekka Jääskeläinen wrote:
> --- a/gcc/brig/ChangeLog
> +++ b/gcc/brig/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-01-26  Pekka Jääskeläinen  
> +
> + Removed stale target-libbrig reference from config-lang.in
> +
>  2017-01-26  Jakub Jelinek  
>  
>   Update copyright years.

Converted this into a standard ChangeLog entry.

Thanks,

Martin

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

Hi Jakub and Matthias,

New overall patch attached. My commit access is pending so I'm relying on Martin
or someone else to get this committed for now.

Added replies inline:

On Thu, Jan 26, 2017 at 2:04 PM, Jakub Jelinek  wrote:
> Hi!
>
> On Thu, Jan 26, 2017 at 01:30:21PM +0200, Pekka Jääskeläinen wrote:
>> diff --git a/ChangeLog b/ChangeLog
>> index 9695f9d..6f4f256 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2017-01-26  Pekka Jskel??inen  
>> +
>> + * configure.ac: Added i[3456789]86-*-linux* as a supported env
>> + for the BRIG FE.
>> + * configure: Regenerated.
>> +
>>  2017-01-24  Pekka Jskel??inen 
>>   Martin Jambor  
>>
>
> It might be better if this isn't present directly in toplevel configure.
> Can you add libhsail-rt/configure.tgt and handle it in toplevel configure.ac
> similarly to libatomic, libcilkrts, liboffloadmic, libitm, libsanitizer,
> libvtv and libmpx, i.e. like:
> # Disable libhsail-rt on unsupported systems.
> if test -d ${srcdir}/libhsail-rt; then
> if test x$enable_libhsail-rt = x; then
> AC_MSG_CHECKING([for libhsail-rt support])
> if (srcdir=${srcdir}/libhsail-rt; \
> . ${srcdir}/configure.tgt; \
> test -n "$UNSUPPORTED")
> then
> AC_MSG_RESULT([no])
> unsupported_languages="$unsupported_languages brig"
> # This implicitly disables also target-libhsail-rt as it won't
> # get added to the build without BRIG FE.
> else
> AC_MSG_RESULT([yes])
> fi
> fi
> fi
> or so?

Done.

>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index f485bb3..d481b25 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2017-01-26  Pekka Jskel??inen 
>
> 1) there should be 2 spaces rather than just one on each side of the name,
> so date two spaces name two spaces 

Fixed.

> 2) when committing, please make sure the unicode characters in your
> name are actually valid UTF-8, not sure if it is my MUA or your or something
> in between, but I'm just seeing  and ??

I believe it's your MUA or something in between. The Scandinavian
letters in my patch look
correct in an editor with UTF-8 encoding on (my emacs seems to also
have utf-8 on by default).
However, I'm not obsessed with those obscure letters so I can write my
surname Jaaskelainen
if preferred (like it's written in the copyright lines).

>> --- a/gcc/brig/ChangeLog
>> +++ b/gcc/brig/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2017-01-26  Pekka Jskel??inen 
>> +
>> + Removed stale target-libbrig reference from config-lang.in
>> +
>>  2017-01-26  Jakub Jelinek  
>>
>>   Update copyright years.
>> diff --git a/gcc/brig/config-lang.in b/gcc/brig/config-lang.in
>> index ab139b3..e390a16 100644
>> --- a/gcc/brig/config-lang.in
>> +++ b/gcc/brig/config-lang.in
>> @@ -28,7 +28,7 @@ language="brig"
>>
>>  compilers="brig1\$(exeext)"
>>
>> -target_libs="target-libbrig target-libhsail-rt"
>> +target_libs="target-libhsail-rt"
>
> If you are BRIG maintainer (but not listed yet in MAINTAINERS?), you can
> approve this yourself.

I added Martin and myself to MAINTAINERS for BRIG FE and libhsail-rt.

>> --- a/gcc/builtin-types.def
>> +++ b/gcc/builtin-types.def
>> @@ -70,7 +70,7 @@ DEF_PRIMITIVE_TYPE (BT_UINTMAX, uintmax_type_node)
>>  DEF_PRIMITIVE_TYPE (BT_INT8, signed_char_type_node)
>>  DEF_PRIMITIVE_TYPE (BT_INT16, short_integer_type_node)
>>  DEF_PRIMITIVE_TYPE (BT_UINT8, char_type_node)
>> -DEF_PRIMITIVE_TYPE (BT_UINT16, short_unsigned_type_node)
>> +DEF_PRIMITIVE_TYPE (BT_UINT16, uint16_type_node)
>>  DEF_PRIMITIVE_TYPE (BT_UINT32, uint32_type_node)
>>  DEF_PRIMITIVE_TYPE (BT_UINT64, uint64_type_node)
>>  DEF_PRIMITIVE_TYPE (BT_WORD, (*lang_hooks.types.type_for_mode) (word_mode, 
>> 1))
>
> This is ok for trunk.  Not sure about the INT8/INT16/UINT8 though,
> 1) char_type_node can be generally signed or unsigned, shouldn't that be
> unsigned_char_type_node instead?

True, fixed.

> 2) wonder what will happen on targets where char is 32-bit or similar.

I wondered it myself, and I'm pretty sure BRIG FE fails in that case.
I didn't want to add new fixed width type nodes for now as I thought
it's a bit too intrusive
at this point. If BRIG FE is ported to such a platform (BTW are there
such targets in gcc
now -- just curious?) this needs to be fixed and the target white
listed in the new configure.tgt.

>> --- a/libhsail-rt/ChangeLog
>> +++ b/libhsail-rt/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2017-01-26  Pekka Jskel??inen 
>> +
>> + * README: Added a proper description of what libhsail-rt is.
>> +
>>  2017-01-26  Jakub Jelinek  
>>
>>   Update copyright years.
>> diff --git a/libhsail-rt/README 

Re: [PATCH] BRIG frontend: request for a global review

[Jakub Jelinek]

Hi!

On Thu, Jan 26, 2017 at 01:30:21PM +0200, Pekka Jääskeläinen wrote:
> diff --git a/ChangeLog b/ChangeLog
> index 9695f9d..6f4f256 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-01-26  Pekka Jskel??inen  
> +
> + * configure.ac: Added i[3456789]86-*-linux* as a supported env
> + for the BRIG FE.
> + * configure: Regenerated.
> +
>  2017-01-24  Pekka Jskel??inen 
>   Martin Jambor  
>  

It might be better if this isn't present directly in toplevel configure.
Can you add libhsail-rt/configure.tgt and handle it in toplevel configure.ac
similarly to libatomic, libcilkrts, liboffloadmic, libitm, libsanitizer,
libvtv and libmpx, i.e. like:
# Disable libhsail-rt on unsupported systems.
if test -d ${srcdir}/libhsail-rt; then
if test x$enable_libhsail-rt = x; then
AC_MSG_CHECKING([for libhsail-rt support])
if (srcdir=${srcdir}/libhsail-rt; \
. ${srcdir}/configure.tgt; \
test -n "$UNSUPPORTED")
then
AC_MSG_RESULT([no])
unsupported_languages="$unsupported_languages brig"
# This implicitly disables also target-libhsail-rt as it won't
# get added to the build without BRIG FE.
else
AC_MSG_RESULT([yes])
fi
fi
fi
or so?

> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index f485bb3..d481b25 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-01-26  Pekka Jskel??inen 

1) there should be 2 spaces rather than just one on each side of the name,
so date two spaces name two spaces 
2) when committing, please make sure the unicode characters in your
name are actually valid UTF-8, not sure if it is my MUA or your or something
in between, but I'm just seeing  and ??

> --- a/gcc/brig/ChangeLog
> +++ b/gcc/brig/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-01-26  Pekka Jskel??inen 
> +
> + Removed stale target-libbrig reference from config-lang.in
> +
>  2017-01-26  Jakub Jelinek  
>  
>   Update copyright years.
> diff --git a/gcc/brig/config-lang.in b/gcc/brig/config-lang.in
> index ab139b3..e390a16 100644
> --- a/gcc/brig/config-lang.in
> +++ b/gcc/brig/config-lang.in
> @@ -28,7 +28,7 @@ language="brig"
>  
>  compilers="brig1\$(exeext)"
>  
> -target_libs="target-libbrig target-libhsail-rt"
> +target_libs="target-libhsail-rt"

If you are BRIG maintainer (but not listed yet in MAINTAINERS?), you can
approve this yourself.

> --- a/gcc/builtin-types.def
> +++ b/gcc/builtin-types.def
> @@ -70,7 +70,7 @@ DEF_PRIMITIVE_TYPE (BT_UINTMAX, uintmax_type_node)
>  DEF_PRIMITIVE_TYPE (BT_INT8, signed_char_type_node)
>  DEF_PRIMITIVE_TYPE (BT_INT16, short_integer_type_node)
>  DEF_PRIMITIVE_TYPE (BT_UINT8, char_type_node)
> -DEF_PRIMITIVE_TYPE (BT_UINT16, short_unsigned_type_node)
> +DEF_PRIMITIVE_TYPE (BT_UINT16, uint16_type_node)
>  DEF_PRIMITIVE_TYPE (BT_UINT32, uint32_type_node)
>  DEF_PRIMITIVE_TYPE (BT_UINT64, uint64_type_node)
>  DEF_PRIMITIVE_TYPE (BT_WORD, (*lang_hooks.types.type_for_mode) (word_mode, 
> 1))

This is ok for trunk.  Not sure about the INT8/INT16/UINT8 though,
1) char_type_node can be generally signed or unsigned, shouldn't that be
unsigned_char_type_node instead?
2) wonder what will happen on targets where char is 32-bit or similar.

> --- a/libhsail-rt/ChangeLog
> +++ b/libhsail-rt/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-01-26  Pekka Jskel??inen 
> +
> + * README: Added a proper description of what libhsail-rt is.
> +
>  2017-01-26  Jakub Jelinek  
>  
>   Update copyright years.
> diff --git a/libhsail-rt/README b/libhsail-rt/README
> index 2792253..64c2107 100644
> --- a/libhsail-rt/README
> +++ b/libhsail-rt/README
> @@ -1,4 +1,10 @@
> -Run autoconf2.64 && automake-1.11  to regenerate the buildfiles.
> -You might need to manually tweak the minor automake version number
> -in configure.ac and aclocal.m4 (search for 1.11.6) in case your
> -local 1.11 minor version doesn't match. 
> \ No newline at end of file
> +This library implements the agent-side runtime functionality required
> +to run HSA finalized programs produced by the BRIG frontend.
> +
> +The library contains both the code required to run kernels on the agent
> +and also functions implementing more complex HSAIL instructions.
> +
> +rt/workitems.c contains the runtime entry function that manages multiple
> +work-item execution using fibers or simple for-loops (in case of work groups
> +without barriers).  Otherwise, the rest of the source files mostly contain
> +functions that typically map directly to HSAIL instructions.

This looks good, but again, you should be able to commit without review if
you are gcc/brig and libhsail-rt maintainer.

Jakub

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

Hi,

Here's a patch which I believe should address all the pointed out BRIG
FE related issues that
have not been committed yet.

I didn't introduce new fixed width int type nodes to tree.h as that
felt too intrusive at this point.
Also, changes.html is still to add.

OK for trunk?

BR,
Pekka



On Thu, Jan 26, 2017 at 10:28 AM, Jakub Jelinek  wrote:
> On Thu, Jan 26, 2017 at 09:38:23AM +0200, Pekka Jääskeläinen wrote:
>> > I suppose that also contrib/update-copyright.py need to be updated?  (I
>> > never looked into that, so don't know.)
>>
>> Does it? The files are (c) FSF now. What should I do here exactly?
>
> I took care of this and committed following (it updates also 3 non-brig
> related files that slipped through with 2016-ish only Copyrights since
> Jan 1st).
>
> 2017-01-26  Jakub Jelinek  
>
> contrib/
> * update-copyright.py: Add libhsail-rt to self.default_dirs
> and call self.add_dir on it.  Add Intel Corporation to external
> authors.
> gcc/
> * brig-builtins.def: Update copyright years.
> * config/arm/arm_acle_builtins.def: Update copyright years.
> gcc/brig/
> Update copyright years.
> gcc/testsuite/
> * brig.dg/dg.exp: Update copyright years.
> * lib/brig-dg.exp: Update copyright years.
> * lib/brig.exp: Update copyright years.
> libhsail-rt/
> Update copyright years.
> libstdc++-v3/
> * libsupc++/eh_atomics.h: Update copyright years.
> * testsuite/20_util/unique_ptr/cons/default.cc: Update copyright 
> years.
>
> --- contrib/update-copyright.py (revision 244917)
> +++ contrib/update-copyright.py (working copy)
> @@ -663,6 +663,7 @@ class GCCCopyright (Copyright):
>  self.add_external_author ('Florida State University')
>  self.add_external_author ('Greg Colvin and Beman Dawes.')
>  self.add_external_author ('Hewlett-Packard Company')
> +self.add_external_author ('Intel Corporation')
>  self.add_external_author ('Information Technology Industry Council.')
>  self.add_external_author ('James Theiler, Brian Gough')
>  self.add_external_author ('Makoto Matsumoto and Takuji Nishimura,')
> @@ -710,6 +711,7 @@ class GCCCmdLine (CmdLine):
>  self.add_dir ('libgfortran')
>  # libgo is imported from upstream.
>  self.add_dir ('libgomp')
> +self.add_dir ('libhsail-rt')
>  self.add_dir ('libiberty')
>  self.add_dir ('libitm')
>  self.add_dir ('libobjc')
> @@ -735,6 +737,7 @@ class GCCCmdLine (CmdLine):
>  'libgcc',
>  'libgfortran',
>  'libgomp',
> +'libhsail-rt',
>  'libiberty',
>  'libitm',
>  'libobjc',
> --- gcc/brig/Make-lang.in   (revision 244917)
> +++ gcc/brig/Make-lang.in   (working copy)
> @@ -1,7 +1,7 @@
>  # Make-lang.in -- Top level -*- makefile -*- fragment for gcc BRIG (HSAIL)
>  # frontend.
>
> -# Copyright (C) 2015 Free Software Foundation, Inc.
> +# Copyright (C) 2015-2017 Free Software Foundation, Inc.
>
>  # This file is part of GCC.
>
> --- gcc/brig/brig-builtins.h(revision 244917)
> +++ gcc/brig/brig-builtins.h(working copy)
> @@ -1,5 +1,5 @@
>  /* brig-builtins.h -- brig builtin definitions
> -   Copyright (C) 2016 Free Software Foundation, Inc.
> +   Copyright (C) 2016-2017 Free Software Foundation, Inc.
>
> Contributed by Pekka Jaaskelainen 
> for General Processor Tech.
> --- gcc/brig/brig-c.h   (revision 244917)
> +++ gcc/brig/brig-c.h   (working copy)
> @@ -1,5 +1,5 @@
>  /* brig-c.h -- Header file for brig input's gcc C interface.
> -   Copyright (C) 2016 Free Software Foundation, Inc.
> +   Copyright (C) 2016-2017 Free Software Foundation, Inc.
> Contributed by Pekka Jaaskelainen 
> for General Processor Tech.
>
> --- gcc/brig/brig-lang.c(revision 244917)
> +++ gcc/brig/brig-lang.c(working copy)
> @@ -1,5 +1,5 @@
>  /* brig-lang.c -- brig (HSAIL) input gcc interface.
> -   Copyright (C) 2016 Free Software Foundation, Inc.
> +   Copyright (C) 2016-2017 Free Software Foundation, Inc.
> Contributed by Pekka Jaaskelainen 
> for General Processor Tech.
>
> --- gcc/brig/brigfrontend/brig-arg-block-handler.cc (revision 244917)
> +++ gcc/brig/brigfrontend/brig-arg-block-handler.cc (working copy)
> @@ -1,5 +1,5 @@
>  /* brig-arg-block-handler.cc -- brig arg block start/end directive handling
> -   Copyright (C) 2016 Free Software Foundation, Inc.
> +   Copyright (C) 2016-2017 Free Software Foundation, Inc.
> Contributed by Pekka Jaaskelainen 
> for General Processor Tech.
>
> --- gcc/brig/brigfrontend/brig-atomic-inst-handler.cc   (revision 244917)
> +++ gcc/brig/brigfrontend/brig-atomic-inst-handler.cc   (working copy)
> @@ -1,5 +1,5 @@
>  /* 

Re: [PATCH] BRIG frontend: request for a global review

[Jakub Jelinek]

On Thu, Jan 26, 2017 at 09:38:23AM +0200, Pekka Jääskeläinen wrote:
> > I suppose that also contrib/update-copyright.py need to be updated?  (I
> > never looked into that, so don't know.)
> 
> Does it? The files are (c) FSF now. What should I do here exactly?

I took care of this and committed following (it updates also 3 non-brig
related files that slipped through with 2016-ish only Copyrights since
Jan 1st).

2017-01-26  Jakub Jelinek  

contrib/
* update-copyright.py: Add libhsail-rt to self.default_dirs
and call self.add_dir on it.  Add Intel Corporation to external
authors.
gcc/
* brig-builtins.def: Update copyright years.
* config/arm/arm_acle_builtins.def: Update copyright years.
gcc/brig/
Update copyright years.
gcc/testsuite/
* brig.dg/dg.exp: Update copyright years.
* lib/brig-dg.exp: Update copyright years.
* lib/brig.exp: Update copyright years.
libhsail-rt/
Update copyright years.
libstdc++-v3/
* libsupc++/eh_atomics.h: Update copyright years.
* testsuite/20_util/unique_ptr/cons/default.cc: Update copyright years.

--- contrib/update-copyright.py (revision 244917)
+++ contrib/update-copyright.py (working copy)
@@ -663,6 +663,7 @@ class GCCCopyright (Copyright):
 self.add_external_author ('Florida State University')
 self.add_external_author ('Greg Colvin and Beman Dawes.')
 self.add_external_author ('Hewlett-Packard Company')
+self.add_external_author ('Intel Corporation')
 self.add_external_author ('Information Technology Industry Council.')
 self.add_external_author ('James Theiler, Brian Gough')
 self.add_external_author ('Makoto Matsumoto and Takuji Nishimura,')
@@ -710,6 +711,7 @@ class GCCCmdLine (CmdLine):
 self.add_dir ('libgfortran')
 # libgo is imported from upstream.
 self.add_dir ('libgomp')
+self.add_dir ('libhsail-rt')
 self.add_dir ('libiberty')
 self.add_dir ('libitm')
 self.add_dir ('libobjc')
@@ -735,6 +737,7 @@ class GCCCmdLine (CmdLine):
 'libgcc',
 'libgfortran',
 'libgomp',
+'libhsail-rt',
 'libiberty',
 'libitm',
 'libobjc',
--- gcc/brig/Make-lang.in   (revision 244917)
+++ gcc/brig/Make-lang.in   (working copy)
@@ -1,7 +1,7 @@
 # Make-lang.in -- Top level -*- makefile -*- fragment for gcc BRIG (HSAIL)
 # frontend.
 
-# Copyright (C) 2015 Free Software Foundation, Inc.
+# Copyright (C) 2015-2017 Free Software Foundation, Inc.
 
 # This file is part of GCC.
 
--- gcc/brig/brig-builtins.h(revision 244917)
+++ gcc/brig/brig-builtins.h(working copy)
@@ -1,5 +1,5 @@
 /* brig-builtins.h -- brig builtin definitions
-   Copyright (C) 2016 Free Software Foundation, Inc.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
 
Contributed by Pekka Jaaskelainen 
for General Processor Tech.
--- gcc/brig/brig-c.h   (revision 244917)
+++ gcc/brig/brig-c.h   (working copy)
@@ -1,5 +1,5 @@
 /* brig-c.h -- Header file for brig input's gcc C interface.
-   Copyright (C) 2016 Free Software Foundation, Inc.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
Contributed by Pekka Jaaskelainen 
for General Processor Tech.
 
--- gcc/brig/brig-lang.c(revision 244917)
+++ gcc/brig/brig-lang.c(working copy)
@@ -1,5 +1,5 @@
 /* brig-lang.c -- brig (HSAIL) input gcc interface.
-   Copyright (C) 2016 Free Software Foundation, Inc.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
Contributed by Pekka Jaaskelainen 
for General Processor Tech.
 
--- gcc/brig/brigfrontend/brig-arg-block-handler.cc (revision 244917)
+++ gcc/brig/brigfrontend/brig-arg-block-handler.cc (working copy)
@@ -1,5 +1,5 @@
 /* brig-arg-block-handler.cc -- brig arg block start/end directive handling
-   Copyright (C) 2016 Free Software Foundation, Inc.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
Contributed by Pekka Jaaskelainen 
for General Processor Tech.
 
--- gcc/brig/brigfrontend/brig-atomic-inst-handler.cc   (revision 244917)
+++ gcc/brig/brigfrontend/brig-atomic-inst-handler.cc   (working copy)
@@ -1,5 +1,5 @@
 /* brig-atomic-inst-handler.cc -- brig atomic instruction handling
-   Copyright (C) 2016 Free Software Foundation, Inc.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
 
Contributed by Pekka Jaaskelainen 
for General Processor Tech.
--- gcc/brig/brigfrontend/brig-basic-inst-handler.cc(revision 244917)
+++ gcc/brig/brigfrontend/brig-basic-inst-handler.cc(working copy)
@@ -1,5 +1,5 @@
 /* brig-basic-inst-handler.cc -- brig basic instruction handling
-   Copyright (C) 2016 Free Software Foundation, Inc.
+   Copyright 

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

On Wed, Jan 25, 2017 at 6:07 PM, Thomas Schwinge
 wrote:
> Hi!
>
> On Wed, 25 Jan 2017 13:21:13 +0100, Jakub Jelinek  wrote:
>> On Wed, Jan 25, 2017 at 11:00:50AM +0100, Thomas Schwinge wrote:
>> > On Tue, 24 Jan 2017 13:52:10 +0100, Martin Jambor  wrote:
>> > > [BRIG front end]
>
> $ git grep --cached libbrig
> gcc/brig/config-lang.in:target_libs="target-libbrig target-libhsail-rt"
>
> What is "libbrig"; should we remove that (as far as I can tell?) stale
> reference?

Yes, a leftover that can be removed.

> $ git show 55a56509bb4ae0c844c27f0679a22844bed3a3c5 -- libhsail-rt/README 
> | filterdiff
> --- /dev/null
> +++ libhsail-rt/README
> @@ -0,0 +1,4 @@
> +Run autoconf2.64 && automake-1.11  to regenerate the buildfiles.
> +You might need to manually tweak the minor automake version number
> +in configure.ac and aclocal.m4 (search for 1.11.6) in case your
> +local 1.11 minor version doesn't match.
> \ No newline at end of file
>
> I don't understand that "manually tweak" comment -- you should just
> install/build the right versions, and run "PATH=[...]:$PATH autoreconf",
> which is the same for all GCC subdirectories.

OK. I'll remove that. IIRC, I had some difficulties with getting the
exact minor versions
of autotools working together, and found out that the minor version
didn't matter
here, so left this as a note.

> Instead, the README file should contain a note what the "libhsail-rt"
> directory is about.

OK, I will add a note.

> $ git show 55a56509bb4ae0c844c27f0679a22844bed3a3c5 -- 
> gcc/builtin-types.def | filterdiff --hunks=1
> diff --git gcc/builtin-types.def gcc/builtin-types.def
> index 91745b4..ee6d052 100644
> --- gcc/builtin-types.def
> +++ gcc/builtin-types.def
> @@ -67,7 +67,10 @@ DEF_PRIMITIVE_TYPE (BT_LONGLONG, 
> long_long_integer_type_node)
>  DEF_PRIMITIVE_TYPE (BT_ULONGLONG, long_long_unsigned_type_node)
>  DEF_PRIMITIVE_TYPE (BT_INTMAX, intmax_type_node)
>  DEF_PRIMITIVE_TYPE (BT_UINTMAX, uintmax_type_node)
> -DEF_PRIMITIVE_TYPE (BT_UINT16, uint16_type_node)
> +DEF_PRIMITIVE_TYPE (BT_INT8, signed_char_type_node)
> +DEF_PRIMITIVE_TYPE (BT_INT16, short_integer_type_node)
> +DEF_PRIMITIVE_TYPE (BT_UINT8, char_type_node)
> +DEF_PRIMITIVE_TYPE (BT_UINT16, short_unsigned_type_node)
>  DEF_PRIMITIVE_TYPE (BT_UINT32, uint32_type_node)
>  DEF_PRIMITIVE_TYPE (BT_UINT64, uint64_type_node)
>  DEF_PRIMITIVE_TYPE (BT_WORD, (*lang_hooks.types.type_for_mode) 
> (word_mode, 1))
>
> Is that change alright?  For instance, uint16_type_node is still used
> elsewhere.  Some of these intN/uintN type_nodes apparently don't exist as
> global_trees; should they, and then be referred to here instead of the
> C-like type_nodes?

Yes, it makes sense. I will fix and test this.

> The "News" section on , and
>  should also be updated, I guess?
> :-)

Yes, of course. I will provide text.

> I suppose that also contrib/update-copyright.py need to be updated?  (I
> never looked into that, so don't know.)

Does it? The files are (c) FSF now. What should I do here exactly?

BR,
Pekka

Re: [PATCH] BRIG frontend: request for a global review

[Thomas Schwinge]

Hi!

On Wed, 25 Jan 2017 13:21:13 +0100, Jakub Jelinek  wrote:
> On Wed, Jan 25, 2017 at 11:00:50AM +0100, Thomas Schwinge wrote:
> > On Tue, 24 Jan 2017 13:52:10 +0100, Martin Jambor  wrote:
> > > [BRIG front end]

$ git grep --cached libbrig
gcc/brig/config-lang.in:target_libs="target-libbrig target-libhsail-rt"

What is "libbrig"; should we remove that (as far as I can tell?) stale
reference?


$ git show 55a56509bb4ae0c844c27f0679a22844bed3a3c5 -- libhsail-rt/README | 
filterdiff
--- /dev/null
+++ libhsail-rt/README
@@ -0,0 +1,4 @@
+Run autoconf2.64 && automake-1.11  to regenerate the buildfiles.
+You might need to manually tweak the minor automake version number
+in configure.ac and aclocal.m4 (search for 1.11.6) in case your
+local 1.11 minor version doesn't match. 
\ No newline at end of file

I don't understand that "manually tweak" comment -- you should just
install/build the right versions, and run "PATH=[...]:$PATH autoreconf",
which is the same for all GCC subdirectories.

Instead, the README file should contain a note what the "libhsail-rt"
directory is about.


$ git show 55a56509bb4ae0c844c27f0679a22844bed3a3c5 -- 
gcc/builtin-types.def | filterdiff --hunks=1
diff --git gcc/builtin-types.def gcc/builtin-types.def
index 91745b4..ee6d052 100644
--- gcc/builtin-types.def
+++ gcc/builtin-types.def
@@ -67,7 +67,10 @@ DEF_PRIMITIVE_TYPE (BT_LONGLONG, 
long_long_integer_type_node)
 DEF_PRIMITIVE_TYPE (BT_ULONGLONG, long_long_unsigned_type_node)
 DEF_PRIMITIVE_TYPE (BT_INTMAX, intmax_type_node)
 DEF_PRIMITIVE_TYPE (BT_UINTMAX, uintmax_type_node)
-DEF_PRIMITIVE_TYPE (BT_UINT16, uint16_type_node)
+DEF_PRIMITIVE_TYPE (BT_INT8, signed_char_type_node)
+DEF_PRIMITIVE_TYPE (BT_INT16, short_integer_type_node)
+DEF_PRIMITIVE_TYPE (BT_UINT8, char_type_node)
+DEF_PRIMITIVE_TYPE (BT_UINT16, short_unsigned_type_node)
 DEF_PRIMITIVE_TYPE (BT_UINT32, uint32_type_node)
 DEF_PRIMITIVE_TYPE (BT_UINT64, uint64_type_node)
 DEF_PRIMITIVE_TYPE (BT_WORD, (*lang_hooks.types.type_for_mode) (word_mode, 
1))

Is that change alright?  For instance, uint16_type_node is still used
elsewhere.  Some of these intN/uintN type_nodes apparently don't exist as
global_trees; should they, and then be referred to here instead of the
C-like type_nodes?


The "News" section on , and
 should also be updated, I guess?
:-)


By the way, see  "Questionable
-Wmisleading-indentation diagnostic in HSAIL-Tools" for a build problem
with HSAILasm that I ran into.  With that resolved (trivial), I'm
reporting from "gcc/testsuite/brig/brig.sum": "# of expected passes 95".

Just one concern there is output like:

[...]
PASS: brig.dg/test/gimple/mem.hsail (test for excess errors)
PASS: mem.hsail.brig scan-tree-dump original "__args;[\n ]+d0 ="
PASS: mem.hsail.brig scan-tree-dump original "\\(__args \\+ 8\\);[\n ]+d2 ="
[...]

..., that is, the "scan-tree-dump"s don't print the full filename of the
test case.  But that problem supposedly isn't specific to the BRIG test
cases.  (I may look into that later.)


> > "contrib/gcc_update" needs to be updated for "libhsail-rt".

Done.

I suppose that also contrib/update-copyright.py need to be updated?  (I
never looked into that, so don't know.)

> > Here is a patch to fix some Autotools issues in libhsail-rt (currently
> > testing); OK for trunk?

> Note, lots of this changed in r244895, so your patch doesn't apply any
> longer.  Still removing AC_CONFIG_MACRO_DIR, removing config.h.in and
> regenerating whatever is affected by that (most likely just configure)
> is in order.

Committed to trunk in r244902:

commit c8cd62c4e211f2e2bfabaf25a64842004e611797
Author: tschwinge 
Date:   Wed Jan 25 15:38:01 2017 +

libhsail-rt: Fix some Autotools issues

contrib/
* gcc_update (files_and_dependencies): Care for "libhsail-rt".

libhsail-rt/
* configure.ac: Don't instantiate AC_CONFIG_MACRO_DIR.
* configure: Regenerate.

libhsail-rt/
* config.h.in: Remove stale file.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@244902 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 contrib/ChangeLog|   4 +
 contrib/gcc_update   |   4 +
 libhsail-rt/ChangeLog|  13 +++
 libhsail-rt/config.h.in  | 217 ---
 libhsail-rt/configure|   6 +-
 libhsail-rt/configure.ac |   2 -
 6 files changed, 23 insertions(+), 223 deletions(-)

diff --git contrib/ChangeLog contrib/ChangeLog
index d429beb..2f862fa 100644
--- contrib/ChangeLog
+++ contrib/ChangeLog
@@ -1,3 +1,7 @@
+2017-01-25  Thomas Schwinge  
+
+   * gcc_update (files_and_dependencies): Care for 

Re: [PATCH] BRIG frontend: request for a global review

[Jakub Jelinek]

On Wed, Jan 25, 2017 at 11:00:50AM +0100, Thomas Schwinge wrote:
> Hi!
> 
> On Tue, 24 Jan 2017 13:52:10 +0100, Martin Jambor  wrote:
> > [BRIG front end]
> 
> "contrib/gcc_update" needs to be updated for "libhsail-rt".
> 
> 
> Here is a patch to fix some Autotools issues in libhsail-rt (currently
> testing); OK for trunk?
> 
> commit 00d64708323f74191ce5a39b223bca92295fc606
> Author: Thomas Schwinge 
> Date:   Wed Jan 25 10:33:56 2017 +0100
> 
> libhsail-rt: Fix some Autotools issues
> 
> * Makefile.am (ACLOCAL_AMFLAGS): Set to "-I .. -I ../config".
> * configure.ac: Don't instantiate AC_CONFIG_MACRO_DIR.
> * config.h.in: Remove stale file.
> * Makefile.in: Regenerate.
> * aclocal.m4: Regenerate.
> * configure: Regenerate.

Note, lots of this changed in r244895, so your patch doesn't apply any
longer.  Still removing AC_CONFIG_MACRO_DIR, removing config.h.in and
regenerating whatever is affected by that (most likely just configure)
is in order.

Jakub

Re: [PATCH] BRIG frontend: request for a global review

[Thomas Schwinge]

Hi!

On Tue, 24 Jan 2017 13:52:10 +0100, Martin Jambor  wrote:
> [BRIG front end]

"contrib/gcc_update" needs to be updated for "libhsail-rt".


Here is a patch to fix some Autotools issues in libhsail-rt (currently
testing); OK for trunk?

commit 00d64708323f74191ce5a39b223bca92295fc606
Author: Thomas Schwinge 
Date:   Wed Jan 25 10:33:56 2017 +0100

libhsail-rt: Fix some Autotools issues

* Makefile.am (ACLOCAL_AMFLAGS): Set to "-I .. -I ../config".
* configure.ac: Don't instantiate AC_CONFIG_MACRO_DIR.
* config.h.in: Remove stale file.
* Makefile.in: Regenerate.
* aclocal.m4: Regenerate.
* configure: Regenerate.
---
 libhsail-rt/Makefile.am  |   4 +-
 libhsail-rt/Makefile.in  |  18 ++--
 libhsail-rt/aclocal.m4   |  72 ++--
 libhsail-rt/config.h.in  | 217 ---
 libhsail-rt/configure|  25 +++---
 libhsail-rt/configure.ac |   2 -
 6 files changed, 71 insertions(+), 267 deletions(-)

diff --git libhsail-rt/Makefile.am libhsail-rt/Makefile.am
index ef12df8..3f8806a 100644
--- libhsail-rt/Makefile.am
+++ libhsail-rt/Makefile.am
@@ -44,14 +44,14 @@
 
 AUTOMAKE_OPTIONS = foreign subdir-objects
 
+ACLOCAL_AMFLAGS = -I .. -I ../config
+
 gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)
 
 MAINT_CHARSET = latin1
 
 mkinstalldirs = $(SHELL) $(toplevel_srcdir)/mkinstalldirs
 
-ACLOCAL_AMFLAGS = -I m4
-
 WARN_CFLAGS = $(WARN_FLAGS) $(WERROR)
 
 # -I/-D flags to pass when compiling.
diff --git libhsail-rt/Makefile.in libhsail-rt/Makefile.in
index 250cfbc..2e5c8df 100644
--- libhsail-rt/Makefile.in
+++ libhsail-rt/Makefile.in
@@ -97,17 +97,17 @@ build_triplet = @build@
 host_triplet = @host@
 target_triplet = @target@
 subdir = .
-DIST_COMMON = README $(srcdir)/Makefile.in $(srcdir)/Makefile.am \
-   $(top_srcdir)/configure $(am__configure_deps) \
-   $(srcdir)/target-config.h.in $(srcdir)/../mkinstalldirs \
-   $(srcdir)/../depcomp
+DIST_COMMON = README ChangeLog $(srcdir)/Makefile.in \
+   $(srcdir)/Makefile.am $(top_srcdir)/configure \
+   $(am__configure_deps) $(srcdir)/target-config.h.in \
+   $(srcdir)/../mkinstalldirs $(srcdir)/../depcomp
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
$(top_srcdir)/../config/lead-dot.m4 \
-   $(top_srcdir)/../config/multi.m4 $(top_srcdir)/../libtool.m4 \
-   $(top_srcdir)/../ltoptions.m4 $(top_srcdir)/../ltsugar.m4 \
-   $(top_srcdir)/../ltversion.m4 $(top_srcdir)/../lt~obsolete.m4 \
-   $(top_srcdir)/configure.ac
+   $(top_srcdir)/../config/override.m4 \
+   $(top_srcdir)/../libtool.m4 $(top_srcdir)/../ltoptions.m4 \
+   $(top_srcdir)/../ltsugar.m4 $(top_srcdir)/../ltversion.m4 \
+   $(top_srcdir)/../lt~obsolete.m4 $(top_srcdir)/configure.ac
 am__configure_deps = $(am__aclocal_m4_deps) $(CONFIGURE_DEPENDENCIES) \
$(ACLOCAL_M4)
 am__CONFIG_DISTCLEAN_FILES = config.status config.cache config.log \
@@ -300,10 +300,10 @@ top_build_prefix = @top_build_prefix@
 top_builddir = @top_builddir@
 top_srcdir = @top_srcdir@
 AUTOMAKE_OPTIONS = foreign subdir-objects
+ACLOCAL_AMFLAGS = -I .. -I ../config
 gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)
 MAINT_CHARSET = latin1
 mkinstalldirs = $(SHELL) $(toplevel_srcdir)/mkinstalldirs
-ACLOCAL_AMFLAGS = -I m4
 WARN_CFLAGS = $(WARN_FLAGS) $(WERROR)
 
 # -I/-D flags to pass when compiling.
diff --git libhsail-rt/aclocal.m4 libhsail-rt/aclocal.m4
index f77a2da..7a56c88 100644
--- libhsail-rt/aclocal.m4
+++ libhsail-rt/aclocal.m4
@@ -1,7 +1,8 @@
-# generated automatically by aclocal 1.11.1 -*- Autoconf -*-
+# generated automatically by aclocal 1.11.6 -*- Autoconf -*-
 
 # Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-# 2005, 2006, 2007, 2008, 2009  Free Software Foundation, Inc.
+# 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation,
+# Inc.
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.
@@ -19,12 +20,15 @@ You have another version of autoconf.  It may work, but is 
not guaranteed to.
 If you have problems, you may need to regenerate the build system entirely.
 To do so, use the procedure documented by the package, typically 
`autoreconf'.])])
 
-# Copyright (C) 2002, 2003, 2005, 2006, 2007, 2008  Free Software Foundation, 
Inc.
+# Copyright (C) 2002, 2003, 2005, 2006, 2007, 2008, 2011 Free Software
+# Foundation, Inc.
 #
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.
 
+# serial 1
+
 # AM_AUTOMAKE_VERSION(VERSION)
 # 
 # Automake X.Y traces this macro to ensure aclocal.m4 has 

Re: [PATCH] BRIG frontend: request for a global review

[David Malcolm]

On Tue, 2017-01-24 at 15:30 -0500, David Malcolm wrote:
> On Tue, 2017-01-24 at 13:52 +0100, Martin Jambor wrote:
> > Hi,
> > 
> > On Mon, Jan 23, 2017 at 02:11:37PM +0100, Richard Biener wrote:
> > > On Mon, Jan 23, 2017 at 1:02 PM, Martin Jambor 
> > > wrote:
> > > > Hi,
> > > > 
> > > > 
> > > > On Mon, Jan 23, 2017 at 12:56:13PM +0100, Richard Biener wrote:
> > > > > On Fri, Jan 20, 2017 at 6:25 PM, Pekka Jääskeläinen <
> > > > > pe...@parmance.com> wrote:
> > > > > > Hi Richard,
> > > > > > 
> > > > > > On Fri, Jan 20, 2017 at 10:26 AM, Richard Biener
> > > > > >  wrote:
> > > > > > > So the #ifdef ENABLE_BRIG_FE shouldn't be needed anymore
> > > > > > > (nor the
> > > > > > > configury for it).
> > > > > > > 
> > > > > > > Otherwise this looks ok to me then.
> > > > > > 
> > > > > > Attached is a patch set with that unnecessary
> > > > > > ENABLE_BRIG_FE
> > > > > > macro
> > > > > > removed.  It has also been refreshed to the latest trunk.
> > > > > 
> > > > > Are you set up to commit these to trunk yourself or do you
> > > > > need
> > > > > help here?
> > > > > 
> > > > 
> > > > Pekka is still in the process of requesting an account at
> > > > gcc.gnu.org.
> > > > We agreed that I would commit the patches.
> > > > 
> > > > Over Saturday and today morning, I have bootstrappet and tested
> > > > them
> > > > in various configurations (although mostly on x86_64 and only a
> > > > little
> > > > bit on aarch64) and (together with a "svn mv
> > > > libgomp/plugin/hsa.h
> > > > include/") I have not found any issue so far.
> > > > 
> > > > I suppose I should go ahead (in any case, if there is any
> > > > fallout,
> > > > reach out to me too, at least until Pekka gets his account).
> > > 
> > > Yes, sounds good to me.
> > > 
> > 
> > It took me more time because I got distracted by PR 79198 and did
> > not
> > want to commit another huge thing while bootstrap was still broken
> > for
> > so many people with my previous patch.  Nevertheless, I have just
> > committed the BRIG FE as revision 244867.
> > 
> > I tried to be careful (as well as quick to avoid conflicts) so I
> > hope
> > there are no issues.  However if there are some, throw complaints
> > my
> > way at least unless Pekka gets an account at gcc.gnu.org.
> > 
> > Thanks,
> 
> A deps issue for the docs I noticed when glancing through the commit:
> 
> diff --git a/gcc/brig/Make-lang.in b/gcc/brig/Make-lang.in
> new file mode 100644 (file)
> index 000..b85b1b0
> --- /dev/null
> +++ b/gcc/brig/Make-lang.in
> 
> [...snip...]
> 
> +# Documentation.
> +
> +GO_TEXI_FILES = \
> +   brig/gccbrig.texi \
> +   $(gcc_docdir)/include/fdl.texi \
> +   $(gcc_docdir)/include/gpl_v3.texi \
> +   $(gcc_docdir)/include/gcc-common.texi \
> +   gcc-vers.texi
> 
> Presumably this should be BRIG_TEXI_FILES, rather than GO_TEXI_FILES?
> 
> +# doc/gccbrig.info: $(BRIG_TEXI_FILES)
> +#  if test "x$(BUILD_INFO)" = xinfo; then \
> +#rm -f doc/gccbrig.info*; \
> +#$(MAKEINFO) $(MAKEINFOFLAGS) -I $(gcc_docdir) \
> +#  -I $(gcc_docdir)/include -o $@ $<; \
> +#  else true; fi
> +
> +# doc/gccbrig.dvi: $(BRIG_TEXI_FILES)
> +#  $(TEXI2DVI) -I $(abs_docdir) -I $(abs_docdir)/include -o $@
> $<
> +
> +# doc/gccbrig.pdf: $(BRIG_TEXI_FILES)
> +#  $(TEXI2PDF) -I $(abs_docdir) -I $(abs_docdir)/include -o $@
> $<
> +
> +$(build_htmldir)/brig/index.html: $(BRIG_TEXI_FILES)
> +   $(mkinstalldirs) $(@D)
> +   rm -f $(@D)/*
> +   $(TEXI2HTML) -I $(gcc_docdir) -I $(gcc_docdir)/include \
> +   -I $(srcdir)/brig -o $(@D) $<
> 
> ...for use in describing the deps of the above.
> 
> Dave

Also: grepping for "Copyright" in the commit: the new files mostly say:

+   Copyright (C) 2016 Free Software Foundation, Inc.

but I see at least one:

+   Copyright (C) 2015-2016 Free Software Foundation, Inc.

and some others.

Should the copyright be updated to cover 2017?

Re: [PATCH] BRIG frontend: request for a global review

[David Malcolm]

On Tue, 2017-01-24 at 13:52 +0100, Martin Jambor wrote:
> Hi,
> 
> On Mon, Jan 23, 2017 at 02:11:37PM +0100, Richard Biener wrote:
> > On Mon, Jan 23, 2017 at 1:02 PM, Martin Jambor 
> > wrote:
> > > Hi,
> > > 
> > > 
> > > On Mon, Jan 23, 2017 at 12:56:13PM +0100, Richard Biener wrote:
> > > > On Fri, Jan 20, 2017 at 6:25 PM, Pekka Jääskeläinen <
> > > > pe...@parmance.com> wrote:
> > > > > Hi Richard,
> > > > > 
> > > > > On Fri, Jan 20, 2017 at 10:26 AM, Richard Biener
> > > > >  wrote:
> > > > > > So the #ifdef ENABLE_BRIG_FE shouldn't be needed anymore
> > > > > > (nor the
> > > > > > configury for it).
> > > > > > 
> > > > > > Otherwise this looks ok to me then.
> > > > > 
> > > > > Attached is a patch set with that unnecessary ENABLE_BRIG_FE
> > > > > macro
> > > > > removed.  It has also been refreshed to the latest trunk.
> > > > 
> > > > Are you set up to commit these to trunk yourself or do you need
> > > > help here?
> > > > 
> > > 
> > > Pekka is still in the process of requesting an account at
> > > gcc.gnu.org.
> > > We agreed that I would commit the patches.
> > > 
> > > Over Saturday and today morning, I have bootstrappet and tested
> > > them
> > > in various configurations (although mostly on x86_64 and only a
> > > little
> > > bit on aarch64) and (together with a "svn mv libgomp/plugin/hsa.h
> > > include/") I have not found any issue so far.
> > > 
> > > I suppose I should go ahead (in any case, if there is any
> > > fallout,
> > > reach out to me too, at least until Pekka gets his account).
> > 
> > Yes, sounds good to me.
> > 
> 
> It took me more time because I got distracted by PR 79198 and did not
> want to commit another huge thing while bootstrap was still broken
> for
> so many people with my previous patch.  Nevertheless, I have just
> committed the BRIG FE as revision 244867.
> 
> I tried to be careful (as well as quick to avoid conflicts) so I hope
> there are no issues.  However if there are some, throw complaints my
> way at least unless Pekka gets an account at gcc.gnu.org.
> 
> Thanks,

A deps issue for the docs I noticed when glancing through the commit:

diff --git a/gcc/brig/Make-lang.in b/gcc/brig/Make-lang.in
new file mode 100644 (file)
index 000..b85b1b0
--- /dev/null
+++ b/gcc/brig/Make-lang.in

[...snip...]

+# Documentation.
+
+GO_TEXI_FILES = \
+   brig/gccbrig.texi \
+   $(gcc_docdir)/include/fdl.texi \
+   $(gcc_docdir)/include/gpl_v3.texi \
+   $(gcc_docdir)/include/gcc-common.texi \
+   gcc-vers.texi

Presumably this should be BRIG_TEXI_FILES, rather than GO_TEXI_FILES?

+# doc/gccbrig.info: $(BRIG_TEXI_FILES)
+#  if test "x$(BUILD_INFO)" = xinfo; then \
+#rm -f doc/gccbrig.info*; \
+#$(MAKEINFO) $(MAKEINFOFLAGS) -I $(gcc_docdir) \
+#  -I $(gcc_docdir)/include -o $@ $<; \
+#  else true; fi
+
+# doc/gccbrig.dvi: $(BRIG_TEXI_FILES)
+#  $(TEXI2DVI) -I $(abs_docdir) -I $(abs_docdir)/include -o $@ $<
+
+# doc/gccbrig.pdf: $(BRIG_TEXI_FILES)
+#  $(TEXI2PDF) -I $(abs_docdir) -I $(abs_docdir)/include -o $@ $<
+
+$(build_htmldir)/brig/index.html: $(BRIG_TEXI_FILES)
+   $(mkinstalldirs) $(@D)
+   rm -f $(@D)/*
+   $(TEXI2HTML) -I $(gcc_docdir) -I $(gcc_docdir)/include \
+   -I $(srcdir)/brig -o $(@D) $<

...for use in describing the deps of the above.

Dave

Re: [PATCH] BRIG frontend: request for a global review

[Martin Jambor]

Hi,

On Mon, Jan 23, 2017 at 02:11:37PM +0100, Richard Biener wrote:
> On Mon, Jan 23, 2017 at 1:02 PM, Martin Jambor  wrote:
> > Hi,
> >
> >
> > On Mon, Jan 23, 2017 at 12:56:13PM +0100, Richard Biener wrote:
> >> On Fri, Jan 20, 2017 at 6:25 PM, Pekka Jääskeläinen  
> >> wrote:
> >> > Hi Richard,
> >> >
> >> > On Fri, Jan 20, 2017 at 10:26 AM, Richard Biener
> >> >  wrote:
> >> >> So the #ifdef ENABLE_BRIG_FE shouldn't be needed anymore (nor the
> >> >> configury for it).
> >> >>
> >> >> Otherwise this looks ok to me then.
> >> >
> >> > Attached is a patch set with that unnecessary ENABLE_BRIG_FE macro
> >> > removed.  It has also been refreshed to the latest trunk.
> >>
> >> Are you set up to commit these to trunk yourself or do you need help here?
> >>
> >
> > Pekka is still in the process of requesting an account at gcc.gnu.org.
> > We agreed that I would commit the patches.
> >
> > Over Saturday and today morning, I have bootstrappet and tested them
> > in various configurations (although mostly on x86_64 and only a little
> > bit on aarch64) and (together with a "svn mv libgomp/plugin/hsa.h
> > include/") I have not found any issue so far.
> >
> > I suppose I should go ahead (in any case, if there is any fallout,
> > reach out to me too, at least until Pekka gets his account).
> 
> Yes, sounds good to me.
> 

It took me more time because I got distracted by PR 79198 and did not
want to commit another huge thing while bootstrap was still broken for
so many people with my previous patch.  Nevertheless, I have just
committed the BRIG FE as revision 244867.

I tried to be careful (as well as quick to avoid conflicts) so I hope
there are no issues.  However if there are some, throw complaints my
way at least unless Pekka gets an account at gcc.gnu.org.

Thanks,

Martin

Re: [PATCH] BRIG frontend: request for a global review

[Richard Biener]

On Mon, Jan 23, 2017 at 1:02 PM, Martin Jambor  wrote:
> Hi,
>
>
> On Mon, Jan 23, 2017 at 12:56:13PM +0100, Richard Biener wrote:
>> On Fri, Jan 20, 2017 at 6:25 PM, Pekka Jääskeläinen  
>> wrote:
>> > Hi Richard,
>> >
>> > On Fri, Jan 20, 2017 at 10:26 AM, Richard Biener
>> >  wrote:
>> >> So the #ifdef ENABLE_BRIG_FE shouldn't be needed anymore (nor the
>> >> configury for it).
>> >>
>> >> Otherwise this looks ok to me then.
>> >
>> > Attached is a patch set with that unnecessary ENABLE_BRIG_FE macro
>> > removed.  It has also been refreshed to the latest trunk.
>>
>> Are you set up to commit these to trunk yourself or do you need help here?
>>
>
> Pekka is still in the process of requesting an account at gcc.gnu.org.
> We agreed that I would commit the patches.
>
> Over Saturday and today morning, I have bootstrappet and tested them
> in various configurations (although mostly on x86_64 and only a little
> bit on aarch64) and (together with a "svn mv libgomp/plugin/hsa.h
> include/") I have not found any issue so far.
>
> I suppose I should go ahead (in any case, if there is any fallout,
> reach out to me too, at least until Pekka gets his account).

Yes, sounds good to me.

Richard.

> Thanks,
>
> Martin
>

Re: [PATCH] BRIG frontend: request for a global review

[Martin Jambor]

Hi,


On Mon, Jan 23, 2017 at 12:56:13PM +0100, Richard Biener wrote:
> On Fri, Jan 20, 2017 at 6:25 PM, Pekka Jääskeläinen  
> wrote:
> > Hi Richard,
> >
> > On Fri, Jan 20, 2017 at 10:26 AM, Richard Biener
> >  wrote:
> >> So the #ifdef ENABLE_BRIG_FE shouldn't be needed anymore (nor the
> >> configury for it).
> >>
> >> Otherwise this looks ok to me then.
> >
> > Attached is a patch set with that unnecessary ENABLE_BRIG_FE macro
> > removed.  It has also been refreshed to the latest trunk.
> 
> Are you set up to commit these to trunk yourself or do you need help here?
> 

Pekka is still in the process of requesting an account at gcc.gnu.org.
We agreed that I would commit the patches.

Over Saturday and today morning, I have bootstrappet and tested them
in various configurations (although mostly on x86_64 and only a little
bit on aarch64) and (together with a "svn mv libgomp/plugin/hsa.h
include/") I have not found any issue so far.

I suppose I should go ahead (in any case, if there is any fallout,
reach out to me too, at least until Pekka gets his account).

Thanks,

Martin

Re: [PATCH] BRIG frontend: request for a global review

[Richard Biener]

On Fri, Jan 20, 2017 at 6:25 PM, Pekka Jääskeläinen  wrote:
> Hi Richard,
>
> On Fri, Jan 20, 2017 at 10:26 AM, Richard Biener
>  wrote:
>> So the #ifdef ENABLE_BRIG_FE shouldn't be needed anymore (nor the
>> configury for it).
>>
>> Otherwise this looks ok to me then.
>
> Attached is a patch set with that unnecessary ENABLE_BRIG_FE macro
> removed.  It has also been refreshed to the latest trunk.

Are you set up to commit these to trunk yourself or do you need help here?

Thanks,
Richard.

> Thanks,
> Pekka

Re: [PATCH] BRIG frontend: request for a global review

[Richard Biener]

On Thu, Jan 19, 2017 at 6:46 PM, Pekka Jääskeläinen  wrote:
> Hi Jakub and Richard,
>
> Attached is an updated BRIG FE patch which adds the HSAIL related
> builtins only internally in the BRIG FE. I didn't add LTO support as I
> believe it's not
> useful for BRIG FE due to it always inputting fully linked BRIGs and
> not mixing with
> other frontends.

So the #ifdef ENABLE_BRIG_FE shouldn't be needed anymore (nor the
configury for it).

Otherwise this looks ok to me then.

Thanks,
Richard.

> BR,
> Pekka
>
>
>
> On Mon, Jan 16, 2017 at 11:07 AM, Jakub Jelinek  wrote:
>> On Mon, Jan 16, 2017 at 09:46:43AM +0100, Richard Biener wrote:
>>> There are 187 of them (well, simple grep of DEF_HSAIL, so probably a bit 
>>> less).
>>> They aren't really documented but I guess that __hsail_bitmask_u64 for 
>>> example
>>> is really equivalent to sth like -1U >> n << m?  So I'm not sure why
>>> you have builtins
>>> like these represened as functions rather than as "expanded" code sequences?
>>>
>>> If that's the ones you are talking about having special target
>>> specific expansion.
>>>
>>> Note that builtins add to GCC startup times and if you don't expect
>>> people to enable
>>> BRIG then I wonder why you are submitting it for inclusion ;)
>>
>> I guess the question is when the DEF_HSAIL* builtins are actually needed.
>> If the FE is separate from the other FEs, I guess it would be enough to
>> define those builtins
>> 1) in the BRIG FE
>> 2) in tree-core.h
>> 3) in lto1 (only if any such builtin has been seen in the IL
>>
>> So, perhaps define DEF_HSAIL* just to DEF_BUILTIN_STUB in builtins.def
>> unless already defined, and override it in the BRIG FE where you create its
>> builtins, and then have some routine in the middle-end similar to
>> initialize_sanitizer_builtins which lazily initializes the DEF_HSAIL*
>> builtins during LTO reading if a call to any of the builtins in the hsail
>> range is noticed?
>>
>> Jakub

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

Hi Jakub and Richard,

Attached is an updated BRIG FE patch which adds the HSAIL related
builtins only internally in the BRIG FE. I didn't add LTO support as I
believe it's not
useful for BRIG FE due to it always inputting fully linked BRIGs and
not mixing with
other frontends.

BR,
Pekka



On Mon, Jan 16, 2017 at 11:07 AM, Jakub Jelinek  wrote:
> On Mon, Jan 16, 2017 at 09:46:43AM +0100, Richard Biener wrote:
>> There are 187 of them (well, simple grep of DEF_HSAIL, so probably a bit 
>> less).
>> They aren't really documented but I guess that __hsail_bitmask_u64 for 
>> example
>> is really equivalent to sth like -1U >> n << m?  So I'm not sure why
>> you have builtins
>> like these represened as functions rather than as "expanded" code sequences?
>>
>> If that's the ones you are talking about having special target
>> specific expansion.
>>
>> Note that builtins add to GCC startup times and if you don't expect
>> people to enable
>> BRIG then I wonder why you are submitting it for inclusion ;)
>
> I guess the question is when the DEF_HSAIL* builtins are actually needed.
> If the FE is separate from the other FEs, I guess it would be enough to
> define those builtins
> 1) in the BRIG FE
> 2) in tree-core.h
> 3) in lto1 (only if any such builtin has been seen in the IL
>
> So, perhaps define DEF_HSAIL* just to DEF_BUILTIN_STUB in builtins.def
> unless already defined, and override it in the BRIG FE where you create its
> builtins, and then have some routine in the middle-end similar to
> initialize_sanitizer_builtins which lazily initializes the DEF_HSAIL*
> builtins during LTO reading if a call to any of the builtins in the hsail
> range is noticed?
>
> Jakub


002-brig-fe-gcc.patch.gz
Description: GNU Zip compressed data


005-diff-to-previous.patch.gz
Description: GNU Zip compressed data

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

OK,

I'll see into adapting the Jakub's idea and also check if some of the
simplest builtins are better expanded
directly to tree nodes instead.

I'm not sure if lto support is needed though as the assumption now is
to have fully linked input to this FE
(all necessary BRIG modules fed in at build time which can be
guaranteed by the HSA finalizer runtime API)
without modules from other FEs never linked in.

Quickly testing, I couldn't measure startup time difference with the
builtins on/off, but of course there
will be some cycles wasted somewhere especially if some code in gcc
traverses all the builtins.

For the audience that might want to enable the FE, I expect the BRIG
FE to be used with x86_64
to have an HSA CPU Agent e.g. with phsa to avoid the need for a HSA
supported GPU in the
machine for developing HSA using projects, and also by other HSA
finalizer implementations
downstream or up. Hopefully this includes also finalization for
AMDGPUs or PTX in the future (if someone
is interested in implementing it). So, there might not be "mainstream
needs" currently that would
warrant enabling by default currently for masses, but hopefully more
so in the future after HSA gets
wider adoption.

Thanks,
Pekka

On Mon, Jan 16, 2017 at 11:07 AM, Jakub Jelinek  wrote:
> On Mon, Jan 16, 2017 at 09:46:43AM +0100, Richard Biener wrote:
>> There are 187 of them (well, simple grep of DEF_HSAIL, so probably a bit 
>> less).
>> They aren't really documented but I guess that __hsail_bitmask_u64 for 
>> example
>> is really equivalent to sth like -1U >> n << m?  So I'm not sure why
>> you have builtins
>> like these represened as functions rather than as "expanded" code sequences?
>>
>> If that's the ones you are talking about having special target
>> specific expansion.
>>
>> Note that builtins add to GCC startup times and if you don't expect
>> people to enable
>> BRIG then I wonder why you are submitting it for inclusion ;)
>
> I guess the question is when the DEF_HSAIL* builtins are actually needed.
> If the FE is separate from the other FEs, I guess it would be enough to
> define those builtins
> 1) in the BRIG FE
> 2) in tree-core.h
> 3) in lto1 (only if any such builtin has been seen in the IL
>
> So, perhaps define DEF_HSAIL* just to DEF_BUILTIN_STUB in builtins.def
> unless already defined, and override it in the BRIG FE where you create its
> builtins, and then have some routine in the middle-end similar to
> initialize_sanitizer_builtins which lazily initializes the DEF_HSAIL*
> builtins during LTO reading if a call to any of the builtins in the hsail
> range is noticed?
>
> Jakub

Re: [PATCH] BRIG frontend: request for a global review

[Jakub Jelinek]

On Mon, Jan 16, 2017 at 09:46:43AM +0100, Richard Biener wrote:
> There are 187 of them (well, simple grep of DEF_HSAIL, so probably a bit 
> less).
> They aren't really documented but I guess that __hsail_bitmask_u64 for example
> is really equivalent to sth like -1U >> n << m?  So I'm not sure why
> you have builtins
> like these represened as functions rather than as "expanded" code sequences?
> 
> If that's the ones you are talking about having special target
> specific expansion.
> 
> Note that builtins add to GCC startup times and if you don't expect
> people to enable
> BRIG then I wonder why you are submitting it for inclusion ;)

I guess the question is when the DEF_HSAIL* builtins are actually needed.
If the FE is separate from the other FEs, I guess it would be enough to
define those builtins
1) in the BRIG FE
2) in tree-core.h
3) in lto1 (only if any such builtin has been seen in the IL

So, perhaps define DEF_HSAIL* just to DEF_BUILTIN_STUB in builtins.def
unless already defined, and override it in the BRIG FE where you create its
builtins, and then have some routine in the middle-end similar to
initialize_sanitizer_builtins which lazily initializes the DEF_HSAIL*
builtins during LTO reading if a call to any of the builtins in the hsail
range is noticed?

Jakub

Re: [PATCH] BRIG frontend: request for a global review

[Richard Biener]

On Fri, Jan 13, 2017 at 4:54 PM, Pekka Jääskeläinen  wrote:
> On Fri, Jan 13, 2017 at 2:34 PM, Richard Biener
>  wrote:
>> On Thu, Jan 12, 2017 at 3:55 PM, Pekka Jääskeläinen  
>> wrote:
>>> Hi,
>>>
>>> A gentle ping...
>>
>> Looking at 002/
>>
>> What's the reason of having brig-builtins.def?  I do not see any
>> middle-end stuff using them
>> and if you just emit calls to the runtime then you do not need
>> "builtins" for this -- just build
>> the function decls in the frontend.  The Fortran frontend has examples
>> for how to do that
>> for almost all entries to the libgfortran runtime.
>>
>> So all changes besides in brig/ look unnecessary to me.
>
> Most of the HSAIL instructions represented as builtins here are fine
> grained enough for
> them to be candidates for expanding as sensible target instruction
> sequences instead of runtime
> function calls.
>
> I understand that there is no code in any of the targets to do so yet.
> But still I wonder is it
> meaningful to convert them to direct runtime function calls -- if/when
> a target wants
> to optimize their HSAIL codegen they have to be moved back. Especially given
> the BRIG FE is not enabled by default and the inclusion of the BRIG
> builtins is done
> only if enabled, does it cause harm?

There are 187 of them (well, simple grep of DEF_HSAIL, so probably a bit less).
They aren't really documented but I guess that __hsail_bitmask_u64 for example
is really equivalent to sth like -1U >> n << m?  So I'm not sure why
you have builtins
like these represened as functions rather than as "expanded" code sequences?

If that's the ones you are talking about having special target
specific expansion.

Note that builtins add to GCC startup times and if you don't expect
people to enable
BRIG then I wonder why you are submitting it for inclusion ;)

Richard.

> BR,
> --Pekka

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

On Fri, Jan 13, 2017 at 11:36 PM, Richard Biener
 wrote:
> Ah, with fibers fixed there's only the fp16.c part left.  Presumably LGPL 
> overall is OK as well (but I'm not a lawyer either).

Seems fp16.c has the GPL3 runtime exception in place, so I believe we
are good here license-wise:

"Under Section 7 of GPL version 3, you are granted additional
   permissions described in the GCC Runtime Library Exception, version
   3.1, as published by the Free Software Foundation."

BR,
Pekka

Re: [PATCH] BRIG frontend: request for a global review

[Richard Biener]

On January 13, 2017 4:43:30 PM GMT+01:00, "Pekka Jääskeläinen" 
 wrote:
>Hi Richard,
>
>Thanks for the review! Replies/questions inline.
>
>On Fri, Jan 13, 2017 at 2:28 PM, Richard Biener
> wrote:
>> On Thu, Jan 12, 2017 at 3:55 PM, Pekka Jääskeläinen
> wrote:
>>> Hi,
>>>
>>> A gentle ping...
>>
>> diff --git a/gcc/configure.ac b/gcc/configure.ac
>> index 140b9f9..06941c5 100644
>> --- a/gcc/configure.ac
>> +++ b/gcc/configure.ac
>> @@ -996,6 +996,11 @@ if test x"$enable_hsa" = x1 ; then
>>  [Define this to enable support for generating HSAIL.])
>>  fi
>>
>> +if echo "$enable_languages" | grep "brig" > /dev/null; then
>> +  AC_DEFINE(ENABLE_BRIG_FE, 1,
>> +[Define this to enable the BRIG (HSAIL) frontend.])
>> +fi
>> +
>>
>> this looks odd and I'd have expected this to be solely controlled via
>> --enable-languages at configure time?
>
>Yes, it is. This AC_DEFINE just adds an autoconf variable
>ENABLE_BRIG_FE
>using which I guard unnecessary inclusion of the brig-builtins.def in
>builtins.def in
>case BRIG FE is disabled.
>
>> @@ -1366,6 +1369,11 @@ assembler  assembler-with-cpp
>>  ada
>>  f77  f77-cpp-input f95  f95-cpp-input
>>  go
>> +<<< HEAD
>> +java
>> +brig
>> +===
>> +>>> gcc-master
>>  @end smallexample
>>
>>  @item -x none
>>
>> unmerged hunk (java is gone).
>
>Cleaned.
>
>> diff --git a/include/hsa-interface.h b/include/hsa-interface.h
>> new file mode 100644
>> index 000..6765751
>> --- /dev/null
>> +++ b/include/hsa-interface.h
>> @@ -0,0 +1,630 @@
>> +/* HSA runtime API 1.0.1 representation description.
>> +   Copyright (C) 2016 Free Software Foundation, Inc.
>> +
>> ...
>>
>> this looks like a sub/superset of libgomp/plugin/hsa.h, please work
>on
>> retaining only one version.
>
>Merged.
>
>> Did you check whether libhsail-rt builds for all GCC targets?  If not
>> you probably want to add
>> a list where to disable the frontend and its runtime (see go / libgo
>> for an example in the
>> toplevel configure.ac).
>
>I have tested only x86 of the GCC upstream targets so far.
>
>Libgo seems to black list known broken ones with a separate
>--enable-libgo
>switch to force enable.  However, BRIG FE and libhsail-rt are disabled
>by default.
>Should I still add a separate switch to force enable BRIG and enable
>it by default for x86?
>
>Or should I force disable it even when enabled with --enable-languages
>when
>building for an untested target?

Go is also not enabled by default.  It's to tell the user the frontend/library 
won't work (and thus disable the frontend even if requested).

Richard.

>
>
>Thanks,
>Pekka

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

On Fri, Jan 13, 2017 at 2:34 PM, Richard Biener
 wrote:
> On Thu, Jan 12, 2017 at 3:55 PM, Pekka Jääskeläinen  
> wrote:
>> Hi,
>>
>> A gentle ping...
>
> Looking at 002/
>
> What's the reason of having brig-builtins.def?  I do not see any
> middle-end stuff using them
> and if you just emit calls to the runtime then you do not need
> "builtins" for this -- just build
> the function decls in the frontend.  The Fortran frontend has examples
> for how to do that
> for almost all entries to the libgfortran runtime.
>
> So all changes besides in brig/ look unnecessary to me.

Most of the HSAIL instructions represented as builtins here are fine
grained enough for
them to be candidates for expanding as sensible target instruction
sequences instead of runtime
function calls.

I understand that there is no code in any of the targets to do so yet.
But still I wonder is it
meaningful to convert them to direct runtime function calls -- if/when
a target wants
to optimize their HSAIL codegen they have to be moved back. Especially given
the BRIG FE is not enabled by default and the inclusion of the BRIG
builtins is done
only if enabled, does it cause harm?

BR,
--Pekka

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

Hi Richard,

Thanks for the review! Replies/questions inline.

On Fri, Jan 13, 2017 at 2:28 PM, Richard Biener
 wrote:
> On Thu, Jan 12, 2017 at 3:55 PM, Pekka Jääskeläinen  
> wrote:
>> Hi,
>>
>> A gentle ping...
>
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 140b9f9..06941c5 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -996,6 +996,11 @@ if test x"$enable_hsa" = x1 ; then
>  [Define this to enable support for generating HSAIL.])
>  fi
>
> +if echo "$enable_languages" | grep "brig" > /dev/null; then
> +  AC_DEFINE(ENABLE_BRIG_FE, 1,
> +[Define this to enable the BRIG (HSAIL) frontend.])
> +fi
> +
>
> this looks odd and I'd have expected this to be solely controlled via
> --enable-languages at configure time?

Yes, it is. This AC_DEFINE just adds an autoconf variable ENABLE_BRIG_FE
using which I guard unnecessary inclusion of the brig-builtins.def in
builtins.def in
case BRIG FE is disabled.

> @@ -1366,6 +1369,11 @@ assembler  assembler-with-cpp
>  ada
>  f77  f77-cpp-input f95  f95-cpp-input
>  go
> +<<< HEAD
> +java
> +brig
> +===
> +>>> gcc-master
>  @end smallexample
>
>  @item -x none
>
> unmerged hunk (java is gone).

Cleaned.

> diff --git a/include/hsa-interface.h b/include/hsa-interface.h
> new file mode 100644
> index 000..6765751
> --- /dev/null
> +++ b/include/hsa-interface.h
> @@ -0,0 +1,630 @@
> +/* HSA runtime API 1.0.1 representation description.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +
> ...
>
> this looks like a sub/superset of libgomp/plugin/hsa.h, please work on
> retaining only one version.

Merged.

> Did you check whether libhsail-rt builds for all GCC targets?  If not
> you probably want to add
> a list where to disable the frontend and its runtime (see go / libgo
> for an example in the
> toplevel configure.ac).

I have tested only x86 of the GCC upstream targets so far.

Libgo seems to black list known broken ones with a separate --enable-libgo
switch to force enable.  However, BRIG FE and libhsail-rt are disabled
by default.
Should I still add a separate switch to force enable BRIG and enable
it by default for x86?

Or should I force disable it even when enabled with --enable-languages when
building for an untested target?


Thanks,
Pekka

Re: [PATCH] BRIG frontend: request for a global review

[Pekka Jääskeläinen]

Hi,

A gentle ping...

On Wed, Dec 14, 2016 at 7:15 PM, Pekka Jääskeläinen  wrote:
> Hi,
>
> I'm calling for a global review for the BRIG frontend for inclusion in 
> upstream.
> The copyright transfer has been taken care of.
>
> The patch set has been approved by Martin Jambor, the upcoming co-maintainer 
> of
> the BRIG frontend, who asked me to send an updated patch set for a
> global reviewer
> to look at.
>
> Please find the patches attached to this email. An introduction and
> a diffstat is below.
>
> Best regards,
> Pekka
>
>
> 
>
> This patch set adds a BRIG (HSAIL) frontend. It can be used as a core
> for an HSAIL finalizer implementation for processors with gcc backends.
>
> It is a bit unusual frontend as the consumed format is a binary
> representation.  The textual HSAIL can be compiled to it with a separate
> assembler tool.
>
> The frontend has been mostly tested with the HSA 1.0 PRM conformance suite
> which it now passes. The accompanied GENERIC-scanning test suite is supposed
> to be only a smoke test.
>
> libhsail-rt implements HSAIL specific builtins and includes a simple runtime
> that implements SPMD execution via setcontext()/getcontext() or loops to
> execute multiple work-item work groups without SPMD/SIMD-default hardware.
>
> We've split it to 4 patches:
>
> 001 - the configuration file changes and misc.
> 002 - the frontend itself
> 003 - libhsail-rt
> 004 - the smoke test suite
>
> The diffstat is as follows:
>
>  .gitignore| 2 +-
>  Makefile.def  | 3 +
>  Makefile.in   |   489 +
>  configure | 1 +
>  configure.ac  | 1 +
>  gcc/brig-builtins.def |   659 +
>  gcc/brig/Make-lang.in |   247 +
>  gcc/brig/brig-builtins.h  |99 +
>  gcc/brig/brig-c.h |66 +
>  gcc/brig/brig-lang.c  |   770 +
>  gcc/brig/brigfrontend/brig-arg-block-handler.cc   |66 +
>  gcc/brig/brigfrontend/brig-atomic-inst-handler.cc |   265 +
>  gcc/brig/brigfrontend/brig-basic-inst-handler.cc  |   865 +
>  gcc/brig/brigfrontend/brig-branch-inst-handler.cc |   221 +
>  gcc/brig/brigfrontend/brig-cmp-inst-handler.cc|   198 +
>  gcc/brig/brigfrontend/brig-code-entry-handler.cc  |  1719 ++
>  gcc/brig/brigfrontend/brig-code-entry-handler.h   |   425 +
>  gcc/brig/brigfrontend/brig-comment-handler.cc |39 +
>  gcc/brig/brigfrontend/brig-control-handler.cc |   108 +
>  .../brigfrontend/brig-copy-move-inst-handler.cc   |73 +
>  gcc/brig/brigfrontend/brig-cvt-inst-handler.cc|   260 +
>  gcc/brig/brigfrontend/brig-fbarrier-handler.cc|44 +
>  gcc/brig/brigfrontend/brig-function-handler.cc|   373 +
>  gcc/brig/brigfrontend/brig-function.cc|   723 +
>  gcc/brig/brigfrontend/brig-function.h |   213 +
>  gcc/brig/brigfrontend/brig-inst-mod-handler.cc|58 +
>  gcc/brig/brigfrontend/brig-label-handler.cc   |37 +
>  gcc/brig/brigfrontend/brig-lane-inst-handler.cc   |84 +
>  gcc/brig/brigfrontend/brig-machine.c  |44 +
>  gcc/brig/brigfrontend/brig-machine.h  |33 +
>  gcc/brig/brigfrontend/brig-mem-inst-handler.cc|   180 +
>  gcc/brig/brigfrontend/brig-module-handler.cc  |41 +
>  gcc/brig/brigfrontend/brig-queue-inst-handler.cc  |93 +
>  gcc/brig/brigfrontend/brig-seg-inst-handler.cc|   146 +
>  gcc/brig/brigfrontend/brig-signal-inst-handler.cc |42 +
>  gcc/brig/brigfrontend/brig-to-generic.cc  |   811 +
>  gcc/brig/brigfrontend/brig-to-generic.h   |   226 +
>  gcc/brig/brigfrontend/brig-util.cc|   447 +
>  gcc/brig/brigfrontend/brig-util.h |53 +
>  gcc/brig/brigfrontend/brig-variable-handler.cc|   264 +
>  gcc/brig/brigfrontend/phsa.h  |69 +
>  gcc/brig/brigspec.c   |   135 +
>  gcc/brig/config-lang.in   |41 +
>  gcc/brig/lang-specs.h |28 +
>  gcc/brig/lang.opt |41 +
>  gcc/builtin-types.def |80 +-
>  gcc/builtins.def  |41 +
>  gcc/config.in | 6 +
>  gcc/configure | 6 +
>  gcc/configure.ac  | 5 +
>  gcc/doc/frontends.texi| 2 +-
>  gcc/doc/invoke.texi   | 8 +
>  gcc/doc/standards.texi| 8 +
>  gcc/testsuite/brig.dg/README  |10 +
>  gcc/testsuite/brig.dg/dg.exp  |27 +
>