diff mbox series

c-family: Use TYPE_OVERFLOW_UNDEFINED instead of !TYPE_UNSIGNED in pointer_sum [PR95903]

Message ID 20200627085856.GA8462@tucnak
State New
Headers show
Series c-family: Use TYPE_OVERFLOW_UNDEFINED instead of !TYPE_UNSIGNED in pointer_sum [PR95903] | expand

Commit Message

Jakub Jelinek June 27, 2020, 8:58 a.m. UTC
Hi!

For lp64 targets and int off ... ptr[off + 1]
is lowered in pointer_sum to *(ptr + ((sizetype) off + (sizetype) 1)).
That is fine when signed integer wrapping is undefined (and is not done
already if off has unsigned type), but changes behavior for -fwrapv, where
overflow is well defined.  Runtime test could be:
int
main ()
{
  char *p = __builtin_malloc (0x100000000UL);
  if (!p) return 0;
  char *q = p + 0x80000000UL;
  int o = __INT_MAX__;
  q[o + 1] = 1;
  if (q[-__INT_MAX__ - 1] != 1) __builtin_abort ();
  return 0;
}
with -fwrapv or so, not included in the testsuite because it requires 4GB
allocation (with some other test it would be enough to have something
slightly above 2GB, but still...).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

On the other side, it would be nice with undefined signed integer overflow
to optimize
extern signed char arr[];
int foo (int off) { off++; return arr[off]; }
at least on targets that have addressing that can include both index and
some immediate, right now we emit
	addl	$1, %edi
	movslq	%edi, %rdi
	movsbl	arr(%rdi), %eax
but we could emit
	movslq	%edi, %rdi
	movsbl	arr+1(%rdi), %eax
Not sure where to do that; after expansion it is too late, because signed
vs. unsigned addition is lost at that point, so maybe the isel pass or some
other late pass?  Wonder if ivopts is able to do that, but ivopts doesn't
work outside of loops, right?

2020-06-27  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/95903
	* c-common.c (pointer_int_sum): Use TYPE_OVERFLOW_UNDEFINED instead of
	!TYPE_UNSIGNED check to see if we can apply distributive law and handle
	smaller precision intop operands separately.

	* c-c++-common/pr95903.c: New test.


	Jakub

Comments

Richard Biener June 27, 2020, 10:12 a.m. UTC | #1
On June 27, 2020 10:58:56 AM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>For lp64 targets and int off ... ptr[off + 1]
>is lowered in pointer_sum to *(ptr + ((sizetype) off + (sizetype) 1)).
>That is fine when signed integer wrapping is undefined (and is not done
>already if off has unsigned type), but changes behavior for -fwrapv,
>where
>overflow is well defined.  Runtime test could be:
>int
>main ()
>{
>  char *p = __builtin_malloc (0x100000000UL);
>  if (!p) return 0;
>  char *q = p + 0x80000000UL;
>  int o = __INT_MAX__;
>  q[o + 1] = 1;
>  if (q[-__INT_MAX__ - 1] != 1) __builtin_abort ();
>  return 0;
>}
>with -fwrapv or so, not included in the testsuite because it requires
>4GB
>allocation (with some other test it would be enough to have something
>slightly above 2GB, but still...).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>On the other side, it would be nice with undefined signed integer
>overflow
>to optimize
>extern signed char arr[];
>int foo (int off) { off++; return arr[off]; }
>at least on targets that have addressing that can include both index
>and
>some immediate, right now we emit
>	addl	$1, %edi
>	movslq	%edi, %rdi
>	movsbl	arr(%rdi), %eax
>but we could emit
>	movslq	%edi, %rdi
>	movsbl	arr+1(%rdi), %eax
>Not sure where to do that; after expansion it is too late, because
>signed
>vs. unsigned addition is lost at that point, so maybe the isel pass or
>some
>other late pass?  Wonder if ivopts is able to do that, but ivopts
>doesn't
>work outside of loops, right?

Right. SLSR was supposed to eventually handle this. 

Richard. 

>2020-06-27  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/95903
>	* c-common.c (pointer_int_sum): Use TYPE_OVERFLOW_UNDEFINED instead of
>	!TYPE_UNSIGNED check to see if we can apply distributive law and
>handle
>	smaller precision intop operands separately.
>
>	* c-c++-common/pr95903.c: New test.
>
>--- gcc/c-family/c-common.c.jj	2020-06-18 11:26:09.522400264 +0200
>+++ gcc/c-family/c-common.c	2020-06-26 10:28:10.385144734 +0200
>@@ -3141,7 +3141,7 @@ pointer_int_sum (location_t loc, enum tr
>     /* If the constant is unsigned, and smaller than the pointer size,
> 	 then we must skip this optimization.  This is because it could cause
> 	 an overflow error if the constant is negative but INTOP is not.  */
>-      && (!TYPE_UNSIGNED (TREE_TYPE (intop))
>+      && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (intop))
> 	  || (TYPE_PRECISION (TREE_TYPE (intop))
> 	      == TYPE_PRECISION (TREE_TYPE (ptrop)))))
>     {
>--- gcc/testsuite/c-c++-common/pr95903.c.jj	2020-06-26
>10:43:19.325937680 +0200
>+++ gcc/testsuite/c-c++-common/pr95903.c	2020-06-26 10:47:24.017382649
>+0200
>@@ -0,0 +1,19 @@
>+/* PR middle-end/95903 */
>+/* { dg-do compile { target lp64 } } */
>+/* { dg-options "-O2 -fwrapv -fdump-tree-optimized" } */
>+/* Verify that for -fwrapv the + 1 addition is performed in the
>parameter's
>+   type before sign extending it.  */
>+/* { dg-final { scan-tree-dump-times "off_\[0-9]+\\\(D\\\) \\+ 1" 2
>"optimized" } } */
>+
>+char
>+foo (const char *ptr, int off)
>+{
>+  off += 1;
>+  return ptr[off];
>+}
>+
>+char
>+bar (const char *ptr, int off)
>+{
>+  return ptr[off + 1];
>+}
>
>	Jakub
diff mbox series

Patch

--- gcc/c-family/c-common.c.jj	2020-06-18 11:26:09.522400264 +0200
+++ gcc/c-family/c-common.c	2020-06-26 10:28:10.385144734 +0200
@@ -3141,7 +3141,7 @@  pointer_int_sum (location_t loc, enum tr
       /* If the constant is unsigned, and smaller than the pointer size,
 	 then we must skip this optimization.  This is because it could cause
 	 an overflow error if the constant is negative but INTOP is not.  */
-      && (!TYPE_UNSIGNED (TREE_TYPE (intop))
+      && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (intop))
 	  || (TYPE_PRECISION (TREE_TYPE (intop))
 	      == TYPE_PRECISION (TREE_TYPE (ptrop)))))
     {
--- gcc/testsuite/c-c++-common/pr95903.c.jj	2020-06-26 10:43:19.325937680 +0200
+++ gcc/testsuite/c-c++-common/pr95903.c	2020-06-26 10:47:24.017382649 +0200
@@ -0,0 +1,19 @@ 
+/* PR middle-end/95903 */
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -fwrapv -fdump-tree-optimized" } */
+/* Verify that for -fwrapv the + 1 addition is performed in the parameter's
+   type before sign extending it.  */
+/* { dg-final { scan-tree-dump-times "off_\[0-9]+\\\(D\\\) \\+ 1" 2 "optimized" } } */
+
+char
+foo (const char *ptr, int off)
+{
+  off += 1;
+  return ptr[off];
+}
+
+char
+bar (const char *ptr, int off)
+{
+  return ptr[off + 1];
+}