On Fri, 5 Jun 2020, Eric Botcazou wrote: > > The patch ends up recording the whole Array ref with variable index. All > > alias analysis code deals with this just fine. IIRC historically we tried > > to save memory with stripping and dropping of MEM_EXPRs. > > OK, I agree that the cleanup makes sense these days and can probably also be > backported onto the 10 branch without too much risk.
I've installed it on trunk but will give it quite a while there before backporting. I'm still somewhat worried about the /* ??? If we end up with a constant or a descriptor do not record a MEM_EXPR. */ else if (CONSTANT_CLASS_P (t) || TREE_CODE (t) == CONSTRUCTOR) ; case, maybe we should assert that bitpos == 0 here since we're dropping it on the floor but eventually inherit offset_known_p == true from the already present MEM_ATTRs. I guess set_mem_attributes_minus_bitpos should be an all-or-nothing with regard to the inherited MEM_ATTRs - take them unmodified or override them completely. But even then - can we simply ignore the passed in bitpos? shall we at least set offset_known_p to false if we do? We're also happily ignoring an inherited attrs.offset. Still this all suggests the take it all or nothing approach. Also for the one case I've seen (through expand_debug_expr though - "semantically" irreleveant thus) where we end up being called for a MEM that was generated for a completely different object, for example a stack slot, but we'll still apply things like alignment analysis on the passed in tree which might be unrelated enough to make the derived alignment invalid for the actual MEM. Richard.