On Fri, Oct 16, 2015 at 9:48 AM, Eric Botcazou <ebotca...@adacore.com> wrote: >> Btw, would be really nice to have libbacktrace support for ada ... > > OK, I'll keep that in mind. > >> While the patch looks technically ok I think you'll run into the same issue >> with a non-zero offset MEM_REF as that will get you a POINTER_PLUS_EXPR >> from build_fold_addr_expr. We might be lucky not to ICE in >> recompute_tree_invariant_for_addr_expr because we can access operand >> zero of that of course. I think recompute_tree_invariant_for_addr_expr >> misses an assert that it receives an ADDR_EXPR and the gimplify.c >> caller would need to handle POINTER_PLUS_EXPR specially. >> >> Or change your patch to also handle non-zero offset MEM_REFs by >> simply gimplifying to POINTER_PLUS_EXPR op0, op1. > > I couldn't cover the new case though, because you need a record with variable > size and an array of those yields a non-constant offset so no MEM_REF and a > record with fixed offset doesn't yield a MEM_REF either for some reason... > > But I can add the assert in recompute_tree_invariant_for_addr_expr:
Sure, if that works it's pre-approved. Your original patch is also ok (though I still think it's incomplete - but we'll wait until a testcase comes up with the assert). Thanks, Richard. > Index: tree.c > =================================================================== > --- tree.c (revision 228794) > +++ tree.c (working copy) > @@ -4248,6 +4248,8 @@ recompute_tree_invariant_for_addr_expr ( > tree node; > bool tc = true, se = false; > > + gcc_assert (TREE_CODE (t) == ADDR_EXPR); > + > /* We started out assuming this address is both invariant and constant, but > does not have side effects. Now go down any handled components and see if > any of them involve offsets that are either non-constant or non-invariant. > > -- > Eric Botcazou
