| Message ID | 20110302174015.GR30899@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/03/2011 03:40 AM, Jakub Jelinek wrote: > Hi! > > The pr47283.C testcase was still failing on ia64-linux, there > the var in question was given a stack slot, but ICEs were caused by > non-canonical MEM_REF in MEM_EXPR of DEBUG_INSN MEM. In particular > there was MEM_REF <ADDR_EXPR <COMPONENT_REF <...> >, 0> on which aliasing > code ICEd. This patch tries to canonicalize it, and if the canonicalization > wasn't sufficient, uses just type and not the MEM_REF for > set_mem_attributes. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2011-03-02 Jakub Jelinek <jakub@redhat.com> > > PR debug/47283 > * cfgexpand.c (expand_debug_expr) <case MEM_REF>: If MEM_REF > first operand is not is_gimple_mem_ref_addr, try to fold it. > If the operand still isn't is_gimple_mem_ref_addr, pass just > its type to set_mem_attributes. > > --- gcc/cfgexpand.c.jj 2011-02-21 15:37:42.000000000 +0100 > +++ gcc/cfgexpand.c 2011-03-02 11:05:32.000000000 +0100 > @@ -2578,11 +2578,25 @@ expand_debug_expr (tree exp) > } > > case MEM_REF: > + if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0))) > + { > + tree newexp = fold_binary (MEM_REF, TREE_TYPE (exp), > + TREE_OPERAND (exp, 0), > + TREE_OPERAND (exp, 1)); > + if (newexp) > + exp = newexp; > + } > + /* FALLTHROUGH */ > case INDIRECT_REF: Do you really want to fallthru with newexp here? In theory it could have folded to anything. My guess is that you want to restart instead... r~
On Thu, Mar 03, 2011 at 03:31:55PM +1000, Richard Henderson wrote: > On 03/03/2011 03:40 AM, Jakub Jelinek wrote: > > case MEM_REF: > > + if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0))) > > + { > > + tree newexp = fold_binary (MEM_REF, TREE_TYPE (exp), > > + TREE_OPERAND (exp, 0), > > + TREE_OPERAND (exp, 1)); > > + if (newexp) > > + exp = newexp; > > + } > > + /* FALLTHROUGH */ > > case INDIRECT_REF: > > Do you really want to fallthru with newexp here? > In theory it could have folded to anything. My guess is that > you want to restart instead... fold_binary only optimizes: /* MEM[&MEM[p, CST1], CST2] -> MEM[p, CST1 + CST2]. */ and /* MEM[&a.b, CST2] -> MEM[&a, offsetof (a, b) + CST2]. */ But if you prefer, I can certainly do there if (newexp) return expand_debug_expr (newexp); instead. Jakub
On Thu, Mar 3, 2011 at 8:22 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Mar 03, 2011 at 03:31:55PM +1000, Richard Henderson wrote: >> On 03/03/2011 03:40 AM, Jakub Jelinek wrote: >> > case MEM_REF: >> > + if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0))) >> > + { >> > + tree newexp = fold_binary (MEM_REF, TREE_TYPE (exp), >> > + TREE_OPERAND (exp, 0), >> > + TREE_OPERAND (exp, 1)); >> > + if (newexp) >> > + exp = newexp; >> > + } >> > + /* FALLTHROUGH */ >> > case INDIRECT_REF: >> >> Do you really want to fallthru with newexp here? >> In theory it could have folded to anything. My guess is that >> you want to restart instead... > > fold_binary only optimizes: > /* MEM[&MEM[p, CST1], CST2] -> MEM[p, CST1 + CST2]. */ > and > /* MEM[&a.b, CST2] -> MEM[&a, offsetof (a, b) + CST2]. */ > > But if you prefer, I can certainly do there > if (newexp) > return expand_debug_expr (newexp); > instead. It's probably more safe, at least for 4.7, in case we ever start to do constant folding here. Btw, I don't really like another caller of set_mem_attributes that passes a type rather than an expression ... OTOH, the folding should never fail to generate a canonical MEM ref (I bet asserting that the MEM_REF is a proper one after folding passes bootstrap just ok), so maybe just drop the debug expr on the floor in this case (eventually ICE when checking is enabled)? At least I can't see how we could end up with a non-canonicalizable address. Richard.
--- gcc/cfgexpand.c.jj 2011-02-21 15:37:42.000000000 +0100 +++ gcc/cfgexpand.c 2011-03-02 11:05:32.000000000 +0100 @@ -2578,11 +2578,25 @@ expand_debug_expr (tree exp) } case MEM_REF: + if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0))) + { + tree newexp = fold_binary (MEM_REF, TREE_TYPE (exp), + TREE_OPERAND (exp, 0), + TREE_OPERAND (exp, 1)); + if (newexp) + exp = newexp; + } + /* FALLTHROUGH */ case INDIRECT_REF: op0 = expand_debug_expr (TREE_OPERAND (exp, 0)); if (!op0) return NULL; + if (POINTER_TYPE_P (TREE_TYPE (exp))) + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); + else + as = ADDR_SPACE_GENERIC; + if (TREE_CODE (exp) == MEM_REF) { if (GET_CODE (op0) == DEBUG_IMPLICIT_PTR @@ -2597,12 +2611,10 @@ expand_debug_expr (tree exp) return NULL; op0 = plus_constant (op0, INTVAL (op1)); - } - if (POINTER_TYPE_P (TREE_TYPE (exp))) - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); - else - as = ADDR_SPACE_GENERIC; + if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0))) + exp = TREE_TYPE (exp); + } op0 = convert_debug_memory_address (targetm.addr_space.address_mode (as), op0, as);