On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
<bernd.edlin...@hotmail.de> wrote:
> Hi,
>
>> Eh ... even
>>
>> register struct { int i; int a[0]; } asm ("ebx");
>>
>> works. Also with int a[1] but not with a[2]. So just handling trailing
>> arrays makes this case regress as well.
>>
>> Now I'd call this use questionable ... but we've likely supported that
>> for decades and cannot change that now.
>>
>> Back to fixing everything in expand.
>>
>> Richard.
>>
>
> Ok, finally you asked for it.
>
> Here is my previous version of that patch again.
>
> 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.
>
>
> Boot-strapped and regression-tested on x86_64-linux-gnu
> OK for trunk?
You point to a weak spot in expansion - that it handles creating
the base MEM to offset with handled components by recursing
into the case that handles bare MEM_REFs. This makes the
bare MEM_REF handling code somewhat awkward (it's the
one to assign mem-attrs which are later adjusted for example).
Maybe a better appropach than adding yet another expand
modifier would be to split out the "base MEM" expansion part
out of the bare MEM_REF handling code so we can call that
instead of recursing.
In this light - instead of a new expand modifier don't you want
an actual flag that specifies we are coming from a call that
wants to expand a base? That is, allow EXPAND_SUM
but with the recursion flag set?
Finally I think the recursion into the VIEW_CONVERT_EXPR case
is only there because of the keep_aligning flag of get_inner_reference
which should be obsolete now that we properly handle its effects
in get_object_alignment. So you wouldn't need to adjust this path
if we finally can get rid of that.
What do others think?
Thanks,
Richard.
> Thanks
> Bernd.
