Re: [PATCH] Harmonize headers between both dg-extract-results scripts

2023-09-18 Thread Paul Iannetta via Gcc-patches
On Thu, Sep 14, 2023 at 04:24:33PM +0200, Paul Iannetta wrote:
> Hi,
> 
> This is a small patch so that both dg-extract-results.py and
> dg-extract-results.sh share the same header.  In particular, it fixes
> the fact that the regexp r'^Test Run By (\S+) on (.*)$' was never
> matched in the python file.

By the way, the bash script dg-extract-results.sh checks whether
python is available by invoking python.  However, it seems that the
policy on newer machines is to not provide python as a symlink (at
least on Ubuntu 22.04 and above; and RHEL 8).  Therefore, we might
want to also check against python3 so that the bash script does not
fail to find python even though it is available.

Thanks,
Paul


> Author: Paul Iannetta 
> Date:   Thu Sep 14 15:43:58 2023 +0200
> 
> Harmonize headers between both dg-extract-results scripts
> 
> The header of the python version looked like:
> Target is ...
> Host   is ...
> The header of the bash version looked like:
> Test run by ... on ...
> Target is ...
> 
> After this change both headers look like:
> Test run by ... on ...
> Target is ...
> Host   is ...
> 
> The order of the tests is not the same but since dg-cmp-results.sh it
> does not matter much.
> 
> contrib/ChangeLog:
> 
> 2023-09-14  Paul Iannetta  
> 
> * dg-extract-results.py: Print the "Test run" line.
> * dg-extract-results.sh: Print the "Host" line.
> 
> diff --git a/contrib/dg-extract-results.py b/contrib/dg-extract-results.py
> index 30aa68771d4..34da1808c5f 100644
> --- a/contrib/dg-extract-results.py
> +++ b/contrib/dg-extract-results.py
> @@ -113,7 +113,7 @@ class Prog:
>  # Whether to create .sum rather than .log output.
>  self.do_sum = True
>  # Regexps used while parsing.
> -self.test_run_re = re.compile (r'^Test Run By (\S+) on (.*)$')
> +self.test_run_re = re.compile (r'^Test run by (\S+) on (.*)$')
>  self.tool_re = re.compile (r'^\t\t=== (.*) tests ===$')
>  self.result_re = re.compile (r'^(PASS|XPASS|FAIL|XFAIL|UNRESOLVED'
>   r'|WARNING|ERROR|UNSUPPORTED|UNTESTED'
> diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
> index ff6c50d029c..57f6fe0e997 100755
> --- a/contrib/dg-extract-results.sh
> +++ b/contrib/dg-extract-results.sh
> @@ -271,7 +271,7 @@ cat $SUM_FILES \
>  
>  # Write the begining of the combined summary file.
>  
> -head -n 2 $FIRST_SUM
> +head -n 3 $FIRST_SUM
>  echo
>  echo "   === $TOOL tests ==="
>  echo






[PATCH] Harmonize headers between both dg-extract-results scripts

2023-09-14 Thread Paul Iannetta via Gcc-patches
Hi,

This is a small patch so that both dg-extract-results.py and
dg-extract-results.sh share the same header.  In particular, it fixes
the fact that the regexp r'^Test Run By (\S+) on (.*)$' was never
matched in the python file.

Thanks,
--
Paul
Kalray


Author: Paul Iannetta 
Date:   Thu Sep 14 15:43:58 2023 +0200

Harmonize headers between both dg-extract-results scripts

The header of the python version looked like:
Target is ...
Host   is ...
The header of the bash version looked like:
Test run by ... on ...
Target is ...

After this change both headers look like:
Test run by ... on ...
Target is ...
Host   is ...

The order of the tests is not the same but since dg-cmp-results.sh it
does not matter much.

contrib/ChangeLog:

2023-09-14  Paul Iannetta  

* dg-extract-results.py: Print the "Test run" line.
* dg-extract-results.sh: Print the "Host" line.

diff --git a/contrib/dg-extract-results.py b/contrib/dg-extract-results.py
index 30aa68771d4..34da1808c5f 100644
--- a/contrib/dg-extract-results.py
+++ b/contrib/dg-extract-results.py
@@ -113,7 +113,7 @@ class Prog:
 # Whether to create .sum rather than .log output.
 self.do_sum = True
 # Regexps used while parsing.
-self.test_run_re = re.compile (r'^Test Run By (\S+) on (.*)$')
+self.test_run_re = re.compile (r'^Test run by (\S+) on (.*)$')
 self.tool_re = re.compile (r'^\t\t=== (.*) tests ===$')
 self.result_re = re.compile (r'^(PASS|XPASS|FAIL|XFAIL|UNRESOLVED'
  r'|WARNING|ERROR|UNSUPPORTED|UNTESTED'
diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index ff6c50d029c..57f6fe0e997 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -271,7 +271,7 @@ cat $SUM_FILES \
 
 # Write the begining of the combined summary file.
 
-head -n 2 $FIRST_SUM
+head -n 3 $FIRST_SUM
 echo
 echo " === $TOOL tests ==="
 echo






Re: [PATCH v4] c++: parser - Support for target address spaces in C++

2022-11-10 Thread Paul Iannetta via Gcc-patches
Hi,

It took a bit of time to rework the rough corners.  I tried to be
mirror as much as possible the C front-end, especially when it comes
to implicit conversions, and warnings; and expose the same hooks as
the C front-end does.  The C++ front-end produces as much warnings
that the C front-end, however they are sometimes a bit less
informative (especially so, when passing arguments to functions).

I also address the following points:

  1.  The handling of the register storage class is grouped with the
  other variables with automatic storage.  This incurs a slight
  dis-alignment since you cannot have global register variables do not
  trigger an error, only a warning.

  2. In template unification, I maintain that we don't want any
  changes to address spaces whatsoever during the unification process,
  hence ignoring the strict flag.  Nevertheless, we allow implicit
  conversion, and I have verified that, indeed,


template  void f(T **);
struct A {
   template  operator T*__seg_fs*();
};
int main()
{
   f((void* __seg_fs *)0);   // (1): void*__seg_fs* -> void** should be 
OK
   void (*p)(void * __seg_fs *) = f; // (2): error
}

works as intended. That is, (1) works if we set __seg_fs as a subspace
of the generic address space, and (2) is always an error.

  3. In template unification, when unifying __as1 T = __as2 U we want
  to unify to the __as1 at most, never to __as2 at most, because the
  function requiring __as1 T may want to mix address space from the
  bigger address space, therefore I think that we want to have the
  smaller address-space to be unified into the bigger of the two.

  4. I left untouched same_type_ignoring_top_level_qualifiers_p, even
  though that was very convenient and often lead to better error
  messages since error were caught earlier.

  5. The handling of conversions is done very late in the calling
  chain, because I absolutely want to fold the conversion and force
  the conversion to appear as an ADDR_SPACE_CONV_EXPR after
  gimplification.

  6.  Currently, I do not handle classes. I see what I can do in a
  further revision and maybe add a target hook to hand down to targets
  the choice of the address space of the vtable.

  7.  This can't be added as a test case, but I checked that:

 // In this test case, __seg_gs is a subset of the generic address
 // space.

 int f (int *);
 int main ()
 {
   int __seg_fs *pa;
   int __seg_gs *pb;
   int *pc;
   pa = (__seg_fs int *) pb; return *pa; // warning: cast to ‘__seg_fs’ address 
space pointer from disjoint ‘__seg_gs’ address space pointer
   pa = (int __seg_fs *) pc; return *pa; // warning: cast to ‘__seg_fs’ address 
space pointer from disjoint generic address space pointer
   pa = pb; return *pa; //  error: invalid conversion from ‘__seg_gs int*’ to 
‘__seg_fs int*’
   pc = pb; return *pc; // __seg_gs int * -> int * converted through 
ADDR_SPACE_CONV_EXPR
   pb = pc; return *pb; // error: invalid conversion from ‘int*’ to ‘__seg_gs 
int*’
   pc = pa; return *pb; //  error: invalid conversion from ‘__seg_fs int*’ to 
‘int*’
   return f (pb); // __seg_gs int * -> int * converted through 
ADDR_SPACE_CONV_EXPR
   // return f (pa); // error: invalid conversion from ‘__seg_fs int*’ to ‘int*’
 // note:   initializing argument 1 of ‘int f(int*)’
 }

Thanks,
Paul

Rebased on current trunk, bootstrapped and regtested.
#  >8 
gcc/cp/ChangeLog:

2022-11-09  Paul Iannetta  

* call.cc (convert_like_internal): Add support for implicit
  conversion between compatible address spaces.  This is done
  here (and not in a higher caller) because we want to force the
  use of cp_fold_convert, which later enable back-end writers to
  tune-up the fine details of the conversion.
* cp-tree.h (enum cp_decl_spec): Add a new decl spec to handle
  address spaces.
(struct cp_decl_specifier_seq): Likewise.
* decl.cc (get_type_quals): Add address space support.
(check_tag_decl): Likewise.
(grokdeclarator): Likewise.
* class.cc (fixed_type_or_null): Add a case for
  ADDR_SPACE_CONVERT_EXPR.
* constexpr.cc (cxx_eval_constant_expression): Likewise.
(potential_constant_expression_1): Likewise.
* cp-gimplify.cc (cp_fold): Likewise.
* error.cc (dump_expr): Likewise.
* expr.cc (mark_use): Likewise.
* tree.cc (cp_stabilize_reference): Likewise.
(strip_typedefs_expr): Likewise.
(cp_tree_equal): Likewise.
(mark_exp_read): Likewise.
* mangle.cc (write_CV_qualifiers_for_type): Mangle address
  spaces using the extended type qualifier scheme.
* parser.cc (cp_lexer_get_preprocessor_token): Add a call to
  targetm.addr_space.diagnose_usage when lexing an address space
  token.
(cp_parser_postfix_expression): Prevent the use of address
spaces with compound 

Re: [PATCH v3] c++: parser - Support for target address spaces in C++

2022-11-10 Thread Paul Iannetta via Gcc-patches
On Thu, Nov 03, 2022 at 02:38:39PM +0100, Georg-Johann Lay wrote:
> > [PATCH v3] c++: parser - Support for target address spaces in C++
> 
> First of all, it is great news that GCC is going to implement named address
> spaces for C++.
> 
> I have some questions:
> 
> 1. How is name-mangling going to work?
> ==
> 
> Clang supports address spaces in C++, and for address-space 1 it does
> generate code like the following:
> 
> #define __flash __attribute__((__address_space__(1)))
> 
> char get_p (const __flash char *p)
> {
> return *p;
> }
> 
> 
> _Z5get_pPU3AS1Kc:
>...
> 
> I.e. address-space 1 is mangled as "AS1".
> 
> (Notice that Clang's attribute actually works like a qualifier here, one
> could not get this to work with GCC attributes.)
> 

Currently, the __address_space__ attribute does not exist in GCC, and
the definition of address-spaces as well as how they are laid-out can
only modified in the back-end.

I agree that this is a convenient attribute to define address-spaces
on the fly, but this lacks the flexibility of specifying which
address-spaces are subsets of others and how they should be promoted.

The mangling will be done with the extended qualifiers extension, for
example, your example would be mangled into "_Z5get_pPU7__flashKc".

> 
> 2. Will it work with compound literals?
> ===
> 
> Currently, the following C code works for target avr:
> 
> const __flash char *pHallo = (const __flash char[]) { "Hallo" };
> 
> This is a pointer in RAM (AS0) that holds the address of a string in flash
> (AS1) and is initialized with that address. Unfortunately, this does not
> work locally:
> 
> const __flash char* get_hallo (void)
> {
> [static] const __flash char *p2 = (const __flash char[]) { "Hallo2" };
> return p2;
> }
> 
> foo.c: In function 'get_hallo':
> foo.c: error: compound literal qualified by address-space qualifier
> 
> Is there any way to make this work now? Would be great!
> 

Currently, I implement the same restrictions as the C front-end, but I
think that this restriction could be lifted.

> 
> 3. Will TARGET_ADDR_SPACE_DIAGNOSE_USAGE still work?
> 
> 
> Currently there is target hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE.
> I did not see it in your patches, so maybe I just missed it? See
> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gccint/Named-Address-Spaces.html#index-TARGET_005fADDR_005fSPACE_005fDIAGNOSE_005fUSAGE
> 

That was a point I overlooked in my previous patch.  This will be in
my new revision where I also add implicit conversion between
address spaces and also expose TARGET_ADDR_SPACE_CONVERT.

> 
> 4. Will it be possible to put C++ virtual tables in ASs, and how?
> =
> 

Currently, I do not allow the declaration of instances of classes in
an address space, mainly to not have to cope with the handling of the
this pointer.  That is,

  __flash Myclass *t;

does not work.  Nevertheless, I admit that this is would be nice to
have.

> One big complaint about avr-g++ is that there is no way to put vtables in
> flash (address-space 1) and to access them accordingly.  How can this be
> achieved with C++ address spaces?

Do you want only the vtables in the flash address space or do you want
to be able to have the whole class content.

1. If you only want the vtables, I think that a target hook called
at vtable creation would do the trick.

2. If you want to be able to have pointer to classes in __flash, I
will need to further the support I have currently implemented to
support the pointer this qualified with an address space.
Retrospectively, I think this have to be implemented.

Paul

> 
> Background: The AVR architecture has non-linear address space, and you
> cannot tell from the numeric value of an address whether it's in RAM or
> flash. You will have to use different instructions depending on the
> location.
> 
> This means that .rodata must be located in RAM, because otherwise one would
> not know whether const char* pointed to RAM or flash, but to de-reference
> you's need different instructions.
> 
> One way out is named address spaces, so we could finally fix
> 
> https://gcc.gnu.org/PR43745
> 
> 
> Regards,
> 
> Johann
> 






Re: [PATCH v3] c++: parser - Support for target address spaces in C++

2022-10-26 Thread Paul Iannetta via Gcc-patches
On Wed, Oct 19, 2022 at 02:55:21PM -0400, Jason Merrill wrote:
> On 10/18/22 13:01, Paul Iannetta wrote:
> > Thank you very much for the detailed review.
> > 
> > On Tue, Oct 18, 2022 at 10:24:23AM -0400, Jason Merrill wrote:
> > > On 10/18/22 03:37, Paul Iannetta wrote:
> > > > On Fri, Oct 14, 2022 at 11:19:50AM -0400, Jason Merrill wrote:
> > > > > On 10/13/22 17:57, Paul Iannetta wrote:
> > > > > > On Thu, Oct 13, 2022 at 03:41:16PM -0400, Jason Merrill wrote:
> > > > > > > On 10/13/22 12:02, Paul Iannetta wrote:
> > > > > > > > On Thu, Oct 13, 2022 at 11:47:42AM -0400, Jason Merrill wrote:
> > > > > > > > > On 10/13/22 11:23, Paul Iannetta wrote:
> > > > > > > > > > On Thu, Oct 13, 2022 at 11:02:24AM -0400, Jason Merrill 
> > > > > > > > > > wrote:
> > > > > > > > > > > On 10/12/22 20:52, Paul Iannetta wrote:
> > > > > > > > > > > > There are quite a few things I would like to clarify 
> > > > > > > > > > > > concerning some
> > > > > > > > > > > > implementation details.
> > > > > > > > > > > > - A variable with automatic storage (which is 
> > > > > > > > > > > > neither a pointer nor
> > > > > > > > > > > >   a reference) cannot be qualified with an 
> > > > > > > > > > > > address space.  I detect
> > > > > > > > > > > >   this by the combination of `sc_none' and `! 
> > > > > > > > > > > > toplevel_bindings_p ()',
> > > > > > > > > > > >   but I've also seen the use of 
> > > > > > > > > > > > `at_function_scope' at other places.
> > > > > > > > > > > >   And I'm unsure which one is appropriate here.
> > > > > > > > > > > >   This detection happens at the very end of 
> > > > > > > > > > > > grokdeclarator because I
> > > > > > > > > > > >   need to know that the type is a pointer, 
> > > > > > > > > > > > which is not know until
> > > > > > > > > > > >   very late in the function.
> > > > > > > > > > > 
> > > > > > > > > > > At that point you have the decl, and you can ask directly 
> > > > > > > > > > > what its storage
> > > > > > > > > > > duration is, perhaps using decl_storage_duration.
> > > > > > > > > > > 
> > > > > > > > > > > But why do you need to know whether the type is a 
> > > > > > > > > > > pointer?  The attribute
> > > > > > > > > > > applies to the target type of the pointer, not the 
> > > > > > > > > > > pointer type.  I think
> > > > > > > > > > > the problem is that you're looking at declspecs when you 
> > > > > > > > > > > ought to be looking
> > > > > > > > > > > at type_quals.
> > > > > > > > > > 
> > > > > > > > > > I need to know that the base type is a pointer to reject 
> > > > > > > > > > invalid
> > > > > > > > > > declarations such as:
> > > > > > > > > > 
> > > > > > > > > >  int f (__seg_fs int a) { } or int f () { 
> > > > > > > > > > __seg_fs int a; }
> > > > > > > > > > 
> > > > > > > > > > because parameters and auto variables can have an address 
> > > > > > > > > > space
> > > > > > > > > > qualifier only if they are pointer or reference type, which 
> > > > > > > > > > I can't
> > > > > > > > > > tell only from type_quals.
> > > > > > > > > 
> > > > > > > > > But "int *__seg_fs a" is just as invalid as the above; the 
> > > > > > > > > difference is not
> > > > > > > > > whether a is a pointer, but whether the 
> > > > > > > > > address-space-qualified is the type
> > > > > > > > > of a itself or some sub-type.
> > > > > > > > 
> > > > > > > > I agree that "int * __seg_fs a" is invalid but it is accepted 
> > > > > > > > by the C
> > > > > > > > front-end, and by clang (both C and C++), the behavior is that 
> > > > > > > > the
> > > > > > > > address-name is silently ignored.
> > > > > > > 
> > > > > > > Hmm, that sounds like a bug; in that case, presumably the user 
> > > > > > > meant to
> > > > > > > qualify the pointed-to type, and silently ignoring seems unlikely 
> > > > > > > to give
> > > > > > > the effect they want.
> > > > > > > 
> > > > > > 
> > > > > > Well, actually, I'm re-reading the draft and "int * __seg_fs a" is
> > > > > > valid.  It means "pointer in address space __seg_fs pointing to an
> > > > > > object in the generic address space", whereas "__seg_fs int * a" 
> > > > > > means
> > > > > > "pointer in the generic address space pointing to an object in the
> > > > > > __seg_fs address-space".
> > > > > > 
> > > > > > Oddities such as, "__seg_fs int * __seg_gs a" are also perfectly
> > > > > > valid.
> > > > > 
> > > > > If a has static storage duration, sure; I was still thinking about
> > > > > declarations with automatic storage duration such as in your example 
> > > > > above.
> > > > > 
> > > > 
> > > > Thanks, I only use type_quals now. I also took into account the style
> > > > recommendations from Jakub, and included the other template tests.
> > > > I rebased over trunk, bootstrapped the compiler and run the "make
> > > > check-gcc" with no regressions on x86.
> > > > 
> > > > Paul
> > > > 
> > > > #  >8 

Re: Announcement: Porting the Docs to Sphinx - 9. November 2022

2022-10-19 Thread Paul Iannetta via Gcc-patches
On Wed, Oct 19, 2022 at 09:24:06AM +0200, Martin Liška wrote:
> On 10/17/22 16:16, Paul Iannetta wrote:
> > Hi Martin,
> > 
> > Thank you very much for porting the documentation to Sphinx, it is
> > very convenient to use, especially the menu on the left and the
> > search bar.
> 
> Thanks, I also like it!
> 
> > 
> > However, I also regularly browse and search the documentation through
> > info, especially when I want to use regexps to search or need to
> > include a special character (eg.,+,-,_,(; this can happen when I
> > search for '(define' ) for example) in the search string.
> > 
> > Does the port to Sphinx means the end of texinfo? Or, will both be
> > available as it is the case now with the official texinfo and your
> > unofficial splichal.eu pages.
> 
> It will be still available same as now where manual pages and info pages
> are built if you compile GCC from sources. We haven't been publishing manual
> pages and info pages on our web pages, people typically get these from
> their distribution packages.

As long as it is possible to build the info manual with "make info", even 
through
something like rst2texinfo,  I would be happy.  Would it be possible
to see the rst source of the port so as to try rst2texinfo on it?

> 
> Does it help? Or do you expect any change regarding what we publish at:
> https://gcc.gnu.org/onlinedocs/
> ?
Currently, there is a tarball with texinfo sources for all the manuals
for each version.

Thanks,
Paul

> 
> Cheers,
> Martin
> 
> > 
> > Paul
> > 
> > On Mon, Oct 17, 2022 at 03:28:34PM +0200, Martin Liška wrote:
> >> Hello.
> >>
> >> Based on the very positive feedback I was given at the Cauldron Sphinx 
> >> Documentation BoF,
> >> I'm planning migrating the documentation on 9th November. There are still 
> >> some minor comments
> >> from Sandra when it comes to the PDF output, but we can address that once 
> >> the conversion is done.
> >>
> >> The reason I'm sending the email now is that I was waiting for latest 
> >> Sphinx release (5.3.0) that
> >> simplifies reference format for options and results in much simpler Option 
> >> summary section ([1])
> >>
> >> The current GCC master (using Sphinx 5.3.0) converted docs can be seen 
> >> here:
> >> https://splichal.eu/scripts/sphinx/
> >>
> >> If you see any issues with the converted documentation, or have a feedback 
> >> about it,
> >> please reply to this email.
> >>
> >> Cheers,
> >> Martin
> >>
> >> [1] https://github.com/sphinx-doc/sphinx/pull/10840
> >> [1] 
> >> https://splichal.eu/scripts/sphinx/gcc/_build/html/gcc-command-options/option-summary.html
> >>
> >>
> >>
> >>
> > 
> > 
> > 
> > 
> 
> 
> 
> 
> 






Re: [PATCH v3] c++: parser - Support for target address spaces in C++

2022-10-18 Thread Paul Iannetta via Gcc-patches
Thank you very much for the detailed review.

On Tue, Oct 18, 2022 at 10:24:23AM -0400, Jason Merrill wrote:
> On 10/18/22 03:37, Paul Iannetta wrote:
> > On Fri, Oct 14, 2022 at 11:19:50AM -0400, Jason Merrill wrote:
> > > On 10/13/22 17:57, Paul Iannetta wrote:
> > > > On Thu, Oct 13, 2022 at 03:41:16PM -0400, Jason Merrill wrote:
> > > > > On 10/13/22 12:02, Paul Iannetta wrote:
> > > > > > On Thu, Oct 13, 2022 at 11:47:42AM -0400, Jason Merrill wrote:
> > > > > > > On 10/13/22 11:23, Paul Iannetta wrote:
> > > > > > > > On Thu, Oct 13, 2022 at 11:02:24AM -0400, Jason Merrill wrote:
> > > > > > > > > On 10/12/22 20:52, Paul Iannetta wrote:
> > > > > > > > > > There are quite a few things I would like to clarify 
> > > > > > > > > > concerning some
> > > > > > > > > > implementation details.
> > > > > > > > > >- A variable with automatic storage (which is 
> > > > > > > > > > neither a pointer nor
> > > > > > > > > >  a reference) cannot be qualified with an address 
> > > > > > > > > > space.  I detect
> > > > > > > > > >  this by the combination of `sc_none' and `! 
> > > > > > > > > > toplevel_bindings_p ()',
> > > > > > > > > >  but I've also seen the use of `at_function_scope' 
> > > > > > > > > > at other places.
> > > > > > > > > >  And I'm unsure which one is appropriate here.
> > > > > > > > > >  This detection happens at the very end of 
> > > > > > > > > > grokdeclarator because I
> > > > > > > > > >  need to know that the type is a pointer, which is 
> > > > > > > > > > not know until
> > > > > > > > > >  very late in the function.
> > > > > > > > > 
> > > > > > > > > At that point you have the decl, and you can ask directly 
> > > > > > > > > what its storage
> > > > > > > > > duration is, perhaps using decl_storage_duration.
> > > > > > > > > 
> > > > > > > > > But why do you need to know whether the type is a pointer?  
> > > > > > > > > The attribute
> > > > > > > > > applies to the target type of the pointer, not the pointer 
> > > > > > > > > type.  I think
> > > > > > > > > the problem is that you're looking at declspecs when you 
> > > > > > > > > ought to be looking
> > > > > > > > > at type_quals.
> > > > > > > > 
> > > > > > > > I need to know that the base type is a pointer to reject invalid
> > > > > > > > declarations such as:
> > > > > > > > 
> > > > > > > > int f (__seg_fs int a) { } or int f () { 
> > > > > > > > __seg_fs int a; }
> > > > > > > > 
> > > > > > > > because parameters and auto variables can have an address space
> > > > > > > > qualifier only if they are pointer or reference type, which I 
> > > > > > > > can't
> > > > > > > > tell only from type_quals.
> > > > > > > 
> > > > > > > But "int *__seg_fs a" is just as invalid as the above; the 
> > > > > > > difference is not
> > > > > > > whether a is a pointer, but whether the address-space-qualified 
> > > > > > > is the type
> > > > > > > of a itself or some sub-type.
> > > > > > 
> > > > > > I agree that "int * __seg_fs a" is invalid but it is accepted by 
> > > > > > the C
> > > > > > front-end, and by clang (both C and C++), the behavior is that the
> > > > > > address-name is silently ignored.
> > > > > 
> > > > > Hmm, that sounds like a bug; in that case, presumably the user meant 
> > > > > to
> > > > > qualify the pointed-to type, and silently ignoring seems unlikely to 
> > > > > give
> > > > > the effect they want.
> > > > > 
> > > > 
> > > > Well, actually, I'm re-reading the draft and "int * __seg_fs a" is
> > > > valid.  It means "pointer in address space __seg_fs pointing to an
> > > > object in the generic address space", whereas "__seg_fs int * a" means
> > > > "pointer in the generic address space pointing to an object in the
> > > > __seg_fs address-space".
> > > > 
> > > > Oddities such as, "__seg_fs int * __seg_gs a" are also perfectly
> > > > valid.
> > > 
> > > If a has static storage duration, sure; I was still thinking about
> > > declarations with automatic storage duration such as in your example 
> > > above.
> > > 
> > 
> > Thanks, I only use type_quals now. I also took into account the style
> > recommendations from Jakub, and included the other template tests.
> > I rebased over trunk, bootstrapped the compiler and run the "make
> > check-gcc" with no regressions on x86.
> > 
> > Paul
> > 
> > #  >8 
> > Add support for custom address spaces in C++
> > 
> > gcc/
> >  * tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.
> > 
> > gcc/c/
> >  * c-decl.cc: Remove c_register_addr_space.
> > 
> > gcc/c-family/
> >  * c-common.cc (c_register_addr_space): Imported from c-decl.cc
> >  (addr_space_superset): Imported from gcc/c/c-typecheck.cc
> >  * c-common.h: Remove the FIXME.
> >  (addr_space_superset): New declaration.
> > 
> > gcc/cp/
> >  * cp-tree.h (enum cp_decl_spec): Add addr_space support.
> >  

Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does)

2022-10-18 Thread Paul Iannetta via Gcc-patches
On Mon, Oct 17, 2022 at 09:22:23AM +0200, Richard Biener wrote:
> On Fri, Oct 14, 2022 at 4:18 PM Paul Iannetta via Gcc-patches
>  wrote:
> >
> > On Wed, Oct 12, 2022 at 01:18:19AM +0200, Paul Iannetta wrote:
> > > On Mon, Oct 10, 2022 at 11:07:06PM +, Joseph Myers wrote:
> > > > On Mon, 10 Oct 2022, Paul Iannetta via Gcc-patches wrote:
> > > >
> > > > > I have a patch to bring this feature to the C front-end as well, and
> > > > > would like to hear your opinion on it, especially since it may affect
> > > > > the feature-set of the objc front-end as well.
> > > >
> > > > > Currently, this is only a tentative patch and I did not add any tests
> > > > > to the testsuite.
> > > >
> > > > I think tests (possibly existing C++ tests moved to c-c++-common?) are
> > > > necessary to judge such a feature; it could better be judged based on
> > > > tests without implementation than based on implementation without tests.
> > >
> > > Currently, this feature has the following tests in g++.dg/ext/
> > >   - vector9.C
> > >   - vector19.C
> > >   - vector21.C
> > >   - vector22.C
> > >   - vector23.C
> > >   - vector27.C
> > >   - vector28.C
> > > provided by Marc Glisse when he implemented the feature for C++.
> > >
> > > They are all handled by my mirror implementation (after removing
> > > C++-only features), save for a case in vector19.C ( v ? '1' : '2',
> > > where v is a vector of unsigned char, but '1' and '2' are considered
> > > as int, which results in a type mismatch.)
> > >
> > > I'll move those tests to c-c++-common tomorrow, but will duplicate
> > > vector19.C and vector23.C which rely on C++-only features.
> > >
> > > During my tests, I've been using variations around this:
> > >
> > > typedef int v2si __attribute__((__vector_size__ (2 * sizeof(int;
> > >
> > > v2si f (v2si a, v2si b, v2si c)
> > > {
> > >   v2si d = a + !b;
> > >   v2si e = a || b;
> > >   return c ? (a + !b) && (c - e && a) : (!!b ^ c && e);
> > > }
> > >
> > > It is already possible to express much of the same thing without the
> > > syntactic sugar but is is barely legible
> > >
> > > typedef int v2si __attribute__((__vector_size__ (2 * sizeof(int;
> > >
> > > v2si f (v2si a, v2si b, v2si c)
> > > {
> > >   v2si d = a + (b == 0);
> > >   v2si e = (a != 0) | (b != 0);
> > >   return ((c != 0) & (((a + (b == 0)) != 0) & (((c - e) != 0) & (a != 
> > > 0
> > >| ((c == 0) & (b == 0) == 0) ^ c) != 0) & (e != 0)));
> > > }
> > >
> > > Paul
> >
> > I still need to check what is done by clang on the objc side, but in
> > order to not conflict with what was done before, a warning is
> > triggered by c_obj_common_truthvalue_conversion and
> > build_unary_operator warns if '!' is used with a vector.  Both warnings
> > are only triggered in pedantic mode as suggested by Iain Sandoe.
> >
> > The support of the binary ops and unary ops works as the C++ front-end
> > does, there is however the case of the ternary conditional operator,
> > where the C standard mandates the promotion of the operands if they
> > have rank less than (unsigned) int, whereas C++ does not.
> >
> > In any case, as per the documentation of VEC_COND_EXPR,
> > "vec0 = vector-condition ? vec1 : vec2" is equivalent to
> > ``` (from tree.def)
> >   for (int i = 0 ; i < n ; ++i)
> > vec0[i] = vector-condtion[i] ? vec1[i] : vec2[i];
> > ```
> > But this is currently not the case, even in C++ where
> > ``` (Ex1)
> > typedef signed char vec2 __attribute__((vector_size(16)));
> > typedef float vec2f __attribute__((vector_size( 2 * sizeof (float;
> >
> > void j (vec2 *x, vec2 *z, vec2f *y, vec2f *t)
> > {
> >   *x = (*y < *t) ? '1' : '0'; // error: inferred scalar type ‘char’ is
> >   // not an integer or floating-point type
> >   // of the same size as ‘float’.
> >
> >   for (int i = 0 ; i < 2 ; ++i)  // fine
> > (*x)[i] = (*y)[i] < (*t)[i] ? '1' : '0'; //
> >
> >   *z = (*x < *z) ? '1' : '0'; // fine
> > }
> > ```
> >
> > The documentation explicitly says:
> > > the ternary operator ?: is 

Re: [PATCH v3] c++: parser - Support for target address spaces in C++

2022-10-18 Thread Paul Iannetta via Gcc-patches
On Fri, Oct 14, 2022 at 11:19:50AM -0400, Jason Merrill wrote:
> On 10/13/22 17:57, Paul Iannetta wrote:
> > On Thu, Oct 13, 2022 at 03:41:16PM -0400, Jason Merrill wrote:
> > > On 10/13/22 12:02, Paul Iannetta wrote:
> > > > On Thu, Oct 13, 2022 at 11:47:42AM -0400, Jason Merrill wrote:
> > > > > On 10/13/22 11:23, Paul Iannetta wrote:
> > > > > > On Thu, Oct 13, 2022 at 11:02:24AM -0400, Jason Merrill wrote:
> > > > > > > On 10/12/22 20:52, Paul Iannetta wrote:
> > > > > > > > There are quite a few things I would like to clarify concerning 
> > > > > > > > some
> > > > > > > > implementation details.
> > > > > > > >   - A variable with automatic storage (which is neither a 
> > > > > > > > pointer nor
> > > > > > > > a reference) cannot be qualified with an address space. 
> > > > > > > >  I detect
> > > > > > > > this by the combination of `sc_none' and `! 
> > > > > > > > toplevel_bindings_p ()',
> > > > > > > > but I've also seen the use of `at_function_scope' at 
> > > > > > > > other places.
> > > > > > > > And I'm unsure which one is appropriate here.
> > > > > > > > This detection happens at the very end of 
> > > > > > > > grokdeclarator because I
> > > > > > > > need to know that the type is a pointer, which is not 
> > > > > > > > know until
> > > > > > > > very late in the function.
> > > > > > > 
> > > > > > > At that point you have the decl, and you can ask directly what 
> > > > > > > its storage
> > > > > > > duration is, perhaps using decl_storage_duration.
> > > > > > > 
> > > > > > > But why do you need to know whether the type is a pointer?  The 
> > > > > > > attribute
> > > > > > > applies to the target type of the pointer, not the pointer type.  
> > > > > > > I think
> > > > > > > the problem is that you're looking at declspecs when you ought to 
> > > > > > > be looking
> > > > > > > at type_quals.
> > > > > > 
> > > > > > I need to know that the base type is a pointer to reject invalid
> > > > > > declarations such as:
> > > > > > 
> > > > > >int f (__seg_fs int a) { } or int f () { __seg_fs 
> > > > > > int a; }
> > > > > > 
> > > > > > because parameters and auto variables can have an address space
> > > > > > qualifier only if they are pointer or reference type, which I can't
> > > > > > tell only from type_quals.
> > > > > 
> > > > > But "int *__seg_fs a" is just as invalid as the above; the difference 
> > > > > is not
> > > > > whether a is a pointer, but whether the address-space-qualified is 
> > > > > the type
> > > > > of a itself or some sub-type.
> > > > 
> > > > I agree that "int * __seg_fs a" is invalid but it is accepted by the C
> > > > front-end, and by clang (both C and C++), the behavior is that the
> > > > address-name is silently ignored.
> > > 
> > > Hmm, that sounds like a bug; in that case, presumably the user meant to
> > > qualify the pointed-to type, and silently ignoring seems unlikely to give
> > > the effect they want.
> > > 
> > 
> > Well, actually, I'm re-reading the draft and "int * __seg_fs a" is
> > valid.  It means "pointer in address space __seg_fs pointing to an
> > object in the generic address space", whereas "__seg_fs int * a" means
> > "pointer in the generic address space pointing to an object in the
> > __seg_fs address-space".
> > 
> > Oddities such as, "__seg_fs int * __seg_gs a" are also perfectly
> > valid.
> 
> If a has static storage duration, sure; I was still thinking about
> declarations with automatic storage duration such as in your example above.
> 

Thanks, I only use type_quals now. I also took into account the style
recommendations from Jakub, and included the other template tests.
I rebased over trunk, bootstrapped the compiler and run the "make
check-gcc" with no regressions on x86.

Paul

#  >8 
Add support for custom address spaces in C++

gcc/
* tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.

gcc/c/
* c-decl.cc: Remove c_register_addr_space.

gcc/c-family/
* c-common.cc (c_register_addr_space): Imported from c-decl.cc
(addr_space_superset): Imported from gcc/c/c-typecheck.cc
* c-common.h: Remove the FIXME.
(addr_space_superset): New declaration.

gcc/cp/
* cp-tree.h (enum cp_decl_spec): Add addr_space support.
(struct cp_decl_specifier_seq): Likewise.
* decl.cc (get_type_quals): Likewise.
(check_tag_decl): Likewise.
(grokdeclarator): Likewise.
* parser.cc (cp_parser_type_specifier): Likewise.
(cp_parser_cv_qualifier_seq_opt): Likewise.
(cp_parser_postfix_expression): Likewise.
(cp_parser_type_specifier): Likewise.
(set_and_check_decl_spec_loc): Likewise.
* typeck.cc (composite_pointer_type): Likewise
(comp_ptr_ttypes_real): Likewise.
(same_type_ignoring_top_level_qualifiers_p): Likewise.
* pt.cc (check_cv_quals_for_unify): 

Re: Announcement: Porting the Docs to Sphinx - 9. November 2022

2022-10-17 Thread Paul Iannetta via Gcc-patches
Hi Martin,

Thank you very much for porting the documentation to Sphinx, it is
very convenient to use, especially the menu on the left and the
search bar.

However, I also regularly browse and search the documentation through
info, especially when I want to use regexps to search or need to
include a special character (eg.,+,-,_,(; this can happen when I
search for '(define' ) for example) in the search string.

Does the port to Sphinx means the end of texinfo? Or, will both be
available as it is the case now with the official texinfo and your
unofficial splichal.eu pages.

Paul

On Mon, Oct 17, 2022 at 03:28:34PM +0200, Martin Liška wrote:
> Hello.
> 
> Based on the very positive feedback I was given at the Cauldron Sphinx 
> Documentation BoF,
> I'm planning migrating the documentation on 9th November. There are still 
> some minor comments
> from Sandra when it comes to the PDF output, but we can address that once the 
> conversion is done.
> 
> The reason I'm sending the email now is that I was waiting for latest Sphinx 
> release (5.3.0) that
> simplifies reference format for options and results in much simpler Option 
> summary section ([1])
> 
> The current GCC master (using Sphinx 5.3.0) converted docs can be seen here:
> https://splichal.eu/scripts/sphinx/
> 
> If you see any issues with the converted documentation, or have a feedback 
> about it,
> please reply to this email.
> 
> Cheers,
> Martin
> 
> [1] https://github.com/sphinx-doc/sphinx/pull/10840
> [1] 
> https://splichal.eu/scripts/sphinx/gcc/_build/html/gcc-command-options/option-summary.html
> 
> 
> 
> 






Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does)

2022-10-14 Thread Paul Iannetta via Gcc-patches
On Wed, Oct 12, 2022 at 01:18:19AM +0200, Paul Iannetta wrote:
> On Mon, Oct 10, 2022 at 11:07:06PM +, Joseph Myers wrote:
> > On Mon, 10 Oct 2022, Paul Iannetta via Gcc-patches wrote:
> > 
> > > I have a patch to bring this feature to the C front-end as well, and
> > > would like to hear your opinion on it, especially since it may affect
> > > the feature-set of the objc front-end as well.
> > 
> > > Currently, this is only a tentative patch and I did not add any tests
> > > to the testsuite.
> > 
> > I think tests (possibly existing C++ tests moved to c-c++-common?) are 
> > necessary to judge such a feature; it could better be judged based on 
> > tests without implementation than based on implementation without tests.
> 
> Currently, this feature has the following tests in g++.dg/ext/
>   - vector9.C
>   - vector19.C
>   - vector21.C
>   - vector22.C
>   - vector23.C
>   - vector27.C
>   - vector28.C
> provided by Marc Glisse when he implemented the feature for C++.
> 
> They are all handled by my mirror implementation (after removing
> C++-only features), save for a case in vector19.C ( v ? '1' : '2',
> where v is a vector of unsigned char, but '1' and '2' are considered
> as int, which results in a type mismatch.)
> 
> I'll move those tests to c-c++-common tomorrow, but will duplicate
> vector19.C and vector23.C which rely on C++-only features.
> 
> During my tests, I've been using variations around this:
> 
> typedef int v2si __attribute__((__vector_size__ (2 * sizeof(int;
> 
> v2si f (v2si a, v2si b, v2si c)
> {
>   v2si d = a + !b;
>   v2si e = a || b;
>   return c ? (a + !b) && (c - e && a) : (!!b ^ c && e);
> }
> 
> It is already possible to express much of the same thing without the
> syntactic sugar but is is barely legible
> 
> typedef int v2si __attribute__((__vector_size__ (2 * sizeof(int;
> 
> v2si f (v2si a, v2si b, v2si c)
> {
>   v2si d = a + (b == 0);
>   v2si e = (a != 0) | (b != 0);
>   return ((c != 0) & (((a + (b == 0)) != 0) & (((c - e) != 0) & (a != 0
>| ((c == 0) & (b == 0) == 0) ^ c) != 0) & (e != 0)));
> }
> 
> Paul

I still need to check what is done by clang on the objc side, but in
order to not conflict with what was done before, a warning is
triggered by c_obj_common_truthvalue_conversion and
build_unary_operator warns if '!' is used with a vector.  Both warnings
are only triggered in pedantic mode as suggested by Iain Sandoe.

The support of the binary ops and unary ops works as the C++ front-end
does, there is however the case of the ternary conditional operator,
where the C standard mandates the promotion of the operands if they
have rank less than (unsigned) int, whereas C++ does not.

In any case, as per the documentation of VEC_COND_EXPR,
"vec0 = vector-condition ? vec1 : vec2" is equivalent to
``` (from tree.def)
  for (int i = 0 ; i < n ; ++i)
vec0[i] = vector-condtion[i] ? vec1[i] : vec2[i];
```
But this is currently not the case, even in C++ where
``` (Ex1)
typedef signed char vec2 __attribute__((vector_size(16)));
typedef float vec2f __attribute__((vector_size( 2 * sizeof (float;

void j (vec2 *x, vec2 *z, vec2f *y, vec2f *t)
{
  *x = (*y < *t) ? '1' : '0'; // error: inferred scalar type ‘char’ is
  // not an integer or floating-point type
  // of the same size as ‘float’.

  for (int i = 0 ; i < 2 ; ++i)  // fine
(*x)[i] = (*y)[i] < (*t)[i] ? '1' : '0'; //

  *z = (*x < *z) ? '1' : '0'; // fine
}
```

The documentation explicitly says:
> the ternary operator ?: is available. a?b:c, where b and c are
> vectors of the same type and a is an integer vector with the same
> number of elements of the same size as b and c, computes all three
> arguments and creates a vector {a[0]?b[0]:c[0], a[1]?b[1]:c[1], …}
Here, "*y < *t" is a boolean vector (and bool is an integral type
([basic.fundamental] 11), so this should be accepted.

An other point is that if we look at
```
  for (int i = 0 ; i < n ; ++i)
vec0[i] = vector-condtion[i] ? vec1[i] : vec2[i];
```
implicit conversions may happen, which is completely over-looked
currently.  That is, the type of (1): "v = v0 ? v1 : v2" is the lowest
common type of v, v1 and v2; and the type of (2): "v0 ? v1 : v2" is the
lowest common type of v1 and v2.  (2) can appear as a parameter, but
even in that case, I think that (2) should be constrained by the type
of the parameter and we are back to case (1).

My points are that:
  - the current implementation has a bug: " *x = (*y < *t) ? '1' :
'0';" from (Ex1) should be fine.
  - the current implementation does not explicetly follow the
documented behavior of VEC_COND_EXPR.

What do you think?

Paul






Re: [PATCH v2] c++: parser - Support for target address spaces in C++

2022-10-13 Thread Paul Iannetta via Gcc-patches
On Thu, Oct 13, 2022 at 03:41:16PM -0400, Jason Merrill wrote:
> On 10/13/22 12:02, Paul Iannetta wrote:
> > On Thu, Oct 13, 2022 at 11:47:42AM -0400, Jason Merrill wrote:
> > > On 10/13/22 11:23, Paul Iannetta wrote:
> > > > On Thu, Oct 13, 2022 at 11:02:24AM -0400, Jason Merrill wrote:
> > > > > On 10/12/22 20:52, Paul Iannetta wrote:
> > > > > > On Tue, Oct 11, 2022 at 09:49:43PM -0400, Jason Merrill wrote:
> > > > > > > 
> > > > > > > It surprises that this is the only place we complain about an 
> > > > > > > object with an
> > > > > > > address-space qualifier.  Shouldn't we also complain about e.g. 
> > > > > > > automatic
> > > > > > > variables/parameters or non-static data members with 
> > > > > > > address-space qualified
> > > > > > > type?
> > > > > > > 
> > > > > > 
> > > > > > Indeed, I was missing quite a few things here.  Thanks.
> > > > > > I used the draft as basis this time and imported from the C
> > > > > > implementation the relevant parts.  This time, the errors get 
> > > > > > properly
> > > > > > emitted when an address space is unduly specified; and comparisons,
> > > > > > assignments and comparisons are taken care of.
> > > > > > 
> > > > > > There are quite a few things I would like to clarify concerning some
> > > > > > implementation details.
> > > > > >  - A variable with automatic storage (which is neither a 
> > > > > > pointer nor
> > > > > >a reference) cannot be qualified with an address space.  I 
> > > > > > detect
> > > > > >this by the combination of `sc_none' and `! 
> > > > > > toplevel_bindings_p ()',
> > > > > >but I've also seen the use of `at_function_scope' at other 
> > > > > > places.
> > > > > >And I'm unsure which one is appropriate here.
> > > > > >This detection happens at the very end of grokdeclarator 
> > > > > > because I
> > > > > >need to know that the type is a pointer, which is not know 
> > > > > > until
> > > > > >very late in the function.
> > > > > 
> > > > > At that point you have the decl, and you can ask directly what its 
> > > > > storage
> > > > > duration is, perhaps using decl_storage_duration.
> > > > > 
> > > > > But why do you need to know whether the type is a pointer?  The 
> > > > > attribute
> > > > > applies to the target type of the pointer, not the pointer type.  I 
> > > > > think
> > > > > the problem is that you're looking at declspecs when you ought to be 
> > > > > looking
> > > > > at type_quals.
> > > > 
> > > > I need to know that the base type is a pointer to reject invalid
> > > > declarations such as:
> > > > 
> > > >   int f (__seg_fs int a) { } or int f () { __seg_fs int a; }
> > > > 
> > > > because parameters and auto variables can have an address space
> > > > qualifier only if they are pointer or reference type, which I can't
> > > > tell only from type_quals.
> > > 
> > > But "int *__seg_fs a" is just as invalid as the above; the difference is 
> > > not
> > > whether a is a pointer, but whether the address-space-qualified is the 
> > > type
> > > of a itself or some sub-type.
> > 
> > I agree that "int * __seg_fs a" is invalid but it is accepted by the C
> > front-end, and by clang (both C and C++), the behavior is that the
> > address-name is silently ignored.
> 
> Hmm, that sounds like a bug; in that case, presumably the user meant to
> qualify the pointed-to type, and silently ignoring seems unlikely to give
> the effect they want.
> 

Well, actually, I'm re-reading the draft and "int * __seg_fs a" is
valid.  It means "pointer in address space __seg_fs pointing to an
object in the generic address space", whereas "__seg_fs int * a" means
"pointer in the generic address space pointing to an object in the
__seg_fs address-space".

Oddities such as, "__seg_fs int * __seg_gs a" are also perfectly
valid.

The reason why I wrongly assumed that the address space was silently
ignored is that I made a simple test which only relied on the mangled
function name...

```
int * __seg_fs a;
template  int f (T *a) { return *a; }
int main () { return f (a); } // f(int*), since a is in __seg_fs
  // but the pointer is to the generic
  // address-space.
```

Implementation-wise, I think I handle that correctly but I'll do a recheck.

> > > You need to look at the qualifiers on type (which should also be the ones 
> > > in
> > > type_quals), not the qualifiers in the declspecs.
> > 
> > I'll have another look, thanks.
> > 
> > > > > >  - I'm having some trouble deciding whether I include those 
> > > > > > three
> > > > > >stub programs as tests, they all compile fine and clang 
> > > > > > accepts
> > > > > >them as well.
> > > > > 
> > > > > Why not?
> > > > 
> > > > I thought they were pretty contrived, since it does not make much
> > > > sense to strip address space qualifiers, even though it does prove
> > > > that the implementation support those contrived but valid 

Re: [PATCH v2] c++: parser - Support for target address spaces in C++

2022-10-13 Thread Paul Iannetta via Gcc-patches
On Thu, Oct 13, 2022 at 11:47:42AM -0400, Jason Merrill wrote:
> On 10/13/22 11:23, Paul Iannetta wrote:
> > On Thu, Oct 13, 2022 at 11:02:24AM -0400, Jason Merrill wrote:
> > > On 10/12/22 20:52, Paul Iannetta wrote:
> > > > On Tue, Oct 11, 2022 at 09:49:43PM -0400, Jason Merrill wrote:
> > > > > 
> > > > > It surprises that this is the only place we complain about an object 
> > > > > with an
> > > > > address-space qualifier.  Shouldn't we also complain about e.g. 
> > > > > automatic
> > > > > variables/parameters or non-static data members with address-space 
> > > > > qualified
> > > > > type?
> > > > > 
> > > > 
> > > > Indeed, I was missing quite a few things here.  Thanks.
> > > > I used the draft as basis this time and imported from the C
> > > > implementation the relevant parts.  This time, the errors get properly
> > > > emitted when an address space is unduly specified; and comparisons,
> > > > assignments and comparisons are taken care of.
> > > > 
> > > > There are quite a few things I would like to clarify concerning some
> > > > implementation details.
> > > > - A variable with automatic storage (which is neither a pointer nor
> > > >   a reference) cannot be qualified with an address space.  I detect
> > > >   this by the combination of `sc_none' and `! toplevel_bindings_p 
> > > > ()',
> > > >   but I've also seen the use of `at_function_scope' at other places.
> > > >   And I'm unsure which one is appropriate here.
> > > >   This detection happens at the very end of grokdeclarator because I
> > > >   need to know that the type is a pointer, which is not know until
> > > >   very late in the function.
> > > 
> > > At that point you have the decl, and you can ask directly what its storage
> > > duration is, perhaps using decl_storage_duration.
> > > 
> > > But why do you need to know whether the type is a pointer?  The attribute
> > > applies to the target type of the pointer, not the pointer type.  I think
> > > the problem is that you're looking at declspecs when you ought to be 
> > > looking
> > > at type_quals.
> > 
> > I need to know that the base type is a pointer to reject invalid
> > declarations such as:
> > 
> >  int f (__seg_fs int a) { } or int f () { __seg_fs int a; }
> > 
> > because parameters and auto variables can have an address space
> > qualifier only if they are pointer or reference type, which I can't
> > tell only from type_quals.
> 
> But "int *__seg_fs a" is just as invalid as the above; the difference is not
> whether a is a pointer, but whether the address-space-qualified is the type
> of a itself or some sub-type.

I agree that "int * __seg_fs a" is invalid but it is accepted by the C
front-end, and by clang (both C and C++), the behavior is that the
address-name is silently ignored.

> You need to look at the qualifiers on type (which should also be the ones in
> type_quals), not the qualifiers in the declspecs.

I'll have another look, thanks.

> > > > - I'm having some trouble deciding whether I include those three
> > > >   stub programs as tests, they all compile fine and clang accepts
> > > >   them as well.
> > > 
> > > Why not?
> > 
> > I thought they were pretty contrived, since it does not make much
> > sense to strip address space qualifiers, even though it does prove
> > that the implementation support those contrived but valid uses.
> 
> Testcases are full of contrived examples testing corner cases.  :)
> 
> > > 
> > > > Ex1:
> > > > ```
> > > > int __seg_fs * fs1;
> > > > int __seg_gs * gs1;
> > > > 
> > > > template struct strip;
> > > > template struct strip<__seg_fs T *> { typedef T type; };
> > > > template struct strip<__seg_gs T *> { typedef T type; };
> > > > 
> > > > int
> > > > main ()
> > > > {
> > > >   *(strip::type *) fs1 == 
> > > > *(strip::type *) gs1;
> > > >   return 0;
> > > > }
> > > > ```
> > > > 
> > > > Ex2:
> > > > ```
> > > > int __seg_fs * fs1;
> > > > int __seg_fs * fs2;
> > > > 
> > > > template auto f (T __seg_fs * a, U __seg_gs * 
> > > > b) { return a; }
> > > > template auto f (T __seg_gs * a, U __seg_fs * 
> > > > b) { return a; }
> > > > 
> > > > int
> > > > main ()
> > > > {
> > > >   f (fs1, gs1);
> > > >   f (gs1, fs1);
> > > >   return 0;
> > > > }
> > > > ```
> > > > 
> > > > Ex3:
> > > > ```
> > > > int __seg_fs * fs1;
> > > > int __seg_gs * gs1;
> > > > 
> > > > template
> > > > auto f (T __seg_fs * a, U __seg_gs * b)
> > > > {
> > > >   return *(T *) a == *(U *) b;
> > > > }
> > > > 
> > > > int
> > > > main ()
> > > > {
> > > >   return f (fs1, gs1);
> > > > }
> > > > ```
> > > > 
> > > > 
> > > > Add support for custom address spaces in C++
> > > > 
> > > > gcc/
> > > >   * tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.
> > > > 
> > > > gcc/c/
> > > >   * c-decl.cc: Remove c_register_addr_space.
> > > > 
> > > > gcc/c-family/
> > > >   * c-common.cc (c_register_addr_space): Imported from 

Re: [PATCH v2] c++: parser - Support for target address spaces in C++

2022-10-13 Thread Paul Iannetta via Gcc-patches
On Thu, Oct 13, 2022 at 11:02:24AM -0400, Jason Merrill wrote:
> On 10/12/22 20:52, Paul Iannetta wrote:
> > On Tue, Oct 11, 2022 at 09:49:43PM -0400, Jason Merrill wrote:
> > > 
> > > It surprises that this is the only place we complain about an object with 
> > > an
> > > address-space qualifier.  Shouldn't we also complain about e.g. automatic
> > > variables/parameters or non-static data members with address-space 
> > > qualified
> > > type?
> > > 
> > 
> > Indeed, I was missing quite a few things here.  Thanks.
> > I used the draft as basis this time and imported from the C
> > implementation the relevant parts.  This time, the errors get properly
> > emitted when an address space is unduly specified; and comparisons,
> > assignments and comparisons are taken care of.
> > 
> > There are quite a few things I would like to clarify concerning some
> > implementation details.
> >- A variable with automatic storage (which is neither a pointer nor
> >  a reference) cannot be qualified with an address space.  I detect
> >  this by the combination of `sc_none' and `! toplevel_bindings_p ()',
> >  but I've also seen the use of `at_function_scope' at other places.
> >  And I'm unsure which one is appropriate here.
> >  This detection happens at the very end of grokdeclarator because I
> >  need to know that the type is a pointer, which is not know until
> >  very late in the function.
> 
> At that point you have the decl, and you can ask directly what its storage
> duration is, perhaps using decl_storage_duration.
> 
> But why do you need to know whether the type is a pointer?  The attribute
> applies to the target type of the pointer, not the pointer type.  I think
> the problem is that you're looking at declspecs when you ought to be looking
> at type_quals.
> 

I need to know that the base type is a pointer to reject invalid
declarations such as:

int f (__seg_fs int a) { } or int f () { __seg_fs int a; }

because parameters and auto variables can have an address space
qualifier only if they are pointer or reference type, which I can't
tell only from type_quals.

> >- I'm having some trouble deciding whether I include those three
> >  stub programs as tests, they all compile fine and clang accepts
> >  them as well.
> 
> Why not?

I thought they were pretty contrived, since it does not make much
sense to strip address space qualifiers, even though it does prove
that the implementation support those contrived but valid uses.

> 
> > Ex1:
> > ```
> > int __seg_fs * fs1;
> > int __seg_gs * gs1;
> > 
> > template struct strip;
> > template struct strip<__seg_fs T *> { typedef T type; };
> > template struct strip<__seg_gs T *> { typedef T type; };
> > 
> > int
> > main ()
> > {
> >  *(strip::type *) fs1 == *(strip::type *) 
> > gs1;
> >  return 0;
> > }
> > ```
> > 
> > Ex2:
> > ```
> > int __seg_fs * fs1;
> > int __seg_fs * fs2;
> > 
> > template auto f (T __seg_fs * a, U __seg_gs * b) { 
> > return a; }
> > template auto f (T __seg_gs * a, U __seg_fs * b) { 
> > return a; }
> > 
> > int
> > main ()
> > {
> >  f (fs1, gs1);
> >  f (gs1, fs1);
> >  return 0;
> > }
> > ```
> > 
> > Ex3:
> > ```
> > int __seg_fs * fs1;
> > int __seg_gs * gs1;
> > 
> > template
> > auto f (T __seg_fs * a, U __seg_gs * b)
> > {
> >  return *(T *) a == *(U *) b;
> > }
> > 
> > int
> > main ()
> > {
> >  return f (fs1, gs1);
> > }
> > ```
> > 
> > 
> > Add support for custom address spaces in C++
> > 
> > gcc/
> >  * tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.
> > 
> > gcc/c/
> >  * c-decl.cc: Remove c_register_addr_space.
> > 
> > gcc/c-family/
> >  * c-common.cc (c_register_addr_space): Imported from c-decl.cc
> >  (addr_space_superset): Imported from gcc/c/c-typecheck.cc
> >  * c-common.h: Remove the FIXME.
> >  (addr_space_superset): New declaration.
> > 
> > gcc/cp/
> >  * cp-tree.h (enum cp_decl_spec): Add addr_space support.
> >  (struct cp_decl_specifier_seq): Likewise.
> >  * decl.cc (get_type_quals): Likewise.
> >  (check_tag_decl): Likewise.
> > (grokdeclarator): Likewise.
> >  * parser.cc (cp_parser_type_specifier): Likewise.
> >  (cp_parser_cv_qualifier_seq_opt): Likewise.
> >  (cp_parser_postfix_expression): Likewise.
> >  (cp_parser_type_specifier): Likewise.
> >  (set_and_check_decl_spec_loc): Likewise.
> >  * typeck.cc (composite_pointer_type): Likewise
> >  (comp_ptr_ttypes_real): Likewise.
> > (same_type_ignoring_top_level_qualifiers_p): Likewise.
> >  * pt.cc (check_cv_quals_for_unify): Likewise.
> >  (unify): Likewise.
> >  * tree.cc: Remove c_register_addr_space stub.
> >  * mangle.cc (write_CV_qualifiers_for_type): Mangle address spaces
> >using the extended qualifier notation.
> > 
> > gcc/doc
> >  * extend.texi (Named 

Re: [PATCH v2] c++: parser - Support for target address spaces in C++

2022-10-13 Thread Paul Iannetta via Gcc-patches
On Thu, Oct 13, 2022 at 07:46:46AM +0200, Jakub Jelinek wrote:
> On Thu, Oct 13, 2022 at 02:52:59AM +0200, Paul Iannetta via Gcc-patches wrote:
> > +   if (type != error_mark_node
> > +   && !ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (type))
> > +   && current_function_decl)
> > + {
> > +   error
> > + ("compound literal qualified by address-space qualifier");
> > +   type = error_mark_node;
> 
> Can you please write this as:
>   error ("compound literal qualified by address-space "
>  "qualifier");
> ?  That is how diagnostics that don't fit on one line are usually written.
> 
> > @@ -23812,6 +23830,11 @@ cp_parser_cv_qualifier_seq_opt (cp_parser* parser)
> >   break;
> > }
> >  
> > +  if (RID_FIRST_ADDR_SPACE <= token->keyword &&
> 
> && should never go at the end of line.
> 
> > + token->keyword <= RID_LAST_ADDR_SPACE)
> > +   cv_qualifier =
> 
> and similarly = (except for aggregate initializers).
> 
> > + ENCODE_QUAL_ADDR_SPACE (token->keyword - RID_FIRST_ADDR_SPACE);
> 
> So:
> 
>   if (RID_FIRST_ADDR_SPACE <= token->keyword
> && token->keyword <= RID_LAST_ADDR_SPACE)
>   cv_qualifier
> = ENCODE_QUAL_ADDR_SPACE (token->keyword - RID_FIRST_ADDR_SPACE);
> 
> > + int unified_cv =
> > +   CLEAR_QUAL_ADDR_SPACE (arg_cv_quals & ~parm_cv_quals)
> > +   | ENCODE_QUAL_ADDR_SPACE (as_common);
> 
> Similarly (but this time with ()s added to ensure correct formatting in
> some editors).
> 
> int unified_cv
>   = (CLEAR_QUAL_ADDR_SPACE (arg_cv_quals & ~parm_cv_quals)
>  | ENCODE_QUAL_ADDR_SPACE (as_common));
> 
> >result_type
> > = cp_build_qualified_type (void_type_node,
> > -  (cp_type_quals (TREE_TYPE (t1))
> > -   | cp_type_quals (TREE_TYPE (t2;
> > +  (CLEAR_QUAL_ADDR_SPACE (cp_type_quals 
> > (TREE_TYPE (t1)))
> > +   | CLEAR_QUAL_ADDR_SPACE (cp_type_quals 
> > (TREE_TYPE (t2)))
> 
> The above 2 lines are way too long.
> I'd suggest to use temporaries, say
> int quals1 = cp_type_quals (TREE_TYPE (t1));
> int quals2 = cp_type_quals (TREE_TYPE (t2));
> and use those.
> 
>   Jakub

Thank you for the style review, I'll apply them in the next iteration.

Paul






Re: [PATCH v2] c++: parser - Support for target address spaces in C++

2022-10-12 Thread Paul Iannetta via Gcc-patches
On Tue, Oct 11, 2022 at 09:49:43PM -0400, Jason Merrill wrote:
> 
> It surprises that this is the only place we complain about an object with an
> address-space qualifier.  Shouldn't we also complain about e.g. automatic
> variables/parameters or non-static data members with address-space qualified
> type?
> 

Indeed, I was missing quite a few things here.  Thanks.
I used the draft as basis this time and imported from the C
implementation the relevant parts.  This time, the errors get properly
emitted when an address space is unduly specified; and comparisons,
assignments and comparisons are taken care of.

There are quite a few things I would like to clarify concerning some
implementation details.
  - A variable with automatic storage (which is neither a pointer nor
a reference) cannot be qualified with an address space.  I detect
this by the combination of `sc_none' and `! toplevel_bindings_p ()',
but I've also seen the use of `at_function_scope' at other places.
And I'm unsure which one is appropriate here.
This detection happens at the very end of grokdeclarator because I
need to know that the type is a pointer, which is not know until
very late in the function.
  - I'm having some trouble deciding whether I include those three
stub programs as tests, they all compile fine and clang accepts
them as well.

Ex1:
```
int __seg_fs * fs1;
int __seg_gs * gs1;

template struct strip;
template struct strip<__seg_fs T *> { typedef T type; };
template struct strip<__seg_gs T *> { typedef T type; };

int
main ()
{
*(strip::type *) fs1 == *(strip::type *) gs1;
return 0;
}
```

Ex2:
```
int __seg_fs * fs1;
int __seg_fs * fs2;

template auto f (T __seg_fs * a, U __seg_gs * b) { 
return a; }
template auto f (T __seg_gs * a, U __seg_fs * b) { 
return a; }

int
main ()
{
f (fs1, gs1);
f (gs1, fs1);
return 0;
}
```

Ex3:
```
int __seg_fs * fs1;
int __seg_gs * gs1;

template
auto f (T __seg_fs * a, U __seg_gs * b)
{
return *(T *) a == *(U *) b;
}

int
main ()
{
return f (fs1, gs1);
}
```


Add support for custom address spaces in C++

gcc/
* tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.

gcc/c/
* c-decl.cc: Remove c_register_addr_space.

gcc/c-family/
* c-common.cc (c_register_addr_space): Imported from c-decl.cc
(addr_space_superset): Imported from gcc/c/c-typecheck.cc
* c-common.h: Remove the FIXME.
(addr_space_superset): New declaration.

gcc/cp/
* cp-tree.h (enum cp_decl_spec): Add addr_space support.
(struct cp_decl_specifier_seq): Likewise.
* decl.cc (get_type_quals): Likewise.
(check_tag_decl): Likewise.
(grokdeclarator): Likewise.
* parser.cc (cp_parser_type_specifier): Likewise.
(cp_parser_cv_qualifier_seq_opt): Likewise.
(cp_parser_postfix_expression): Likewise.
(cp_parser_type_specifier): Likewise.
(set_and_check_decl_spec_loc): Likewise.
* typeck.cc (composite_pointer_type): Likewise
(comp_ptr_ttypes_real): Likewise.
(same_type_ignoring_top_level_qualifiers_p): Likewise.
* pt.cc (check_cv_quals_for_unify): Likewise.
(unify): Likewise.
* tree.cc: Remove c_register_addr_space stub.
* mangle.cc (write_CV_qualifiers_for_type): Mangle address spaces
  using the extended qualifier notation.

gcc/doc
* extend.texi (Named Address Spaces): add a mention about C++
  support.

gcc/testsuite/
* g++.dg/abi/mangle-addr-space1.C: New test.
* g++.dg/abi/mangle-addr-space2.C: New test.
* g++.dg/parse/addr-space.C: New test.
* g++.dg/parse/addr-space1.C: New test.
* g++.dg/parse/addr-space2.C: New test.
* g++.dg/parse/template/spec-addr-space.C: New test.
* g++.dg/ext/addr-space-decl.C: New test.
* g++.dg/ext/addr-space-ref.C: New test.
* g++.dg/ext/addr-space-ops.C: New test.

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Date:  Sun Oct 9 16:02:22 2022 +0200
#
# On branch releases/gcc-12
# Your branch is ahead of 'origin/releases/gcc-12' by 2 commits.
#   (use "git push" to publish your local commits)
#
# Changes to be committed:
#   modified:   gcc/c-family/c-common.cc
#   modified:   gcc/c-family/c-common.h
#   modified:   gcc/c/c-decl.cc
#   modified:   gcc/c/c-typeck.cc
#   modified:   gcc/cp/cp-tree.h
#   modified:   gcc/cp/decl.cc
#   modified:   gcc/cp/mangle.cc
#   modified:   gcc/cp/parser.cc
#   modified:   gcc/cp/pt.cc
#   modified:   gcc/cp/tree.cc
#   modified:   gcc/cp/typeck.cc
#   modified:   gcc/doc/extend.texi
#   new file:   gcc/testsuite/g++.dg/abi/mangle-addr-space1.C
#   new file:   gcc/testsuite/g++.dg/abi/mangle-addr-space2.C
#   new file:   gcc/testsuite/g++.dg/ext/addr-space-decl.C
#   new file:  

Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does)

2022-10-11 Thread Paul Iannetta via Gcc-patches
On Mon, Oct 10, 2022 at 11:07:06PM +, Joseph Myers wrote:
> On Mon, 10 Oct 2022, Paul Iannetta via Gcc-patches wrote:
> 
> > I have a patch to bring this feature to the C front-end as well, and
> > would like to hear your opinion on it, especially since it may affect
> > the feature-set of the objc front-end as well.
> 
> > Currently, this is only a tentative patch and I did not add any tests
> > to the testsuite.
> 
> I think tests (possibly existing C++ tests moved to c-c++-common?) are 
> necessary to judge such a feature; it could better be judged based on 
> tests without implementation than based on implementation without tests.

Currently, this feature has the following tests in g++.dg/ext/
  - vector9.C
  - vector19.C
  - vector21.C
  - vector22.C
  - vector23.C
  - vector27.C
  - vector28.C
provided by Marc Glisse when he implemented the feature for C++.

They are all handled by my mirror implementation (after removing
C++-only features), save for a case in vector19.C ( v ? '1' : '2',
where v is a vector of unsigned char, but '1' and '2' are considered
as int, which results in a type mismatch.)

I'll move those tests to c-c++-common tomorrow, but will duplicate
vector19.C and vector23.C which rely on C++-only features.

During my tests, I've been using variations around this:

typedef int v2si __attribute__((__vector_size__ (2 * sizeof(int;

v2si f (v2si a, v2si b, v2si c)
{
  v2si d = a + !b;
  v2si e = a || b;
  return c ? (a + !b) && (c - e && a) : (!!b ^ c && e);
}

It is already possible to express much of the same thing without the
syntactic sugar but is is barely legible

typedef int v2si __attribute__((__vector_size__ (2 * sizeof(int;

v2si f (v2si a, v2si b, v2si c)
{
  v2si d = a + (b == 0);
  v2si e = (a != 0) | (b != 0);
  return ((c != 0) & (((a + (b == 0)) != 0) & (((c - e) != 0) & (a != 0
   | ((c == 0) & (b == 0) == 0) ^ c) != 0) & (e != 0)));
}

Paul






Re: [PATCH v2] c++: parser - Support for target address spaces in C++

2022-10-11 Thread Paul Iannetta via Gcc-patches
Thank you very much for the comments.

On Mon, Oct 10, 2022 at 03:20:13PM -0400, Jason Merrill wrote:
> On 10/9/22 12:12, Paul Iannetta wrote:
> > That's a nice feature! LLVM is using AS to mangle the
> > address-name qualified types but that would have required an update of
> > libiberty's demangler in the binutils as well.
> 
> And they haven't proposed this mangling to
> 
>   https://github.com/itanium-cxx-abi/cxx-abi/
> 
> yet, either.

When looking at clang/lib/AST/{Microsoft,Itamium}Mangle.cpp, the
comments may suggest that they wanted to implement them as extended
qualifiers prefixed by an `U' but that's not what they ended up doing.

> You certainly want some template tests, say
> 
> template 
> int f (T *p) { return *p; }
> __seg_fs int *a;
> int main() { f(a); }
> // test for mangling of f<__seg_fs int>
> 
> -
> 
> template 
> int f (T __seg_gs *p) { return *p; }
> __seg_fs int *a;
> int main() { f(a); } // error, conflicting address spaces

Indeed, I was getting the first one right by a stroke of luck but not
the second.  I've consequently adapted the part which checks and
computes the unification of cv-qualifiers in the presence of address
spaces.  The idea being that a type parameter T can match any address
spaces but an address-space qualified parameter can't accept more
general address spaces than itself.

> > +/* Return true if between two named address spaces, whether there is a 
> > superset
> > +   named address space that encompasses both address spaces.  If there is a
> > +   superset, return which address space is the superset.  */
> > +
> > +bool
> > +addr_space_superset (addr_space_t as1, addr_space_t as2,
> > +addr_space_t * common)
> > +{
> > +  if (as1 == as2)
> > +{
> > +  *common = as1;
> > +  return true;
> > +}
> > +  else if (targetm.addr_space.subset_p (as1, as2))
> > +{
> > +  *common = as2;
> > +  return true;
> > +}
> > +  else if (targetm.addr_space.subset_p (as2, as1))
> > +{
> > +  *common = as1;
> > +  return true;
> > +}
> > +  else
> > +return false;
> 
> So it's not possible for the two spaces to both be subsets of a third?
> 

According to the [N1275, page 38]:
Address spaces may overlap in a nested fashion. For any two address
spaces, either the address spaces must be disjoint, they must be
equivalent, or one must be a subset of the other. [...] There is no
requirement that named address spaces (intrinsic or otherwise) be
subsets of the generic address space.
[N1275]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1275.pdf

Hence, two disjoint address spaces can't be subsets of a third, per
the draft.

> 
> New non-bit-fields should be added before the bit-fields.
> 

Done.

> > diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
> > index eb53e0ebeb4..06625ad9a4b 100644
> > --- a/gcc/cp/mangle.cc
> > +++ b/gcc/cp/mangle.cc
> > @@ -2509,6 +2509,15 @@ write_CV_qualifiers_for_type (const tree type)
> >array.  */
> > cp_cv_quals quals = TYPE_QUALS (type);
> > +  if (DECODE_QUAL_ADDR_SPACE (quals))
> > +{
> > +  addr_space_t as = DECODE_QUAL_ADDR_SPACE (quals);
> 
> This can be
> 
> if (addr_space_t as = DECODE_QUAL_ADDR_SPACE (quals))
> 
> so you don't need to repeat it.

I thought this was c++17 only, but in fact c++17 only broadened the
idiom.  Nice! (It would be even nicer to have this feature in C as
well)
> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > index 763df6f479b..110ceafc98b 100644
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > [...]
> > @@ -23812,6 +23830,13 @@ cp_parser_cv_qualifier_seq_opt (cp_parser* parser)
> >   break;
> > }
> > +  if (RID_FIRST_ADDR_SPACE <= token->keyword &&
> > + token->keyword <= RID_LAST_ADDR_SPACE)
> > +   {
> > + cv_qualifier =
> > +   ENCODE_QUAL_ADDR_SPACE (token->keyword - RID_FIRST_ADDR_SPACE);
> > +   }
> 
> We usually omit braces around a single statement.
> 
Done.

#  >8 
Add support for custom address spaces in C++

gcc/
* tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.

gcc/c/
* c-decl.cc: Remove c_register_addr_space.

gcc/c-family/
* c-common.cc (c_register_addr_space): Imported from c-decl.cc
(addr_space_superset): Imported from gcc/c/c-typecheck.cc
* c-common.h: Remove the FIXME.
(addr_space_superset): New declaration.

gcc/cp/
* cp-tree.h (enum cp_decl_spec): Add addr_space support.
(struct cp_decl_specifier_seq): Likewise.
* decl.cc (get_type_quals): Likewise.
(check_tag_decl): Likewise.
* parser.cc (cp_parser_type_specifier): Likewise.
(cp_parser_cv_qualifier_seq_opt): Likewise.
(cp_parser_postfix_expression): Likewise.
(cp_parser_type_specifier): Likewise.
(set_and_check_decl_spec_loc): Likewise.
* typeck.cc (composite_pointer_type): Likewise
(comp_ptr_ttypes_real): Likewise.
* 

Re: [RFC] Add support for vectors in comparisons (like the C++ frontend does)

2022-10-10 Thread Paul Iannetta via Gcc-patches
Hi Ian,

Observations are also very welcomed, thank you!

On Mon, Oct 10, 2022 at 03:37:24PM +0100, Iain Sandoe wrote:
> Hi Paul,
> 
> Not a review of the patch - but a couple of observations.
> 
> > On 10 Oct 2022, at 15:11, Paul Iannetta via Gcc-patches 
> >  wrote:
> 
> > I am trying to bridge the gap between the extensions supported by the C
> > and C++ front-ends.  When it comes to vector extensions, C++ supports
> > vectors in comparisons and within conditional expressions whereas the C
> > front-end does not.
> 
> Equivalence seems, on the face of it, a reasonable objective - but I am
> curious as whether there is some more concrete motivation for the patch,
> e.g. some codebase that currently does not work but would with this change?
> 

The main motivation behind the equivalence is that, we have C and C++
codebases, and it is not very convenient to have to remember which
extension is allowed in C and not in C++ and vice-versa.  And, in this
case, it makes it harder for GCC to generate conditional moves for
code using vectors, especially since `a ? b : c` is not recognized,
and thus, we cannot rely on the `VEC_COND_EXPR` construction.

> > I have a patch to bring this feature to the C front-end as well, and
> > would like to hear your opinion on it, especially since it may affect
> > the feature-set of the objc front-end as well.
> 
> Likewise, I am interested in the motivation for the ObjC change.  The usual
> initial filter for consideration of functional changes (at least for the NeXT
> runtime on Darwin) is “What does current clang do?” or, even better, “what
> does current Xcode do?”.   There are several (possibly many) cases where
> C in clang has extensions to C++ behaviour - and vice-versa (because there
> is a single  front-end codebase, that happens easily, whether by design o
>  accident).

Well, the only motivation was that it was easier to make the change
for both front-ends at the same time and avoided some checks.  I'll
add them so that it won't introduce a divergence on what is accepted
by the objc front-end.

Thanks,
Paul

> 
> The reason for the ‘clang litmus test’ is that the effective language standard
> for ObjectiveC is determined by that compiler.
> 
> cheers
> Iain
> 
> > I have tried to mirror as much as possible what the C++ front-end does
> > and checked that both front-end produce the same GIMPLE for all the
> > simple expressions as well as some more complex combinations of the
> > operators (?:, !, ^, || and &&).
> > 
> > Currently, this is only a tentative patch and I did not add any tests
> > to the testsuite.  Moreover, the aarch64's target-specific testsuite
> > explicitly tests the non-presence of this feature, which will have to
> > be removed.
> > 
> > I've run the testsuite on x86 and I've not seen any regressions.
> > 
> > Cheers,
> > Paul
> > 
> > #  >8 
> > Support for vector types in simple comparisons
> > 
> > gcc/
> > 
> > * doc/extend.texi: Remove the C++ mention, since both C and C++
> >   support the all the mentioned features.
> > 
> > gcc/c/
> > 
> > * c-typeck.cc (build_unary_op): Add support for vector for the
> >   unary exclamation mark.
> > (build_conditional_expr): Add support for vector in conditional
> > expressions.
> > (build_binary_op): Add support for vector for &&, || and ^.
> > (c_objc_common_truthvalue_conversion): Remove the special gards
> > preventing vector types.
> > 
> > #  >8 
> > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> > index 17185fd3da4..03ade14cae9 100644
> > --- a/gcc/c/c-typeck.cc
> > +++ b/gcc/c/c-typeck.cc
> > @@ -4536,12 +4536,15 @@ build_unary_op (location_t location, enum tree_code 
> > code, tree xarg,
> > case TRUTH_NOT_EXPR:
> >   if (typecode != INTEGER_TYPE && typecode != FIXED_POINT_TYPE
> >   && typecode != REAL_TYPE && typecode != POINTER_TYPE
> > - && typecode != COMPLEX_TYPE)
> > + && typecode != COMPLEX_TYPE && typecode != VECTOR_TYPE)
> > {
> >   error_at (location,
> > "wrong type argument to unary exclamation mark");
> >   return error_mark_node;
> > }
> > +  if (gnu_vector_type_p (TREE_TYPE (arg)))
> > +   return build_binary_op (location, EQ_EXPR, arg,
> > +   build_zero_cst (TREE_TYPE (arg)), false);
> >   if (int_operands)
> > {
> >

[RFC] Add support for vectors in comparisons (like the C++ frontend does)

2022-10-10 Thread Paul Iannetta via Gcc-patches
Hi,

I am trying to bridge the gap between the extensions supported by the C
and C++ front-ends.  When it comes to vector extensions, C++ supports
vectors in comparisons and within conditional expressions whereas the C
front-end does not.

I have a patch to bring this feature to the C front-end as well, and
would like to hear your opinion on it, especially since it may affect
the feature-set of the objc front-end as well.

I have tried to mirror as much as possible what the C++ front-end does
and checked that both front-end produce the same GIMPLE for all the
simple expressions as well as some more complex combinations of the
operators (?:, !, ^, || and &&).

Currently, this is only a tentative patch and I did not add any tests
to the testsuite.  Moreover, the aarch64's target-specific testsuite
explicitly tests the non-presence of this feature, which will have to
be removed.

I've run the testsuite on x86 and I've not seen any regressions.

Cheers,
Paul

#  >8 
Support for vector types in simple comparisons

gcc/

* doc/extend.texi: Remove the C++ mention, since both C and C++
  support the all the mentioned features.

gcc/c/

* c-typeck.cc (build_unary_op): Add support for vector for the
  unary exclamation mark.
(build_conditional_expr): Add support for vector in conditional
expressions.
(build_binary_op): Add support for vector for &&, || and ^.
(c_objc_common_truthvalue_conversion): Remove the special gards
preventing vector types.

#  >8 
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 17185fd3da4..03ade14cae9 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -4536,12 +4536,15 @@ build_unary_op (location_t location, enum tree_code 
code, tree xarg,
 case TRUTH_NOT_EXPR:
   if (typecode != INTEGER_TYPE && typecode != FIXED_POINT_TYPE
  && typecode != REAL_TYPE && typecode != POINTER_TYPE
- && typecode != COMPLEX_TYPE)
+ && typecode != COMPLEX_TYPE && typecode != VECTOR_TYPE)
{
  error_at (location,
"wrong type argument to unary exclamation mark");
  return error_mark_node;
}
+  if (gnu_vector_type_p (TREE_TYPE (arg)))
+   return build_binary_op (location, EQ_EXPR, arg,
+   build_zero_cst (TREE_TYPE (arg)), false);
   if (int_operands)
{
  arg = c_objc_common_truthvalue_conversion (location, xarg);
@@ -5477,6 +5480,129 @@ build_conditional_expr (location_t colon_loc, tree 
ifexp, bool ifexp_bcp,
   result_type = type2;
 }
 
+  if (gnu_vector_type_p (TREE_TYPE (ifexp))
+  && VECTOR_INTEGER_TYPE_P (TREE_TYPE (ifexp)))
+{
+  tree ifexp_type = TREE_TYPE (ifexp);
+
+  /* If ifexp is another cond_expr choosing between -1 and 0,
+then we can use its comparison.  It may help to avoid
+additional comparison, produce more accurate diagnostics
+and enables folding.  */
+  if (TREE_CODE (ifexp) == VEC_COND_EXPR
+ && integer_minus_onep (TREE_OPERAND (ifexp, 1))
+ && integer_zerop (TREE_OPERAND (ifexp, 2)))
+   ifexp = TREE_OPERAND (ifexp, 0);
+
+  tree op1_type = TREE_TYPE (op1);
+  tree op2_type = TREE_TYPE (op2);
+
+  if (!VECTOR_TYPE_P (op1_type) && !VECTOR_TYPE_P (op2_type))
+   {
+ /* Rely on the error messages of the scalar version.  */
+ tree scal =
+   build_conditional_expr (colon_loc, integer_one_node, ifexp_bcp,
+   op1, op1_original_type, op1_loc,
+   op2, op2_original_type, op2_loc);
+ if (scal == error_mark_node)
+   return error_mark_node;
+ tree stype = TREE_TYPE (scal);
+ tree ctype = TREE_TYPE (ifexp_type);
+ if (TYPE_SIZE (stype) != TYPE_SIZE (ctype)
+ || (!INTEGRAL_TYPE_P (stype) && !SCALAR_FLOAT_TYPE_P (stype)))
+   {
+ error_at (colon_loc,
+   "inferred scalar type %qT is not an integer or "
+   "floating-point type of the same size as %qT", stype,
+   COMPARISON_CLASS_P (ifexp)
+   ? TREE_TYPE (TREE_TYPE (TREE_OPERAND (ifexp, 0)))
+   : ctype);
+ return error_mark_node;
+   }
+
+ tree vtype = build_opaque_vector_type (stype,
+TYPE_VECTOR_SUBPARTS
+(ifexp_type));
+ /* The warnings (like Wsign-conversion) have already been
+given by the scalar build_conditional_expr. We still check
+unsafe_conversion_p to forbid truncating long long -> float.  */
+ if (unsafe_conversion_p (stype, op1, NULL_TREE, false))
+   {
+ error_at (colon_loc, "conversion of scalar 

Re: [PATCH] c++: parser - Support for target address spaces in C++

2022-10-09 Thread Paul Iannetta via Gcc-patches
Hi,

On Thu, Oct 06, 2022 at 01:34:40PM -0400, Jason Merrill wrote:
[snip]
> 
> Hmm?  We mangle __restrict:
> 
> void f(int *__restrict *p) { } // _Z1fPrPi
> 

Indeed, I have overlooked that point.  Thank you for pointing it out.

> but cv-qualifiers (including restrict) are dropped on the top-level type of
> a parameter.
> 
> If address spaces should be mangled, it probably makes sense to use the same
>  mangling that we already use for attributes in
> write_CV_qualifiers_for_type.
> 

That's a nice feature! LLVM is using AS to mangle the
address-name qualified types but that would have required an update of
libiberty's demangler in the binutils as well.  Following your advice,
I implemented the mangling of address_names as U__address_name
(eg., U8__seg_fs), underscores may appear but that does not seem to be
a problem.

I don't think that there should be anything to do on the template
side, but I am no expert of what is expected of templates when it
comes to strip or forward qualifiers. So, please tell me if I
overlooked something on that side.

Now concerning the meat of the patch itself.

The C part of the support of address spaces does not appear to have
dedicated tests, especially since it is mostly a target specific
feature.  I, however, added a few tests, one for the mangling,
and others which test the different error messages that might
appear when using address spaces in an unsafe way.

Most of the patch tries to mirror the commit
   309434d678721acedb299f20cdc2b4778062b180
which introduces address spaces to the C front-end.  The C and C++
parsers share a lot but are subtly different so there might be some
discrepancies.  In particular, I ensure that address spaces do not
conflict, that only one address space can be specified at a time, and
that the type conversion mechanism pick the superset of the address
spaces if available. I also forbid address spaces in compound literals.

I have only tested the patch on x86 and on our not-yet-upstream
architecture, without seeing any regressions.

NB: I do not add a DSO for the moment because I have started the process of
assigning copyright to the FSF.

#  >8 
Add support for custom address spaces in C++

gcc/
* tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.

gcc/c/
* c-decl.cc: Remove c_register_addr_space.

gcc/c-family/
* c-common.cc (c_register_addr_space): Imported from c-decl.cc
(addr_space_superset): Imported from gcc/c/c-typecheck.cc
* c-common.h: Remove the FIXME.
(addr_space_superset): Add prototype.

gcc/cp/
* cp-tree.h (enum cp_decl_spec): Add addr_space support.
(struct cp_decl_specifier_seq): Likewise.
* decl.cc (get_type_quals): Likewise.
(check_tag_decl): Likewise.
* parser.cc (cp_parser_type_specifier): Likewise.
(cp_parser_cv_qualifier_seq_opt): Likewise.
(cp_parser_postfix_expression): Likewise.
(cp_parser_type_specifier): Likewise.
(set_and_check_decl_spec_loc): Likewise.
* typeck.cc (composite_pointer_type): Likewise
(comp_ptr_ttypes_real): Likewise.
* tree.cc: Remove c_register_addr_space stub.
* mangle.cc (write_CV_qualifiers_for_type): Mangle address spaces
  using the extended qualifier notation.

gcc/doc
* extend.texi (Named Address Spaces): add a mention about C++
  support.

gcc/testsuite/
* g++.dg/abi/mangle-addr-space1.C: New test.
* g++.dg/parse/addr-space.C: New test.
* g++.dg/parse/addr-space1.C: New test.
* g++.dg/parse/addr-space2.C: New test.

#  >8 
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index bb0544eeaea..ff1146ecc25 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -615,6 +615,33 @@ c_addr_space_name (addr_space_t as)
   return IDENTIFIER_POINTER (ridpointers [rid]);
 }
 
+/* Return true if between two named address spaces, whether there is a superset
+   named address space that encompasses both address spaces.  If there is a
+   superset, return which address space is the superset.  */
+
+bool
+addr_space_superset (addr_space_t as1, addr_space_t as2,
+addr_space_t * common)
+{
+  if (as1 == as2)
+{
+  *common = as1;
+  return true;
+}
+  else if (targetm.addr_space.subset_p (as1, as2))
+{
+  *common = as2;
+  return true;
+}
+  else if (targetm.addr_space.subset_p (as2, as1))
+{
+  *common = as1;
+  return true;
+}
+  else
+return false;
+}
+
 /* Push current bindings for the function name VAR_DECLS.  */
 
 void
@@ -2809,6 +2836,25 @@ c_build_bitfield_integer_type (unsigned HOST_WIDE_INT 
width, int unsignedp)
   return build_nonstandard_integer_type (width, unsignedp);
 }
 
+/* Register reserved keyword WORD as qualifier for address space AS.  */
+
+void
+c_register_addr_space 

[RFC] c++: parser - Support for target address spaces in C++

2022-10-06 Thread Paul Iannetta via Gcc-patches
Hi,

Presently, GCC supports target address spaces as a GNU extension (cf.
`info -n "(gcc)Named Address Spaces"`).  This is however supported
only by the C frontend, which is a bit sad, since all the GIMPLE
machinery is readily available and, moreover, LLVM supports this GNU
extension both for C and C++.

Here is a first attempt at a patch to enable the support for target
address spaces in C++.  The basic idea is only to make the parser
recognize address spaces, and lower them into GIMPLE, in the same
fashion as the C parser.  This also makes it possible to merge the
function `c_register_addr_space` in one place which is better than
before.

The patch still has some drawbacks compared to its C counterpart.
Namely, much like the `__restrict__` keyword, it is always enabled and
-std=c++11 won't disable it (whereas -std=c11) would reject address
spaces.  Not also that the mangler ignores address spaces, much like
it ignores __restrict__.

Depending on the reception, I'll add further testcases and will update
the relevant part of the documentation.

Cheers,
Paul

Author: Paul Iannetta 
Date:   Wed Oct 5 16:44:36 2022 +0200

Add support for custom address spaces in C++

gcc/
* tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.

gcc/c/
* c-decl.c: Remove c_register_addr_space.

gcc/c-family/
* c-common.c (c_register_addr_space): Imported from c-decl.c
* c-common.h: Remove the FIXME.

gcc/cp/
* cp-tree.h (enum cp_decl_spec): Add addr_space support.
(struct cp_decl_specifier_seq): Likewise.
* decl.c (get_type_quals): Likewise.
* parser.c (cp_parser_type_specifier): Likewise.
(cp_parser_cv_qualifier_seq_opt): Likewise
* tree.c: Remove c_register_addr_space stub.

Signed-off-by: Paul Iannetta 

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 064c2f263f0..282ba54ab70 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -2615,6 +2615,25 @@ c_build_bitfield_integer_type (unsigned HOST_WIDE_INT 
width, int unsignedp)
   return build_nonstandard_integer_type (width, unsignedp);
 }
 
+/* Register reserved keyword WORD as qualifier for address space AS.  */
+
+void
+c_register_addr_space (const char *word, addr_space_t as)
+{
+  int rid = RID_FIRST_ADDR_SPACE + as;
+  tree id;
+
+  /* Address space qualifiers are only supported
+ in C with GNU extensions enabled.  */
+  if (c_dialect_objc () || flag_no_asm)
+return;
+
+  id = get_identifier (word);
+  C_SET_RID_CODE (id, rid);
+  TREE_LANG_FLAG_0 (id) = 1;
+  ridpointers[rid] = id;
+}
+
 /* The C version of the register_builtin_type langhook.  */
 
 void
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index ed39b7764bf..f2c1df0c8de 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -808,9 +808,6 @@ extern const struct attribute_spec 
c_common_format_attribute_table[];
 
 extern tree (*make_fname_decl) (location_t, tree, int);
 
-/* In c-decl.c and cp/tree.c.  FIXME.  */
-extern void c_register_addr_space (const char *str, addr_space_t as);
-
 /* In c-common.c.  */
 extern bool in_late_binary_op;
 extern const char *c_addr_space_name (addr_space_t as);
@@ -926,6 +923,7 @@ extern void c_common_finish (void);
 extern void c_common_parse_file (void);
 extern FILE *get_dump_info (int, dump_flags_t *);
 extern alias_set_type c_common_get_alias_set (tree);
+extern void c_register_addr_space (const char *, addr_space_t);
 extern void c_register_builtin_type (tree, const char*);
 extern bool c_promoting_integer_type_p (const_tree);
 extern bool self_promoting_args_p (const_tree);
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 8e24b522ee4..278d1248d1c 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -11927,25 +11927,6 @@ c_parse_final_cleanups (void)
   ext_block = NULL;
 }
 
-/* Register reserved keyword WORD as qualifier for address space AS.  */
-
-void
-c_register_addr_space (const char *word, addr_space_t as)
-{
-  int rid = RID_FIRST_ADDR_SPACE + as;
-  tree id;
-
-  /* Address space qualifiers are only supported
- in C with GNU extensions enabled.  */
-  if (c_dialect_objc () || flag_no_asm)
-return;
-
-  id = get_identifier (word);
-  C_SET_RID_CODE (id, rid);
-  C_IS_RESERVED_WORD (id) = 1;
-  ridpointers [rid] = id;
-}
-
 /* Return identifier to look up for omp declare reduction.  */
 
 tree
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 15ec4cd199f..4ae971ccb90 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5974,6 +5974,7 @@ enum cp_decl_spec {
   ds_const,
   ds_volatile,
   ds_restrict,
+  ds_addr_space,
   ds_inline,
   ds_virtual,
   ds_explicit,
@@ -6046,6 +6047,8 @@ struct cp_decl_specifier_seq {
   /* True iff the alternate "__intN__" form of the __intN type has been
  used.  */
   BOOL_BITFIELD int_n_alt: 1;
+  /* The address space that the declaration belongs to.  */
+  addr_space_t address_space;
 };
 
 /* The various kinds of declarators.  */
diff 

Re: [PING^1][PATCH] postscan_insn hook not called after input_asm

2022-04-07 Thread Paul Iannetta via Gcc-patches
Hi,

I would like to draw your attention on this patch, it's a pretty tiny patch 
which does not affect many targets.

- Original Message -
From: "gcc-patches" 
To: "gcc-patches" 
Sent: Tuesday, March 29, 2022 4:10:16 PM
Subject: [PATCH] postscan_insn hook not called after input_asm

Hi,

While working on the Kalray port of gcc, I noticed that the hook 
TARGET_ASM_FINAL_POSTSCAN_INSN is not called after emitting an instruction 
coming from a basic asm block.  Here is a patch which fixes this behavior.

The following check:
```
$ find gcc/config/ -type f -exec grep "#define TARGET_ASM_FINAL" "{}" +
gcc/config/m68k/m68k.cc:#define TARGET_ASM_FINAL_POSTSCAN_INSN 
m68k_asm_final_postscan_insn
gcc/config/avr/avr.cc:#define TARGET_ASM_FINAL_POSTSCAN_INSN 
avr_asm_final_postscan_insn
gcc/config/mips/mips.cc:#define TARGET_ASM_FINAL_POSTSCAN_INSN 
mips_final_postscan_insn
```
reveals that m68k, avr and mips are the only affected targets upstream.

Paul Iannetta 
Kalray






[PATCH] postscan_insn hook not called after input_asm

2022-03-29 Thread Paul Iannetta via Gcc-patches
Hi,

While working on the Kalray port of gcc, I noticed that the hook 
TARGET_ASM_FINAL_POSTSCAN_INSN is not called after emitting an instruction 
coming from a basic asm block.  Here is a patch which fixes this behavior.

The following check:
```
$ find gcc/config/ -type f -exec grep "#define TARGET_ASM_FINAL" "{}" +
gcc/config/m68k/m68k.cc:#define TARGET_ASM_FINAL_POSTSCAN_INSN 
m68k_asm_final_postscan_insn
gcc/config/avr/avr.cc:#define TARGET_ASM_FINAL_POSTSCAN_INSN 
avr_asm_final_postscan_insn
gcc/config/mips/mips.cc:#define TARGET_ASM_FINAL_POSTSCAN_INSN 
mips_final_postscan_insn
```
reveals that m68k, avr and mips are the only affected targets upstream.

Paul Iannetta 
Kalray




0001-postscan_insn-hook-not-called-after-input_asm.patch
Description: application/mbox