On Thu, Jun 05, 2014 at 09:46:25AM +0200, Richard Biener wrote:
> On Wed, Jun 4, 2014 at 10:06 PM, Senthil Kumar Selvaraj
> <senthil_kumar.selva...@atmel.com> wrote:
> > For the AVR target, assertions in convert_debug_memory_address cause a
> > couple of ICEs (see PR 52472). Jakub suggested returning a NULL rtx,
> > which works, but on debugging further, I found that expand_debug_expr
> > appears to incorrectly compute the address space for ADDR_EXPR and
> > MEM_REFs.
> >
> > For ADDR_EXPR, TREE_TYPE(exp) is a POINTER_TYPE (always?), but in
> > the generic address space, even if the object whose address is taken
> > is in a different address space. expand_debug_expr takes
> > TYPE_ADDR_SPACE(TREE_TYPE(exp)) and therefore computes the address
> > space as generic. convert_debug_memory_address then asserts that the
> > mode is a valid pointer mode in the address space and fails.
> >
> > Similarly, for MEM_REFs, TREE_TYPE(exp) is the type of the
> > dereferenced value, and therefore checking if it is a POINTER_TYPE
> > doesn't help for a single pointer dereference. The address space
> > gets computed as generic even if the pointer points to a different
> > address space. The address mode for the generic address space is
> > passed to convert_debug_memory_address, and the assertion that that mode
> > must match the mode of the rtx then fails.
> >
> > The below patch attempts to fix this by picking the right TREE_TYPE to
> > pass to TYPE_ADDR_SPACE for MEM_REF (use type of arg 0) and
> > ADDR_EXPR (check for pointer type and look at nested addr space).
> >
> > Does this look reasonable or did I get it all wrong?
> >
> > Regards
> > Senthil
> >
> > diff --git gcc/cfgexpand.c gcc/cfgexpand.c
> > index 8b0e466..ca78953 100644
> > --- gcc/cfgexpand.c
> > +++ gcc/cfgexpand.c
> > @@ -3941,8 +3941,8 @@ expand_debug_expr (tree exp)
> > op0 = plus_constant (inner_mode, op0, INTVAL (op1));
> > }
> >
> > - if (POINTER_TYPE_P (TREE_TYPE (exp)))
> > - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp)));
> > + if (POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0))))
> > + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp,
> > 0))));
> > else
> > as = ADDR_SPACE_GENERIC;
>
> TREE_OPERAND (exp, 0) always has pointer type so I'd change this to
>
> as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0))));
>
> > @@ -4467,7 +4467,11 @@ expand_debug_expr (tree exp)
> > return NULL;
> > }
> >
> > - as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
> > + if (POINTER_TYPE_P (TREE_TYPE (exp)))
> > + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp)));
> > + else
> > + as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
> > +
>
> Likewise - TREE_TYPE (exp) is always a pointer type. Otherwise the
> patch looks ok to me.
>
> Richard.
>
> > op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as);
> >
> > return op0;
Modified patch attached. This fixes PR 52472 (the ADDR_EXPR case) as well
as a couple of ICEs in gcc.target/avr/torture/addr-space-2-x.c (MEM_REF
case). I've also added a new testcase for the PR.
I don't have commit access - could someone apply this for me please?
It'd be great if this was backported to 4.9 and 4.8 branches as well.
Regards
Senthil
gcc/ChangeLog
2014-06-05 Senthil Kumar Selvaraj <senthil_kumar.selva...@atmel.com>
PR target/52472
* cfgexpand.c (expand_debug_expr): Use address space of nested
TREE_TYPE for ADDR_EXPR and MEM_REF.
gcc/testsuite/ChangeLog
2014-06-05 Senthil Kumar Selvaraj <senthil_kumar.selva...@atmel.com>
PR target/52472
* gcc.target/avr/pr52472.c: New test.
diff --git gcc/cfgexpand.c gcc/cfgexpand.c
index 8b0e466..e161cb7 100644
--- gcc/cfgexpand.c
+++ gcc/cfgexpand.c
@@ -3941,10 +3941,7 @@ expand_debug_expr (tree exp)
op0 = plus_constant (inner_mode, op0, INTVAL (op1));
}
- if (POINTER_TYPE_P (TREE_TYPE (exp)))
- as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp)));
- else
- as = ADDR_SPACE_GENERIC;
+ as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0))));
op0 = convert_debug_memory_address (targetm.addr_space.address_mode (as),
op0, as);
@@ -4467,7 +4464,7 @@ expand_debug_expr (tree exp)
return NULL;
}
- as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
+ as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp)));
op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as);
return op0;
diff --git gcc/testsuite/gcc.target/avr/pr52472.c
gcc/testsuite/gcc.target/avr/pr52472.c
new file mode 100644
index 0000000..701cfb4
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr52472.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -g -Wno-pointer-to-int-cast" } */
+
+/* This testcase exposes PR52472. expand_debug_expr mistakenly
+ considers the address space of data to be generic, and
+ asserts that PSImode pointers aren't valid in the generic
+ address space. */
+
+extern const __memx unsigned data[][10];
+
+unsigned long ice (void)
+{
+ unsigned long addr32;
+
+ return addr32 = ((unsigned long) data);
+}
