Message ID | alpine.LSU.2.20.1709041617300.14191@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
Series | Handle wide-chars in native_encode_string | expand |
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.)
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;
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.
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; }