[PATCH] Handle wide-chars in native_encode_string

[PATCH] Handle wide-chars in native_encode_string

[Richard Biener]

The following patch cures the missed optimization in PR82084, vectorizing
a wide-char initializer

  wchar_t strs[4][2]= {  L"A", L"B", L"C" , L"D"};

with AVX to

  MEM[(wchar_t[2] *)&strs] = { 65, 66, 67, 68 };

it's not entirely clear to me whether the individual STRING_CSTs we
build for the gimplified code

        strs[0] = "A";
        strs[1] = "B";
        strs[2] = "C";
        strs[3] = "D";

always have a consistend "character" size and how the individual
"characters" are encoded.  The patch assumes that the array element
type of the STRING_CST can be used to get access to individual
characters by means of the element type size and those elements
are stored in host byteorder.  Which means the patch simply handles
16bit and 32bit "characters" as 16bit and 32bit integers and encodes
them with the same rules as such integers.

Joseph, are there more considerations for encoding the target
representation of STRING_CSTs?

Looks I was too lazy to lookup answers to those questions from
the RTL expansion code which hopefully outputs constant initializers
properly.

Apart from vectorization in the mentioned testcase we also gain
constant folding from pices from this change (but I don't adjust
fold_read_from_constant_string yet).

Thanks,
Richard.

2017-09-04  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/82084
	* fold-const.c (native_encode_string): Handle wide characters.
	(can_native_encode_string_p): Likewise.

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 251661)
+++ gcc/fold-const.c	(working copy)
@@ -7187,26 +7187,71 @@ native_encode_string (const_tree expr, u
   if (! can_native_encode_string_p (expr))
     return 0;
 
-  HOST_WIDE_INT total_bytes = tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE (expr)));
+  tree type = TREE_TYPE (expr);
+  HOST_WIDE_INT total_bytes = tree_to_shwi (TYPE_SIZE_UNIT (type));
+  int orig_off = off;
   if ((off == -1 && total_bytes > len)
       || off >= total_bytes)
     return 0;
   if (off == -1)
     off = 0;
-  if (TREE_STRING_LENGTH (expr) - off < MIN (total_bytes, len))
+
+  HOST_WIDE_INT elsz = tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
+  if (elsz == 1)
     {
-      int written = 0;
-      if (off < TREE_STRING_LENGTH (expr))
+      if (TREE_STRING_LENGTH (expr) - off < MIN (total_bytes, len))
 	{
-	  written = MIN (len, TREE_STRING_LENGTH (expr) - off);
-	  memcpy (ptr, TREE_STRING_POINTER (expr) + off, written);
+	  int written = 0;
+	  if (off < TREE_STRING_LENGTH (expr))
+	    {
+	      written = MIN (len, TREE_STRING_LENGTH (expr) - off);
+	      memcpy (ptr, TREE_STRING_POINTER (expr) + off, written);
+	    }
+	  memset (ptr + written, 0,
+		  MIN (total_bytes - written, len - written));
 	}
-      memset (ptr + written, 0,
-	      MIN (total_bytes - written, len - written));
+      else
+	memcpy (ptr, TREE_STRING_POINTER (expr) + off, MIN (total_bytes, len));
+      return MIN (total_bytes - off, len);
     }
   else
-    memcpy (ptr, TREE_STRING_POINTER (expr) + off, MIN (total_bytes, len));
-  return MIN (total_bytes - off, len);
+    {
+      tree ielt = build_nonstandard_integer_type (elsz * 8, true);
+      int offset = 0;
+      bool first = true;
+      for (int o = off & ~(elsz - 1); o < total_bytes; o += elsz)
+	{
+	  unsigned HOST_WIDE_INT c;
+	  switch (elsz)
+	    {
+	    case 2:
+	      {
+		uint16_t s;
+		memcpy (&s, TREE_STRING_POINTER (expr) + o, 2);
+		c = s;
+		break;
+	      }
+	    case 4:
+	      {
+		uint32_t i;
+		memcpy (&i, TREE_STRING_POINTER (expr) + o, 4);
+		c = i;
+		break;
+	      }
+	    default:
+	      gcc_unreachable ();
+	    }
+	  tree elem = build_int_cstu (ielt, c);
+	  int res = native_encode_expr (elem, ptr+offset, len-offset,
+					first ? off & (elsz - 1) : 0);
+	  if ((orig_off == -1 && res != elsz)
+	      || res == 0)
+	    return 0;
+	  offset += res;
+	  first = false;
+	}
+      return offset;
+    }
 }
 
 
@@ -7491,10 +7536,11 @@ can_native_encode_string_p (const_tree e
 
   if (TREE_CODE (type) != ARRAY_TYPE
       || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
-      || (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (type)))
-	  != BITS_PER_UNIT)
       || !tree_fits_shwi_p (TYPE_SIZE_UNIT (type)))
     return false;
+  HOST_WIDE_INT elsz = tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
+  if (elsz != 1 && elsz != 2 && elsz != 4)
+    return false;
   return true;
 }
 

Re: [PATCH] Handle wide-chars in native_encode_string

[Joseph Myers]

On Mon, 4 Sep 2017, Richard Biener wrote:

> always have a consistend "character" size and how the individual
> "characters" are encoded.  The patch assumes that the array element
> type of the STRING_CST can be used to get access to individual
> characters by means of the element type size and those elements
> are stored in host byteorder.  Which means the patch simply handles

It's actually target byte order, i.e. the STRING_CST stores the same 
sequence of target bytes as would appear on the target system (modulo 
certain strings such as in asm statements and attributes, for which 
translation to the execution character set is disabled because those 
strings are only processed in the compiler on the host, not on the target 
- but you should never encounter such strings in the optimizers etc.).  
This is documented in generic.texi (complete with a warning about how it's 
not well-defined what the encoding is if target bytes are not the same as 
host bytes).

I suspect that, generically in the compiler, the use of C++ might make it 
easier than it would have been some time ago to build some abstractions 
around target strings that work for all of narrow strings, wide strings, 
char16_t strings etc. (for extracting individual elements - or individual 
characters which might be multibyte characters in the narrow string case, 
etc.) - as would be useful for e.g. wide string format checking and more 
generally for making e.g. optimizations for narrow strings also work for 
wide strings.  (Such abstractions wouldn't solve the question of what the 
format is if host and target bytes differ, but their use would reduce the 
number of places needing changing to establish a definition of the format 
in that case if someone were to do a port to a system with bytes bigger 
than 8 bits.)

However, as I understand the place you're patching, it doesn't have any 
use for such an abstraction; it just needs to copy a sequence of bytes 
from one place to another.  (And even with host bytes different from 
target bytes, clearly it would make sense to define the internal 
interfaces to make the encodings consistent so this function still only 
needs to copy bytes from one place to another and still doesn't need such 
abstractions.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Handle wide-chars in native_encode_string

[Richard Biener]

On Mon, 4 Sep 2017, Joseph Myers wrote:

> On Mon, 4 Sep 2017, Richard Biener wrote:
> 
> > always have a consistend "character" size and how the individual
> > "characters" are encoded.  The patch assumes that the array element
> > type of the STRING_CST can be used to get access to individual
> > characters by means of the element type size and those elements
> > are stored in host byteorder.  Which means the patch simply handles
> 
> It's actually target byte order, i.e. the STRING_CST stores the same 
> sequence of target bytes as would appear on the target system (modulo 
> certain strings such as in asm statements and attributes, for which 
> translation to the execution character set is disabled because those 
> strings are only processed in the compiler on the host, not on the target 
> - but you should never encounter such strings in the optimizers etc.).  
> This is documented in generic.texi (complete with a warning about how it's 
> not well-defined what the encoding is if target bytes are not the same as 
> host bytes).

Ah thanks.

> I suspect that, generically in the compiler, the use of C++ might make it 
> easier than it would have been some time ago to build some abstractions 
> around target strings that work for all of narrow strings, wide strings, 
> char16_t strings etc. (for extracting individual elements - or individual 
> characters which might be multibyte characters in the narrow string case, 
> etc.) - as would be useful for e.g. wide string format checking and more 
> generally for making e.g. optimizations for narrow strings also work for 
> wide strings.  (Such abstractions wouldn't solve the question of what the 
> format is if host and target bytes differ, but their use would reduce the 
> number of places needing changing to establish a definition of the format 
> in that case if someone were to do a port to a system with bytes bigger 
> than 8 bits.)
> 
> However, as I understand the place you're patching, it doesn't have any 
> use for such an abstraction; it just needs to copy a sequence of bytes 
> from one place to another.  (And even with host bytes different from 
> target bytes, clearly it would make sense to define the internal 
> interfaces to make the encodings consistent so this function still only 
> needs to copy bytes from one place to another and still doesn't need such 
> abstractions.)

Right.  Given they are in target representation the patch becomes much
simpler and we can handle all STRING_CSTs modulo for the case where
BITS_PER_UNIT != CHAR_BIT (as you say).  I suppose we can easily
declare we'll never support a CHAR_BIT != 8 host and we currently
don't have any BITS_PER_UNIT != 8 port (we had c4x).  I'm not
sure what constraints we have on CHAR_TYPE_SIZE vs. BITS_PER_UNIT,
or for what port it would make sense to have differing values.
Or what it means for native encoding (should the BITS_PER_UNIT != CHAR_BIT
test be CHAR_TYPE_SIZE != CHAR_BIT instead?).  BITS_PER_UNIT is
also only documented in rtl.texi rather than in tm.texi.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2017-09-05  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/82084
	* fold-const.c (can_native_encode_string_p): Handle wide characters.

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 251661)
+++ gcc/fold-const.c	(working copy)
@@ -7489,10 +7489,11 @@ can_native_encode_string_p (const_tree e
 {
   tree type = TREE_TYPE (expr);
 
-  if (TREE_CODE (type) != ARRAY_TYPE
+  /* Wide-char strings are encoded in target byte-order so native
+     encoding them is trivial.  */
+  if (BITS_PER_UNIT != CHAR_BIT
+      || TREE_CODE (type) != ARRAY_TYPE
       || TREE_CODE (TREE_TYPE (type)) != INTEGER_TYPE
-      || (GET_MODE_BITSIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (type)))
-	  != BITS_PER_UNIT)
       || !tree_fits_shwi_p (TYPE_SIZE_UNIT (type)))
     return false;
   return true;

Re: [PATCH] Handle wide-chars in native_encode_string

[Joseph Myers]

On Tue, 5 Sep 2017, Richard Biener wrote:

> don't have any BITS_PER_UNIT != 8 port (we had c4x).  I'm not
> sure what constraints we have on CHAR_TYPE_SIZE vs. BITS_PER_UNIT,
> or for what port it would make sense to have differing values.

BITS_PER_UNIT = size of QImode = unit that target hardware addresses count 
in.

CHAR_TYPE_SIZE = size of target char in the C ABI.

sizeof (char) is always 1 by definition, but in principle you could have 
an architecture where the addressable unit at the hardware level is 
smaller than C char.  CHAR_TYPE_SIZE must always be a multiple of 
BITS_PER_UNIT, and CHAR_TYPE_SIZE != BITS_PER_UNIT is probably even more 
bitrotten (I don't know if we've ever had such a port) than BITS_PER_UNIT 
!= 8.

-- 
Joseph S. Myers
joseph@codesourcery.com