Hi Eric,
On Fri, 27 Sep 2013 10:36:57, Eric Botcazou wrote: > >> Sure, but the modifier is not meant to force something into memory, >> especially when it is already in an register. Remember, we are only >> talking of structures here, and we only want to access one member. >> >> It is more the other way round: >> It says: "You do not have to load the value in a register, if it is already >> in memory I'm happy" > > EXPAND_MEMORY means we are interested in a memory result, even if > the memory is constant and we could have propagated a constant value. */ > > We definitely want to propagate constant values here, look at the code below. > And it already lists explicit cases where we really need to splill to memory. > > -- > Eric Botcazou I'd like to continue this discussion by proposing this updated patch. I have now added a new value "EXPAND_REFERENCE" to the expand_modifier enumeration. It is almost like EXPAND_MEMORY but it does not interfere with constant values. I have done the same modification to VIEW_CONVERT_EXPR too, because this is a possible inner reference, itself. It is however inherently hard to test around this code. To understand this patch it is good to know what type of object the return value "tem" of get_inner_reference can be. From the program logic at get_inner_reference it is clear that the return value may *not* be BIT_FIELD_REF, COMPONENT_REF, ARRAY_REF, ARRAY_RANGE_REF, REALPART_EXPR, IMAGPART_EXPR. The result may be VIEW_CONVERT_EXPR only on a STRICT_ALIGNMENT target. This is probably further restricted because exp is gimplified. Usually the result will be a MEM_REF or a SSA_NAME of the memory where the structure is to be found. When you look at where EXPAND_MEMORY is handled you see it is special-cased in TARGET_MEM_REF, MEM_REF, ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF, ARRAY_RANGE_REF. At TARGET_MEM_REF, MEM_REF, VIEW_CONVERT_EXPR, it should be the same if EXPAND_MEMORY, EXPAND_WRITE or EXPAND_REFERENCE is given: If it is an unaligned memory, we just return the unaligned reference. This was missing for VIEW_CONVERT_EXPR, and unfortunately I have no test case, because it is only a problem for STRICT_ALIGNMENT targets, and even there it will certainly be really hard to find test cases that exercise this code. In ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF, ARRAY_RANGE_REF we do not have to touch the handling of the outer modifier. However we pass EXPAND_REFERENCE to the inner object, which should not be a recursive use of any ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF, ARRAY_RANGE_REF. TARGET_MEM_REF, MEM_REF and VIEW_CONVERT_EXPR know how to handle EXPAND_REFERENCE, anything else handles it like EXPAND_NORMAL. OK, what do you think of it now? Thanks Bernd.
2013-10-03 Bernd Edlinger <bernd.edlin...@hotmail.de> PR middle-end/57748 * expr.h (expand_modifier): New enum value EXPAND_REFERENCE. * expr.c (expand_expr_real_1): Use EXAND_REFERENCE on base object. testsuite/ PR middle-end/57748 * gcc.dg/torture/pr57748-3.c: New test.
patch-pr57748-2.diff
Description: Binary data