[PATCH][GIMPLE FE] Add parsing of MEM_REFs

[PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

The following fills the gap of missed handling of MEM_REF parsing.
As we want to represent all info that is on a MEM_REF the existing
dumping isn't sufficent so I resorted to

  __MEM '<' type-name [ ',' number ] '>'
        '(' [ '(' type-name ')' ] unary-expression
         [ '+' number ] ')'  */

where optional parts are in []s.  So for

  __MEM < int, 16 > ( (char *) &x + 1 )

we access x as 16-bit aligned int at offset 1 with TBAA type 'char'.

Naturally VIEW_CONVERT_EXPR would look like __VCE < int > (x) then,
TARGET_MEM_REF would simply get some additional operands in the
above function-like __MEM (rather than + step * index + index2).

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

The testcase parses, dumps and then parses again to the same output.

Any comments / objections to the syntax (of __MEM) and/or suggestions
for VIEW_CONVERT_EXPR or TARGET_MEM_REF?

As you can see I adjusted dumping of pointer constants (we can't
parse the B suffix and large unsigned numbers get a warning so
add 'U').  There's the general issue that we dump

  short x;
  x = 1;

and then lex the '1' as type int and there's no suffixes for integer
types smaller than int which means we can't write those constants
type correct :/  Suggestions welcome (currently we ICE with type
mismatches in those cases, we can avoid that by auto-fixing during
parsing but I'd like to be explicit somehow).

Thanks,
Richard.

2017-01-11  Richard Biener  <rguenther@suse.de>

	* tree-pretty-print.c (dump_generic_node): Provide -gimple
	variant for MEM_REF.  Sanitize INTEGER_CST for -gimple.

	c/
	* gimple-parser.c (c_parser_gimple_postfix_expression): Parse
	__MEM.

	* gcc.dg/gimplefe-21.c: New testcase.

Index: gcc/tree-pretty-print.c
===================================================================
*** gcc/tree-pretty-print.c	(revision 244312)
--- gcc/tree-pretty-print.c	(working copy)
*************** dump_generic_node (pretty_printer *pp, t
*** 1459,1465 ****
  
      case MEM_REF:
        {
! 	if (integer_zerop (TREE_OPERAND (node, 1))
  	    /* Dump the types of INTEGER_CSTs explicitly, for we can't
  	       infer them and MEM_ATTR caching will share MEM_REFs
  	       with differently-typed op0s.  */
--- 1459,1496 ----
  
      case MEM_REF:
        {
! 	if (flags & TDF_GIMPLE)
! 	  {
! 	    pp_string (pp, "__MEM <");
! 	    dump_generic_node (pp, TREE_TYPE (node),
! 			       spc, flags | TDF_SLIM, false);
! 	    if (TYPE_ALIGN (TREE_TYPE (node))
! 		!= TYPE_ALIGN (TYPE_MAIN_VARIANT (TREE_TYPE (node))))
! 	      {
! 		pp_string (pp, ", ");
! 		pp_decimal_int (pp, TYPE_ALIGN (TREE_TYPE (node)));
! 	      }
! 	    pp_greater (pp);
! 	    pp_string (pp, " (");
! 	    if (TREE_TYPE (TREE_OPERAND (node, 0))
! 		!= TREE_TYPE (TREE_OPERAND (node, 1)))
! 	      {
! 		pp_left_paren (pp);
! 		dump_generic_node (pp, TREE_TYPE (TREE_OPERAND (node, 1)),
! 				   spc, flags | TDF_SLIM, false);
! 		pp_right_paren (pp);
! 	      }
! 	    dump_generic_node (pp, TREE_OPERAND (node, 0),
! 			       spc, flags | TDF_SLIM, false);
! 	    if (! integer_zerop (TREE_OPERAND (node, 1)))
! 	      {
! 		pp_string (pp, " + ");
! 		dump_generic_node (pp, TREE_OPERAND (node, 1),
! 				   spc, flags | TDF_SLIM, false);
! 	      }
! 	    pp_right_paren (pp);
! 	  }
! 	else if (integer_zerop (TREE_OPERAND (node, 1))
  	    /* Dump the types of INTEGER_CSTs explicitly, for we can't
  	       infer them and MEM_ATTR caching will share MEM_REFs
  	       with differently-typed op0s.  */
*************** dump_generic_node (pretty_printer *pp, t
*** 1633,1639 ****
        break;
  
      case INTEGER_CST:
!       if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE)
  	{
  	  /* In the case of a pointer, one may want to divide by the
  	     size of the pointed-to type.  Unfortunately, this not
--- 1664,1671 ----
        break;
  
      case INTEGER_CST:
!       if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE
! 	  && ! (flags & TDF_GIMPLE))
  	{
  	  /* In the case of a pointer, one may want to divide by the
  	     size of the pointed-to type.  Unfortunately, this not
*************** dump_generic_node (pretty_printer *pp, t
*** 1661,1667 ****
        else if (tree_fits_shwi_p (node))
  	pp_wide_integer (pp, tree_to_shwi (node));
        else if (tree_fits_uhwi_p (node))
! 	pp_unsigned_wide_integer (pp, tree_to_uhwi (node));
        else
  	{
  	  wide_int val = node;
--- 1693,1703 ----
        else if (tree_fits_shwi_p (node))
  	pp_wide_integer (pp, tree_to_shwi (node));
        else if (tree_fits_uhwi_p (node))
! 	{
! 	  pp_unsigned_wide_integer (pp, tree_to_uhwi (node));
! 	  if (flags & TDF_GIMPLE)
! 	    pp_character (pp, 'U');
! 	}
        else
  	{
  	  wide_int val = node;
Index: gcc/c/gimple-parser.c
===================================================================
*** gcc/c/gimple-parser.c	(revision 244312)
--- gcc/c/gimple-parser.c	(working copy)
*************** c_parser_gimple_postfix_expression (c_pa
*** 727,732 ****
--- 727,804 ----
        if (c_parser_peek_token (parser)->id_kind == C_ID_ID)
  	{
  	  tree id = c_parser_peek_token (parser)->value;
+ 	  if (strcmp (IDENTIFIER_POINTER (id), "__MEM") == 0)
+ 	    {
+ 	      /* __MEM '<' type-name [ ',' number ] '>'
+ 	               '(' [ '(' type-name ')' ] unary-expression
+ 		           [ '+' number ] ')'  */
+ 	      location_t loc = c_parser_peek_token (parser)->location;
+ 	      c_parser_consume_token (parser);
+ 	      struct c_type_name *type_name = NULL;
+ 	      tree alignment = NULL_TREE;
+ 	      if (c_parser_require (parser, CPP_LESS, "expected %<<%>"))
+ 	        {
+ 		  type_name = c_parser_type_name (parser);
+ 		  /* Optional alignment.  */
+ 		  if (c_parser_next_token_is (parser, CPP_COMMA))
+ 		    {
+ 		      c_parser_consume_token (parser);
+ 		      alignment
+ 			= c_parser_gimple_postfix_expression (parser).value;
+ 		    }
+ 		  c_parser_skip_until_found (parser,
+ 					     CPP_GREATER, "expected %<>%>");
+ 		}
+ 	      struct c_expr ptr;
+ 	      tree alias_off = NULL_TREE;
+ 	      if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
+ 		{
+ 		  tree alias_type = NULL_TREE;
+ 		  /* Optional alias-type cast.  */
+ 		  if (c_parser_next_token_is (parser, CPP_OPEN_PAREN))
+ 		    {
+ 		      c_parser_consume_token (parser);
+ 		      struct c_type_name *alias_type_name
+ 			= c_parser_type_name (parser);
+ 		      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+ 						 "expected %<)%>");
+ 		      if (alias_type_name)
+ 			{
+ 			  tree tem;
+ 			  alias_type = groktypename (alias_type_name,
+ 						     &tem, NULL);
+ 			}
+ 		    }
+ 		  ptr = c_parser_gimple_unary_expression (parser);
+ 		  if (! alias_type)
+ 		    alias_type = TREE_TYPE (ptr.value);
+ 		  /* Optional constant offset.  */
+ 		  if (c_parser_next_token_is (parser, CPP_PLUS))
+ 		    {
+ 		      c_parser_consume_token (parser);
+ 		      alias_off
+ 			= c_parser_gimple_postfix_expression (parser).value;
+ 		      alias_off = fold_convert (alias_type, alias_off);
+ 		    }
+ 		  if (! alias_off)
+ 		    alias_off = build_int_cst (alias_type, 0);
+ 		  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+ 					     "expected %<)%>");
+ 		}
+ 	      if (! type_name || c_parser_error (parser))
+ 		{
+ 		  c_parser_set_error (parser, false);
+ 		  return expr;
+ 		}
+ 	      tree tem = NULL_TREE;
+ 	      tree type = groktypename (type_name, &tem, NULL);
+ 	      if (alignment)
+ 		type = build_aligned_type (type, tree_to_uhwi (alignment));
+ 	      expr.value = build2_loc (loc, MEM_REF,
+ 				       type, ptr.value, alias_off);
+ 	      break;
+ 	    }
+ 	  /* SSA name.  */
  	  unsigned version, ver_offset;
  	  if (! lookup_name (id)
  	      && c_parser_parse_ssa_name_id (id, &version, &ver_offset))
Index: gcc/testsuite/gcc.dg/gimplefe-21.c
===================================================================
*** gcc/testsuite/gcc.dg/gimplefe-21.c	(nonexistent)
--- gcc/testsuite/gcc.dg/gimplefe-21.c	(working copy)
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do compile } */
+ /* { dg-options "-fgimple" } */
+ 
+ float __GIMPLE ()
+ foo (int * p)
+ {
+   float f;
+   float D1800;
+   unsigned int D1799;
+ 
+   D1799 = __MEM <unsigned int, 8> ((char *)p + 1);
+   __MEM <unsigned int, 16> ((char *)&f + 0xfffffffffffffffe) = D1799;
+   __MEM <int> (p) = 1;
+   __MEM <int, 2> (p) = 1;
+   __MEM <int> (p + 2) = 1;
+   __MEM <int> ((char *)p) = 1;
+   D1800 = f;
+   return D1800;
+ }

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Joseph Myers]

On Wed, 11 Jan 2017, Richard Biener wrote:

> As you can see I adjusted dumping of pointer constants (we can't
> parse the B suffix and large unsigned numbers get a warning so
> add 'U').  There's the general issue that we dump
> 
>   short x;
>   x = 1;
> 
> and then lex the '1' as type int and there's no suffixes for integer
> types smaller than int which means we can't write those constants
> type correct :/  Suggestions welcome (currently we ICE with type
> mismatches in those cases, we can avoid that by auto-fixing during
> parsing but I'd like to be explicit somehow).

You could always dump as ((short) 1); that's valid C.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Wed, 11 Jan 2017, Joseph Myers wrote:

> On Wed, 11 Jan 2017, Richard Biener wrote:
> 
> > As you can see I adjusted dumping of pointer constants (we can't
> > parse the B suffix and large unsigned numbers get a warning so
> > add 'U').  There's the general issue that we dump
> > 
> >   short x;
> >   x = 1;
> > 
> > and then lex the '1' as type int and there's no suffixes for integer
> > types smaller than int which means we can't write those constants
> > type correct :/  Suggestions welcome (currently we ICE with type
> > mismatches in those cases, we can avoid that by auto-fixing during
> > parsing but I'd like to be explicit somehow).
> 
> You could always dump as ((short) 1); that's valid C.

Indeed.  It makes parsing a little more awkward as (short) 1 is not
a postfix expression.

I suppose adding additional suffixes for char/short conditional on
-fgimple would be another way (either via setting user_literals option
or teaching libcpp about those itself).  's' and 'ss' (CPP_N_SMALL_SMALL
and CPP_N_SMALL_SMALL_SMALL?) would be my choice...

I'll give the (short) 1 parsing a try though to see how awkward it
really gets.

Thanks,
Richard.

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Thu, 12 Jan 2017, Richard Biener wrote:

> On Wed, 11 Jan 2017, Joseph Myers wrote:
> 
> > On Wed, 11 Jan 2017, Richard Biener wrote:
> > 
> > > As you can see I adjusted dumping of pointer constants (we can't
> > > parse the B suffix and large unsigned numbers get a warning so
> > > add 'U').  There's the general issue that we dump
> > > 
> > >   short x;
> > >   x = 1;
> > > 
> > > and then lex the '1' as type int and there's no suffixes for integer
> > > types smaller than int which means we can't write those constants
> > > type correct :/  Suggestions welcome (currently we ICE with type
> > > mismatches in those cases, we can avoid that by auto-fixing during
> > > parsing but I'd like to be explicit somehow).
> > 
> > You could always dump as ((short) 1); that's valid C.
> 
> Indeed.  It makes parsing a little more awkward as (short) 1 is not
> a postfix expression.
> 
> I suppose adding additional suffixes for char/short conditional on
> -fgimple would be another way (either via setting user_literals option
> or teaching libcpp about those itself).  's' and 'ss' (CPP_N_SMALL_SMALL
> and CPP_N_SMALL_SMALL_SMALL?) would be my choice...
> 
> I'll give the (short) 1 parsing a try though to see how awkward it
> really gets.

Ok, doesn't look a good way to go.  Apart from making it difficult
to handle in the parser you can't distinguish a conversion from an
integer literal and a short literal for

 short s;
 s_1 = (short) 1;

I've pasted below what I came up with for 's' and 'ss'.  It's just
a prototype as it doens't handle larger literals and bit-precision
literals we have in the IL.  So I suppose user_literals and
interpreting uN/sN with N being the actual precision looks better
here?  Likewise we need something for pointer literals (where we
might need to encode the address-space as well...).  A simple pN
would work for a start I suppose.

It looks like we currently have no way to write __int128 literals in C?

__int128 x = 0xffffeeeeffffeeeeffffeeeeffffeeee;

yields

t.c:1:14: warning: integer constant is too large for its type
 __int128 x = 0xffffeeeeffffeeeeffffeeeeffffeeee;
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and truncation to 64bits which looks like a limitation of cpp_num.

I'd like to solve the issue of short and char literals for GIMPLE
testcases for GCC 6, any preference on the solution?  There's also
bool literals (1-bit precision integers in GIMPLE...), for those
using _True and _False might be a workaround.  Though 1u1 and 0u1
would work for those as well.

Thanks,
Richard.

2017-01-12  Richard Biener  <rguenther@suse.de>

	c-family/
	* c-lex.c (narrowest_unsigned_type): Handle CPP_N_TINY and
	CPP_N_MINISCULE.
	(narrowest_signed_type): Likewise.

	libcpp/
	* include/cpplib.h (CPP_N_TINY): New.
	(CPP_N_MINISCULE): Likewise.
	* expr.c (interpret_int_suffix): Map 's' to CPP_N_TINY and
	'ss' to CPP_N_MINISCULE if ext_numeric_literals.

	* tree-pretty-print.c (dump_generic_node): Dump integer constant
	type suffixes with -gimple.

Index: gcc/c-family/c-lex.c
===================================================================
--- gcc/c-family/c-lex.c	(revision 244350)
+++ gcc/c-family/c-lex.c	(working copy)
@@ -641,7 +641,11 @@ narrowest_unsigned_type (const widest_in
 {
   int itk;
 
-  if ((flags & CPP_N_WIDTH) == CPP_N_SMALL)
+  if ((flags & CPP_N_WIDTH) == CPP_N_MINISCULE)
+    itk = itk_unsigned_char;
+  else if ((flags & CPP_N_WIDTH) == CPP_N_TINY)
+    itk = itk_unsigned_short;
+  else if ((flags & CPP_N_WIDTH) == CPP_N_SMALL)
     itk = itk_unsigned_int;
   else if ((flags & CPP_N_WIDTH) == CPP_N_MEDIUM)
     itk = itk_unsigned_long;
@@ -669,7 +673,11 @@ narrowest_signed_type (const widest_int
 {
   int itk;
 
-  if ((flags & CPP_N_WIDTH) == CPP_N_SMALL)
+  if ((flags & CPP_N_WIDTH) == CPP_N_MINISCULE)
+    itk = itk_signed_char;
+  else if ((flags & CPP_N_WIDTH) == CPP_N_TINY)
+    itk = itk_short;
+  else if ((flags & CPP_N_WIDTH) == CPP_N_SMALL)
     itk = itk_int;
   else if ((flags & CPP_N_WIDTH) == CPP_N_MEDIUM)
     itk = itk_long;
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c	(revision 244350)
+++ gcc/tree-pretty-print.c	(working copy)
@@ -1710,6 +1710,28 @@ dump_generic_node (pretty_printer *pp, t
 	  print_hex (val, pp_buffer (pp)->digit_buffer);
 	  pp_string (pp, pp_buffer (pp)->digit_buffer);
 	}
+      if (flags & TDF_GIMPLE)
+	{
+	  if (TYPE_UNSIGNED (TREE_TYPE (node)))
+	    pp_character (pp, 'u');
+	  if (POINTER_TYPE_P (TREE_TYPE (node)))
+	    ;
+	  else if (TYPE_PRECISION (TREE_TYPE (node))
+		   == TYPE_PRECISION (unsigned_char_type_node))
+	    pp_string (pp, "ss");
+	  else if (TYPE_PRECISION (TREE_TYPE (node))
+		   == TYPE_PRECISION (short_unsigned_type_node))
+	    pp_character (pp, 's');
+	  else if (TYPE_PRECISION (TREE_TYPE (node))
+		   == TYPE_PRECISION (unsigned_type_node))
+	    ;
+	  else if (TYPE_PRECISION (TREE_TYPE (node))
+		   == TYPE_PRECISION (long_unsigned_type_node))
+	    pp_character (pp, 'l');
+	  else if (TYPE_PRECISION (TREE_TYPE (node))
+		   == TYPE_PRECISION (long_long_unsigned_type_node))
+	    pp_string (pp, "ll");
+	}
       if (TREE_OVERFLOW (node))
 	pp_string (pp, "(OVF)");
       break;
Index: libcpp/expr.c
===================================================================
--- libcpp/expr.c	(revision 244350)
+++ libcpp/expr.c	(working copy)
@@ -293,40 +293,46 @@ cpp_interpret_float_suffix (cpp_reader *
   return interpret_float_suffix (pfile, (const unsigned char *)s, len);
 }
 
-/* Subroutine of cpp_classify_number.  S points to an integer suffix
+/* Subroutine of cpp_classify_number.  SUFF points to an integer suffix
    of length LEN, possibly zero. Returns 0 for an invalid suffix, or a
    flag vector describing the suffix.  */
 static unsigned int
-interpret_int_suffix (cpp_reader *pfile, const uchar *s, size_t len)
+interpret_int_suffix (cpp_reader *pfile, const uchar *suff, size_t len)
 {
-  size_t u, l, i;
+  size_t u, l, i, s;
 
-  u = l = i = 0;
+  u = l = i = s = 0;
 
   while (len--)
-    switch (s[len])
+    switch (suff[len])
       {
       case 'u': case 'U':	u++; break;
       case 'i': case 'I':
       case 'j': case 'J':	i++; break;
       case 'l': case 'L':	l++;
 	/* If there are two Ls, they must be adjacent and the same case.  */
-	if (l == 2 && s[len] != s[len + 1])
+	if (l == 2 && suff[len] != suff[len + 1])
+	  return 0;
+	break;
+      case 's': case 'S':	s++;
+	/* If there are two Ss, they must be adjacent and the same case.  */
+	if (s == 2 && suff[len] != suff[len + 1])
 	  return 0;
 	break;
       default:
 	return 0;
       }
 
-  if (l > 2 || u > 1 || i > 1)
+  if (l > 2 || s > 2 || (l > 0 && s > 0) || u > 1 || i > 1)
     return 0;
 
-  if (i && !CPP_OPTION (pfile, ext_numeric_literals))
+  if ((i || s) && !CPP_OPTION (pfile, ext_numeric_literals))
     return 0;
 
   return ((i ? CPP_N_IMAGINARY : 0)
 	  | (u ? CPP_N_UNSIGNED : 0)
-	  | ((l == 0) ? CPP_N_SMALL
+	  | ((l == 0) ? (s == 0 ? CPP_N_SMALL
+				: (s == 1) ? CPP_N_TINY : CPP_N_MINISCULE)
 	     : (l == 1) ? CPP_N_MEDIUM : CPP_N_LARGE));
 }
 
Index: libcpp/include/cpplib.h
===================================================================
--- libcpp/include/cpplib.h	(revision 244350)
+++ libcpp/include/cpplib.h	(working copy)
@@ -947,6 +947,8 @@ struct cpp_num
 #define CPP_N_MEDIUM	0x0020	/* long, double, long _Fract/_Accum.  */
 #define CPP_N_LARGE	0x0040	/* long long, long double,
 				   long long _Fract/Accum.  */
+#define CPP_N_TINY	0x0080  /* short.  */
+#define CPP_N_MINISCULE	0x0090  /* char.  */
 
 #define CPP_N_WIDTH_MD	0xF0000	/* machine defined.  */
 #define CPP_N_MD_W	0x10000

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Jakub Jelinek]

On Thu, Jan 12, 2017 at 10:52:23AM +0100, Richard Biener wrote:
> > I'll give the (short) 1 parsing a try though to see how awkward it
> > really gets.
> 
> Ok, doesn't look a good way to go.  Apart from making it difficult
> to handle in the parser you can't distinguish a conversion from an
> integer literal and a short literal for
> 
>  short s;
>  s_1 = (short) 1;

As there are tons of types the integer literals can have, wouldn't it be
better to just introduce _Literal <type, value> where you could supply
the type if it is not one where C provides a suffix for it or int?
Then we could avoid adding lots of suffixes for new and newer types.
Of course for integer literals with int, unsigned int, {,unsigned} long {,long}
one would still use no suffix, U, {,U}L{,L} suffixes.

> It looks like we currently have no way to write __int128 literals in C?
> 
> __int128 x = 0xffffeeeeffffeeeeffffeeeeffffeeee;

Yeah, one typically has to use
__int128 x = (((unsigned __int128) 0xffffeeeeffffeeeeULL) << 64) | 0xffffeeeeffffeeeeULL;
or something similar.

	Jakub

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Thu, 12 Jan 2017, Jakub Jelinek wrote:

> On Thu, Jan 12, 2017 at 10:52:23AM +0100, Richard Biener wrote:
> > > I'll give the (short) 1 parsing a try though to see how awkward it
> > > really gets.
> > 
> > Ok, doesn't look a good way to go.  Apart from making it difficult
> > to handle in the parser you can't distinguish a conversion from an
> > integer literal and a short literal for
> > 
> >  short s;
> >  s_1 = (short) 1;
> 
> As there are tons of types the integer literals can have, wouldn't it be
> better to just introduce _Literal <type, value> where you could supply
> the type if it is not one where C provides a suffix for it or int?
> Then we could avoid adding lots of suffixes for new and newer types.
> Of course for integer literals with int, unsigned int, {,unsigned} long {,long}
> one would still use no suffix, U, {,U}L{,L} suffixes.

That's an interesting idea.  _Literal (type) value might be alternative
syntax (a cast to be evaulated as literal).  I'll give it a shot.

> > It looks like we currently have no way to write __int128 literals in C?
> > 
> > __int128 x = 0xffffeeeeffffeeeeffffeeeeffffeeee;
> 
> Yeah, one typically has to use
> __int128 x = (((unsigned __int128) 0xffffeeeeffffeeeeULL) << 64) | 0xffffeeeeffffeeeeULL;
> or something similar.

I see.

Thanks,
Richard.

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Prathamesh Kulkarni]

On 12 January 2017 at 16:15, Richard Biener <rguenther@suse.de> wrote:
> On Thu, 12 Jan 2017, Jakub Jelinek wrote:
>
>> On Thu, Jan 12, 2017 at 10:52:23AM +0100, Richard Biener wrote:
>> > > I'll give the (short) 1 parsing a try though to see how awkward it
>> > > really gets.
>> >
>> > Ok, doesn't look a good way to go.  Apart from making it difficult
>> > to handle in the parser you can't distinguish a conversion from an
>> > integer literal and a short literal for
>> >
>> >  short s;
>> >  s_1 = (short) 1;
>>
>> As there are tons of types the integer literals can have, wouldn't it be
>> better to just introduce _Literal <type, value> where you could supply
>> the type if it is not one where C provides a suffix for it or int?
>> Then we could avoid adding lots of suffixes for new and newer types.
>> Of course for integer literals with int, unsigned int, {,unsigned} long {,long}
>> one would still use no suffix, U, {,U}L{,L} suffixes.
>
> That's an interesting idea.  _Literal (type) value might be alternative
> syntax (a cast to be evaulated as literal).  I'll give it a shot.
Um, how about just type(value) ?
s_1 = short (10);
AFAIU this isn't legal C and could be added as an extension ?

Thanks,
Prathamesh
>
>> > It looks like we currently have no way to write __int128 literals in C?
>> >
>> > __int128 x = 0xffffeeeeffffeeeeffffeeeeffffeeee;
>>
>> Yeah, one typically has to use
>> __int128 x = (((unsigned __int128) 0xffffeeeeffffeeeeULL) << 64) | 0xffffeeeeffffeeeeULL;
>> or something similar.
>
> I see.
>
> Thanks,
> Richard.

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Jakub Jelinek]

On Thu, Jan 12, 2017 at 04:38:27PM +0530, Prathamesh Kulkarni wrote:
> On 12 January 2017 at 16:15, Richard Biener <rguenther@suse.de> wrote:
> > On Thu, 12 Jan 2017, Jakub Jelinek wrote:
> >
> >> On Thu, Jan 12, 2017 at 10:52:23AM +0100, Richard Biener wrote:
> >> > > I'll give the (short) 1 parsing a try though to see how awkward it
> >> > > really gets.
> >> >
> >> > Ok, doesn't look a good way to go.  Apart from making it difficult
> >> > to handle in the parser you can't distinguish a conversion from an
> >> > integer literal and a short literal for
> >> >
> >> >  short s;
> >> >  s_1 = (short) 1;
> >>
> >> As there are tons of types the integer literals can have, wouldn't it be
> >> better to just introduce _Literal <type, value> where you could supply
> >> the type if it is not one where C provides a suffix for it or int?
> >> Then we could avoid adding lots of suffixes for new and newer types.
> >> Of course for integer literals with int, unsigned int, {,unsigned} long {,long}
> >> one would still use no suffix, U, {,U}L{,L} suffixes.
> >
> > That's an interesting idea.  _Literal (type) value might be alternative
> > syntax (a cast to be evaulated as literal).  I'll give it a shot.
> Um, how about just type(value) ?
> s_1 = short (10);
> AFAIU this isn't legal C and could be added as an extension ?

Even C++ doesn't allow this syntax if the type isn't a single token.
So you can't use unsigned __int128 (5) or unsigned char (7) or signed char (-5).

	Jakub

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Thu, 12 Jan 2017, Jakub Jelinek wrote:

> On Thu, Jan 12, 2017 at 04:38:27PM +0530, Prathamesh Kulkarni wrote:
> > On 12 January 2017 at 16:15, Richard Biener <rguenther@suse.de> wrote:
> > > On Thu, 12 Jan 2017, Jakub Jelinek wrote:
> > >
> > >> On Thu, Jan 12, 2017 at 10:52:23AM +0100, Richard Biener wrote:
> > >> > > I'll give the (short) 1 parsing a try though to see how awkward it
> > >> > > really gets.
> > >> >
> > >> > Ok, doesn't look a good way to go.  Apart from making it difficult
> > >> > to handle in the parser you can't distinguish a conversion from an
> > >> > integer literal and a short literal for
> > >> >
> > >> >  short s;
> > >> >  s_1 = (short) 1;
> > >>
> > >> As there are tons of types the integer literals can have, wouldn't it be
> > >> better to just introduce _Literal <type, value> where you could supply
> > >> the type if it is not one where C provides a suffix for it or int?
> > >> Then we could avoid adding lots of suffixes for new and newer types.
> > >> Of course for integer literals with int, unsigned int, {,unsigned} long {,long}
> > >> one would still use no suffix, U, {,U}L{,L} suffixes.
> > >
> > > That's an interesting idea.  _Literal (type) value might be alternative
> > > syntax (a cast to be evaulated as literal).  I'll give it a shot.
> > Um, how about just type(value) ?
> > s_1 = short (10);
> > AFAIU this isn't legal C and could be added as an extension ?
> 
> Even C++ doesn't allow this syntax if the type isn't a single token.
> So you can't use unsigned __int128 (5) or unsigned char (7) or signed char (-5).

It also lacks expressiveness when comparing

  short s;
  s_1 = (short) 1;
  s_2 = short (1);

IMHO having a _Literal somewhere makes it more obvious what is meant
(I'm still going to dump 1u or 1l instead of _Literal (unsigned) 1
of course as that's even more easy to recognize).

Richard.

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Jakub Jelinek]

On Thu, Jan 12, 2017 at 01:35:32PM +0100, Richard Biener wrote:
> It also lacks expressiveness when comparing
> 
>   short s;
>   s_1 = (short) 1;
>   s_2 = short (1);
> 
> IMHO having a _Literal somewhere makes it more obvious what is meant
> (I'm still going to dump 1u or 1l instead of _Literal (unsigned) 1
> of course as that's even more easy to recognize).

So, with the _Literal (type) constant syntax, what are we going to emit
and parse for __int128 constants?
_Literal (__int128) (0x123456ULL << 64 || 0x123456ULL), something else?

	Jakub

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Thu, 12 Jan 2017, Jakub Jelinek wrote:

> On Thu, Jan 12, 2017 at 01:35:32PM +0100, Richard Biener wrote:
> > It also lacks expressiveness when comparing
> > 
> >   short s;
> >   s_1 = (short) 1;
> >   s_2 = short (1);
> > 
> > IMHO having a _Literal somewhere makes it more obvious what is meant
> > (I'm still going to dump 1u or 1l instead of _Literal (unsigned) 1
> > of course as that's even more easy to recognize).
> 
> So, with the _Literal (type) constant syntax, what are we going to emit
> and parse for __int128 constants?
> _Literal (__int128) (0x123456ULL << 64 || 0x123456ULL), something else?

Yes, unless we teach libcpp about larger than 64bit literals.

The following patch implements _Literal (but not proper dumping of
> 64bit literals for __int128 nor parsing the above).

I think that's a good start which also covers pointer types nicely.
For integers we might at some point want to adopt sN and uN suffixes
which appearantly MSVC supports to handle native 128bit literals.

For

struct X { int a : 2; };
void __GIMPLE ()
foo (struct X * p)
{
  p->a = _Literal (int : 2) 1;
  return;
}

and thus 2 bit literals c_parser_type_name needs to be extended
or for _Literal we can handle : 2 on our own.  But for loads
from p->a into a temporary we need to handle that in declarations as well.

struct X { int a : 2; };
int __GIMPLE ()
foo (struct X * p)
{
  int : 2 a; // int a : 2;
  a = p->a;
  return a;
}

Richard.

2017-01-12  Richard Biener  <rguenther@suse.de>

	c/
	* gimple-parser.c (c_parser_gimple_postfix_expression): Parse
	_Literal ( type-name ) number.

	* tree-pretty-print.c (dump_generic_node): Dump INTEGER_CSTs
	as _Literal ( type ) number in case usual suffixes do not
	preserve all information.

	* gcc.dg/gimplefe-22.c: New testcase.

Index: gcc/c/gimple-parser.c
===================================================================
--- gcc/c/gimple-parser.c	(revision 244350)
+++ gcc/c/gimple-parser.c	(working copy)
@@ -799,6 +799,32 @@ c_parser_gimple_postfix_expression (c_pa
 				       type, ptr.value, alias_off);
 	      break;
 	    }
+	  else if (strcmp (IDENTIFIER_POINTER (id), "_Literal") == 0)
+	    {
+	      /* _Literal '(' type-name ')' number  */
+	      c_parser_consume_token (parser);
+	      tree type = NULL_TREE;
+	      if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
+		{
+		  struct c_type_name *type_name = c_parser_type_name (parser);
+		  tree tem;
+		  if (type_name)
+		    type = groktypename (type_name, &tem, NULL);
+		  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+					     "expected %<)%>");
+		}
+	      tree val = c_parser_gimple_postfix_expression (parser).value;
+	      if (! type
+		  || ! val
+		  || val == error_mark_node
+		  || TREE_CODE (val) != INTEGER_CST)
+		{
+		  c_parser_error (parser, "invalid _Literal");
+		  return expr;
+		}
+	      expr.value = fold_convert (type, val);
+	      return expr;
+	    }
 	  /* SSA name.  */
 	  unsigned version, ver_offset;
 	  if (! lookup_name (id)
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c	(revision 244350)
+++ gcc/tree-pretty-print.c	(working copy)
@@ -1664,6 +1664,16 @@ dump_generic_node (pretty_printer *pp, t
       break;
 
     case INTEGER_CST:
+      if (flags & TDF_GIMPLE
+	  && (POINTER_TYPE_P (TREE_TYPE (node))
+	      || (TYPE_PRECISION (TREE_TYPE (node))
+		  < TYPE_PRECISION (integer_type_node))
+	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
+	{
+	  pp_string (pp, "_Literal (");
+	  dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
+	  pp_string (pp, ") ");
+	}
       if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE
 	  && ! (flags & TDF_GIMPLE))
 	{
@@ -1693,11 +1703,7 @@ dump_generic_node (pretty_printer *pp, t
       else if (tree_fits_shwi_p (node))
 	pp_wide_integer (pp, tree_to_shwi (node));
       else if (tree_fits_uhwi_p (node))
-	{
-	  pp_unsigned_wide_integer (pp, tree_to_uhwi (node));
-	  if (flags & TDF_GIMPLE)
-	    pp_character (pp, 'U');
-	}
+	pp_unsigned_wide_integer (pp, tree_to_uhwi (node));
       else
 	{
 	  wide_int val = node;
@@ -1710,6 +1716,24 @@ dump_generic_node (pretty_printer *pp, t
 	  print_hex (val, pp_buffer (pp)->digit_buffer);
 	  pp_string (pp, pp_buffer (pp)->digit_buffer);
 	}
+      if ((flags & TDF_GIMPLE)
+	  && (POINTER_TYPE_P (TREE_TYPE (node))
+	      || (TYPE_PRECISION (TREE_TYPE (node))
+		  < TYPE_PRECISION (integer_type_node))
+	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
+	{
+	  if (TYPE_UNSIGNED (TREE_TYPE (node)))
+	    pp_character (pp, 'u');
+	  if (TYPE_PRECISION (TREE_TYPE (node))
+	      == TYPE_PRECISION (unsigned_type_node))
+	    ;
+	  else if (TYPE_PRECISION (TREE_TYPE (node))
+		   == TYPE_PRECISION (long_unsigned_type_node))
+	    pp_character (pp, 'l');
+	  else if (TYPE_PRECISION (TREE_TYPE (node))
+		   == TYPE_PRECISION (long_long_unsigned_type_node))
+	    pp_string (pp, "ll");
+	}
       if (TREE_OVERFLOW (node))
 	pp_string (pp, "(OVF)");
       break;
Index: gcc/testsuite/gcc.dg/gimplefe-22.c
===================================================================
--- gcc/testsuite/gcc.dg/gimplefe-22.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/gimplefe-22.c	(working copy)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple" } */
+
+void __GIMPLE ()
+foo (short * p)
+{
+  *p = _Literal (short int) 1;
+  return;
+}

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Marc Glisse]

On Thu, 12 Jan 2017, Richard Biener wrote:

> On Thu, 12 Jan 2017, Jakub Jelinek wrote:
>
>> On Thu, Jan 12, 2017 at 01:35:32PM +0100, Richard Biener wrote:
>>> It also lacks expressiveness when comparing
>>>
>>>   short s;
>>>   s_1 = (short) 1;
>>>   s_2 = short (1);
>>>
>>> IMHO having a _Literal somewhere makes it more obvious what is meant
>>> (I'm still going to dump 1u or 1l instead of _Literal (unsigned) 1
>>> of course as that's even more easy to recognize).
>>
>> So, with the _Literal (type) constant syntax, what are we going to emit
>> and parse for __int128 constants?
>> _Literal (__int128) (0x123456ULL << 64 || 0x123456ULL), something else?
>
> Yes, unless we teach libcpp about larger than 64bit literals.

Teaching libcpp about 128bit literals sounds like a much nicer idea 
indeed...

-- 
Marc Glisse

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Jakub Jelinek]

On Thu, Jan 12, 2017 at 04:45:41PM +0100, Marc Glisse wrote:
> > On Thu, 12 Jan 2017, Jakub Jelinek wrote:
> > 
> > > On Thu, Jan 12, 2017 at 01:35:32PM +0100, Richard Biener wrote:
> > > > It also lacks expressiveness when comparing
> > > > 
> > > >   short s;
> > > >   s_1 = (short) 1;
> > > >   s_2 = short (1);
> > > > 
> > > > IMHO having a _Literal somewhere makes it more obvious what is meant
> > > > (I'm still going to dump 1u or 1l instead of _Literal (unsigned) 1
> > > > of course as that's even more easy to recognize).
> > > 
> > > So, with the _Literal (type) constant syntax, what are we going to emit
> > > and parse for __int128 constants?
> > > _Literal (__int128) (0x123456ULL << 64 || 0x123456ULL), something else?
> > 
> > Yes, unless we teach libcpp about larger than 64bit literals.
> 
> Teaching libcpp about 128bit literals sounds like a much nicer idea
> indeed...

Well, we need some suffix that will not clash with C++ mandated udlits
very soon, it is already bad that i and Q suffixes are problematic.
By sN, did you mean s128 or literally sN?
I've googled around a little bit and only found comments about
1ui64 and 1i64 suffixes, so that would mean accepting instead of l/L
i128 or I128.

	Jakub

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On January 12, 2017 4:53:54 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Thu, Jan 12, 2017 at 04:45:41PM +0100, Marc Glisse wrote:
>> > On Thu, 12 Jan 2017, Jakub Jelinek wrote:
>> > 
>> > > On Thu, Jan 12, 2017 at 01:35:32PM +0100, Richard Biener wrote:
>> > > > It also lacks expressiveness when comparing
>> > > > 
>> > > >   short s;
>> > > >   s_1 = (short) 1;
>> > > >   s_2 = short (1);
>> > > > 
>> > > > IMHO having a _Literal somewhere makes it more obvious what is
>meant
>> > > > (I'm still going to dump 1u or 1l instead of _Literal
>(unsigned) 1
>> > > > of course as that's even more easy to recognize).
>> > > 
>> > > So, with the _Literal (type) constant syntax, what are we going
>to emit
>> > > and parse for __int128 constants?
>> > > _Literal (__int128) (0x123456ULL << 64 || 0x123456ULL), something
>else?
>> > 
>> > Yes, unless we teach libcpp about larger than 64bit literals.
>> 
>> Teaching libcpp about 128bit literals sounds like a much nicer idea
>> indeed...
>
>Well, we need some suffix that will not clash with C++ mandated udlits
>very soon, it is already bad that i and Q suffixes are problematic.
>By sN, did you mean s128 or literally sN?
>I've googled around a little bit and only found comments about
>1ui64 and 1i64 suffixes, so that would mean accepting instead of l/L
>i128 or I128.

Indeed i64 and ui64 it was, found referenced on some stack overflow post.  There was i16 and ui16 as well, can't see i128 mentioned there though.

Richard.

>
>	Jakub

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Joseph Myers]

On Thu, 12 Jan 2017, Jakub Jelinek wrote:

> Well, we need some suffix that will not clash with C++ mandated udlits
> very soon, it is already bad that i and Q suffixes are problematic.
> By sN, did you mean s128 or literally sN?
> I've googled around a little bit and only found comments about
> 1ui64 and 1i64 suffixes, so that would mean accepting instead of l/L
> i128 or I128.

i128 and I128 seem reasonable as a user-visible C extension (they could of 
course be mixed in any order with u, U for unsigned, and with i, I, j, J 
for imaginary numbers).  I don't know what's appropriate for C++.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Thu, 12 Jan 2017, Richard Biener wrote:

> On Thu, 12 Jan 2017, Jakub Jelinek wrote:
> 
> > On Thu, Jan 12, 2017 at 01:35:32PM +0100, Richard Biener wrote:
> > > It also lacks expressiveness when comparing
> > > 
> > >   short s;
> > >   s_1 = (short) 1;
> > >   s_2 = short (1);
> > > 
> > > IMHO having a _Literal somewhere makes it more obvious what is meant
> > > (I'm still going to dump 1u or 1l instead of _Literal (unsigned) 1
> > > of course as that's even more easy to recognize).
> > 
> > So, with the _Literal (type) constant syntax, what are we going to emit
> > and parse for __int128 constants?
> > _Literal (__int128) (0x123456ULL << 64 || 0x123456ULL), something else?
> 
> Yes, unless we teach libcpp about larger than 64bit literals.
> 
> The following patch implements _Literal (but not proper dumping of
> > 64bit literals for __int128 nor parsing the above).
> 
> I think that's a good start which also covers pointer types nicely.
> For integers we might at some point want to adopt sN and uN suffixes
> which appearantly MSVC supports to handle native 128bit literals.
> 
> For
> 
> struct X { int a : 2; };
> void __GIMPLE ()
> foo (struct X * p)
> {
>   p->a = _Literal (int : 2) 1;
>   return;
> }
> 
> and thus 2 bit literals c_parser_type_name needs to be extended
> or for _Literal we can handle : 2 on our own.  But for loads
> from p->a into a temporary we need to handle that in declarations as well.
> 
> struct X { int a : 2; };
> int __GIMPLE ()
> foo (struct X * p)
> {
>   int : 2 a; // int a : 2;
>   a = p->a;
>   return a;
> }

I have bootstrapped and tested the patch below and installed it.  We
can add support for parsing vector and complex constants via the
same mechanism so having _Literal ( type ) x looks useful even if
we get another means of handling integer literals in the future.

Richard.

> 2017-01-12  Richard Biener  <rguenther@suse.de>
> 
> 	c/
> 	* gimple-parser.c (c_parser_gimple_postfix_expression): Parse
> 	_Literal ( type-name ) number.
> 
> 	* tree-pretty-print.c (dump_generic_node): Dump INTEGER_CSTs
> 	as _Literal ( type ) number in case usual suffixes do not
> 	preserve all information.
> 
> 	* gcc.dg/gimplefe-22.c: New testcase.
> 
> Index: gcc/c/gimple-parser.c
> ===================================================================
> --- gcc/c/gimple-parser.c	(revision 244350)
> +++ gcc/c/gimple-parser.c	(working copy)
> @@ -799,6 +799,32 @@ c_parser_gimple_postfix_expression (c_pa
>  				       type, ptr.value, alias_off);
>  	      break;
>  	    }
> +	  else if (strcmp (IDENTIFIER_POINTER (id), "_Literal") == 0)
> +	    {
> +	      /* _Literal '(' type-name ')' number  */
> +	      c_parser_consume_token (parser);
> +	      tree type = NULL_TREE;
> +	      if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
> +		{
> +		  struct c_type_name *type_name = c_parser_type_name (parser);
> +		  tree tem;
> +		  if (type_name)
> +		    type = groktypename (type_name, &tem, NULL);
> +		  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> +					     "expected %<)%>");
> +		}
> +	      tree val = c_parser_gimple_postfix_expression (parser).value;
> +	      if (! type
> +		  || ! val
> +		  || val == error_mark_node
> +		  || TREE_CODE (val) != INTEGER_CST)
> +		{
> +		  c_parser_error (parser, "invalid _Literal");
> +		  return expr;
> +		}
> +	      expr.value = fold_convert (type, val);
> +	      return expr;
> +	    }
>  	  /* SSA name.  */
>  	  unsigned version, ver_offset;
>  	  if (! lookup_name (id)
> Index: gcc/tree-pretty-print.c
> ===================================================================
> --- gcc/tree-pretty-print.c	(revision 244350)
> +++ gcc/tree-pretty-print.c	(working copy)
> @@ -1664,6 +1664,16 @@ dump_generic_node (pretty_printer *pp, t
>        break;
>  
>      case INTEGER_CST:
> +      if (flags & TDF_GIMPLE
> +	  && (POINTER_TYPE_P (TREE_TYPE (node))
> +	      || (TYPE_PRECISION (TREE_TYPE (node))
> +		  < TYPE_PRECISION (integer_type_node))
> +	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
> +	{
> +	  pp_string (pp, "_Literal (");
> +	  dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
> +	  pp_string (pp, ") ");
> +	}
>        if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE
>  	  && ! (flags & TDF_GIMPLE))
>  	{
> @@ -1693,11 +1703,7 @@ dump_generic_node (pretty_printer *pp, t
>        else if (tree_fits_shwi_p (node))
>  	pp_wide_integer (pp, tree_to_shwi (node));
>        else if (tree_fits_uhwi_p (node))
> -	{
> -	  pp_unsigned_wide_integer (pp, tree_to_uhwi (node));
> -	  if (flags & TDF_GIMPLE)
> -	    pp_character (pp, 'U');
> -	}
> +	pp_unsigned_wide_integer (pp, tree_to_uhwi (node));
>        else
>  	{
>  	  wide_int val = node;
> @@ -1710,6 +1716,24 @@ dump_generic_node (pretty_printer *pp, t
>  	  print_hex (val, pp_buffer (pp)->digit_buffer);
>  	  pp_string (pp, pp_buffer (pp)->digit_buffer);
>  	}
> +      if ((flags & TDF_GIMPLE)
> +	  && (POINTER_TYPE_P (TREE_TYPE (node))
> +	      || (TYPE_PRECISION (TREE_TYPE (node))
> +		  < TYPE_PRECISION (integer_type_node))
> +	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
> +	{
> +	  if (TYPE_UNSIGNED (TREE_TYPE (node)))
> +	    pp_character (pp, 'u');
> +	  if (TYPE_PRECISION (TREE_TYPE (node))
> +	      == TYPE_PRECISION (unsigned_type_node))
> +	    ;
> +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> +		   == TYPE_PRECISION (long_unsigned_type_node))
> +	    pp_character (pp, 'l');
> +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> +		   == TYPE_PRECISION (long_long_unsigned_type_node))
> +	    pp_string (pp, "ll");
> +	}
>        if (TREE_OVERFLOW (node))
>  	pp_string (pp, "(OVF)");
>        break;
> Index: gcc/testsuite/gcc.dg/gimplefe-22.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/gimplefe-22.c	(nonexistent)
> +++ gcc/testsuite/gcc.dg/gimplefe-22.c	(working copy)
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fgimple" } */
> +
> +void __GIMPLE ()
> +foo (short * p)
> +{
> +  *p = _Literal (short int) 1;
> +  return;
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Jakub Jelinek]

On Fri, Jan 13, 2017 at 09:15:22AM +0100, Richard Biener wrote:
> > @@ -1710,6 +1716,24 @@ dump_generic_node (pretty_printer *pp, t
> >  	  print_hex (val, pp_buffer (pp)->digit_buffer);
> >  	  pp_string (pp, pp_buffer (pp)->digit_buffer);
> >  	}
> > +      if ((flags & TDF_GIMPLE)
> > +	  && (POINTER_TYPE_P (TREE_TYPE (node))
> > +	      || (TYPE_PRECISION (TREE_TYPE (node))
> > +		  < TYPE_PRECISION (integer_type_node))
> > +	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
> > +	{
> > +	  if (TYPE_UNSIGNED (TREE_TYPE (node)))
> > +	    pp_character (pp, 'u');
> > +	  if (TYPE_PRECISION (TREE_TYPE (node))
> > +	      == TYPE_PRECISION (unsigned_type_node))
> > +	    ;
> > +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> > +		   == TYPE_PRECISION (long_unsigned_type_node))
> > +	    pp_character (pp, 'l');
> > +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> > +		   == TYPE_PRECISION (long_long_unsigned_type_node))
> > +	    pp_string (pp, "ll");
> > +	}

Not sure if I understand this.  The outer if condition says that only the
sub-int or strange precision or pointer types do that, but then
you compare the precisions against long and long long.  That will be
true only for pointers.  Don't you want the u/l/ll/ul/ull suffixes emitted
for integers even when they have normal precision and _Literal is not used?
Or is that handled somewhere else?

	Jakub

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Fri, 13 Jan 2017, Jakub Jelinek wrote:

> On Fri, Jan 13, 2017 at 09:15:22AM +0100, Richard Biener wrote:
> > > @@ -1710,6 +1716,24 @@ dump_generic_node (pretty_printer *pp, t
> > >  	  print_hex (val, pp_buffer (pp)->digit_buffer);
> > >  	  pp_string (pp, pp_buffer (pp)->digit_buffer);
> > >  	}
> > > +      if ((flags & TDF_GIMPLE)
> > > +	  && (POINTER_TYPE_P (TREE_TYPE (node))
> > > +	      || (TYPE_PRECISION (TREE_TYPE (node))
> > > +		  < TYPE_PRECISION (integer_type_node))
> > > +	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
> > > +	{
> > > +	  if (TYPE_UNSIGNED (TREE_TYPE (node)))
> > > +	    pp_character (pp, 'u');
> > > +	  if (TYPE_PRECISION (TREE_TYPE (node))
> > > +	      == TYPE_PRECISION (unsigned_type_node))
> > > +	    ;
> > > +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> > > +		   == TYPE_PRECISION (long_unsigned_type_node))
> > > +	    pp_character (pp, 'l');
> > > +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> > > +		   == TYPE_PRECISION (long_long_unsigned_type_node))
> > > +	    pp_string (pp, "ll");
> > > +	}
> 
> Not sure if I understand this.  The outer if condition says that only the
> sub-int or strange precision or pointer types do that, but then
> you compare the precisions against long and long long.  That will be
> true only for pointers.  Don't you want the u/l/ll/ul/ull suffixes emitted
> for integers even when they have normal precision and _Literal is not used?
> Or is that handled somewhere else?

Oops, you are right - it shouldn't be the same condition that controls
whether to add _Literal (type).  It should be the inverse.  Maybe
we need to add suffixes anyway even for say 61bit precision constants
to avoid warnings from libcpp.

I'll fix it up after testing a few cases.

Richard.

Re: [PATCH][GIMPLE FE] Add parsing of MEM_REFs

[Richard Biener]

On Fri, 13 Jan 2017, Richard Biener wrote:

> On Fri, 13 Jan 2017, Jakub Jelinek wrote:
> 
> > On Fri, Jan 13, 2017 at 09:15:22AM +0100, Richard Biener wrote:
> > > > @@ -1710,6 +1716,24 @@ dump_generic_node (pretty_printer *pp, t
> > > >  	  print_hex (val, pp_buffer (pp)->digit_buffer);
> > > >  	  pp_string (pp, pp_buffer (pp)->digit_buffer);
> > > >  	}
> > > > +      if ((flags & TDF_GIMPLE)
> > > > +	  && (POINTER_TYPE_P (TREE_TYPE (node))
> > > > +	      || (TYPE_PRECISION (TREE_TYPE (node))
> > > > +		  < TYPE_PRECISION (integer_type_node))
> > > > +	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
> > > > +	{
> > > > +	  if (TYPE_UNSIGNED (TREE_TYPE (node)))
> > > > +	    pp_character (pp, 'u');
> > > > +	  if (TYPE_PRECISION (TREE_TYPE (node))
> > > > +	      == TYPE_PRECISION (unsigned_type_node))
> > > > +	    ;
> > > > +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> > > > +		   == TYPE_PRECISION (long_unsigned_type_node))
> > > > +	    pp_character (pp, 'l');
> > > > +	  else if (TYPE_PRECISION (TREE_TYPE (node))
> > > > +		   == TYPE_PRECISION (long_long_unsigned_type_node))
> > > > +	    pp_string (pp, "ll");
> > > > +	}
> > 
> > Not sure if I understand this.  The outer if condition says that only the
> > sub-int or strange precision or pointer types do that, but then
> > you compare the precisions against long and long long.  That will be
> > true only for pointers.  Don't you want the u/l/ll/ul/ull suffixes emitted
> > for integers even when they have normal precision and _Literal is not used?
> > Or is that handled somewhere else?
> 
> Oops, you are right - it shouldn't be the same condition that controls
> whether to add _Literal (type).  It should be the inverse.  Maybe
> we need to add suffixes anyway even for say 61bit precision constants
> to avoid warnings from libcpp.
> 
> I'll fix it up after testing a few cases.

Fixed as follows, bootstrapped / tested on x86_64-unknown-linux-gnu
and committed with the GIMPLE_NOP parsing from the other patch.

Richard.

2017-01-13  Richard Biener  <rguenther@suse.de>

	* tree-pretty-print.c (dump_generic_node): Fix inverted condition
	for dumping GIMPLE INTEGER_CSTs.

Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c	(revision 244393)
+++ gcc/tree-pretty-print.c	(working copy)
@@ -1717,10 +1717,10 @@ dump_generic_node (pretty_printer *pp, t
 	  pp_string (pp, pp_buffer (pp)->digit_buffer);
 	}
       if ((flags & TDF_GIMPLE)
-	  && (POINTER_TYPE_P (TREE_TYPE (node))
-	      || (TYPE_PRECISION (TREE_TYPE (node))
-		  < TYPE_PRECISION (integer_type_node))
-	      || exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
+	  && ! (POINTER_TYPE_P (TREE_TYPE (node))
+		|| (TYPE_PRECISION (TREE_TYPE (node))
+		    < TYPE_PRECISION (integer_type_node))
+		|| exact_log2 (TYPE_PRECISION (TREE_TYPE (node))) == -1))
 	{
 	  if (TYPE_UNSIGNED (TREE_TYPE (node)))
 	    pp_character (pp, 'u');