On November 5, 2015 7:55:12 PM GMT+01:00, Alan Lawrence <alan.lawre...@arm.com>
wrote:
>On 3 November 2015 at 14:01, Richard Biener
><richard.guent...@gmail.com> wrote:
>>
>> Hum. I still wonder why we need all this complication ...
>
>Well, certainly I'd love to make it simpler, and if the complication
>is because I've gone about trying to deal with especially Ada in the
>wrong way...
>
>> I would
>> expect that if
>> we simply decided to completely scalarize a constant pool aggregate
>load then
>> we can always do that. SRA should then simply emit the _same_ IL as
>it does
>> for other complete scalarization element accesses. The only
>difference is
>> that the result should be simplifiable to omit the element load from
>> the constant pool.
>>
>> 1) The FRE pass following SRA should do that for you.
>...
>> That is, I'd like to see the patch greatly simplified to just
>consider
>> constant pool
>> complete scalarization as if it were a regular variable.
>
>Hmm, can you clarify, do you mean I should *not* replace constant pool
>values with their DECL_INITIAL? The attempt to substitute in the
>initial value is what leads to most of the problems. For example, in
>gnat/opt31.adb, create_access finds this expression accessing *.LC0:
>
>MEM[(interfaces__unsigned_8[(sizetype) <PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->LB0:<PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->UB0 >= <PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->LB0 ? (sizetype) <PLACEHOLDER_EXPR
>struct opt31__messages_t___XUP>.P_BOUNDS->UB0 : (sizetype)
><PLACEHOLDER_EXPR struct opt31__messages_t___XUP>.P_BOUNDS->LB0 +
>4294967295] *)&*.LC0][1 ...]{lb: 1 sz: 1}
>
>this is an ARRAY_RANGE_REF of a MEM_REF of an ADDR_EXPR of *.LC0. So
>far I haven't extended subst_constant_pool_initial to handle
>ARRAY_RANGE_REFs, as it can't even handle this MEM_REF:
>
>MEM[(interfaces__unsigned_8[(sizetype) <PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->LB0:<PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->UB0 >= <PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->LB0 ? (sizetype) <PLACEHOLDER_EXPR
>struct opt31__messages_t___XUP>.P_BOUNDS->UB0 : (sizetype)
><PLACEHOLDER_EXPR struct opt31__messages_t___XUP>.P_BOUNDS->LB0 +
>4294967295] *)&*.LC0]
>
>because the type here has size:
>
>MIN_EXPR <_GLOBAL.SZ2.ada_opt31 (<PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->UB0, <PLACEHOLDER_EXPR struct
>opt31__messages_t___XUP>.P_BOUNDS->LB0), 17179869176>
>
>inside the MEM_REF of the ADDR_EXPR is *.LC0, whose DECL_INITIAL is a
>4-element array (fine). Sadly while the MEM_REF
>type_contains_placeholder_p, the type of the outer ARRAY_RANGE_REF
>does not....
>
>One possibility is that this whole construct, ARRAY_RANGE_REF that it
>is, should mark *.LC0 in cannot_scalarize_away_bitmap.
>
>However, disqualified_constants is also necessary if I want a
>meaningful assertion that we do not re-add constant pool entries as
>candidates after we've discovered them - or should I try to rely on
>there only being one expression that accesses each constant pool
>entry? (I don't think that's guaranteed as tree_output_constant_def
>does hashing, I admit I haven't really tried to break that assumption)
I see.
>> 2) You should be able to use fold_ctor_reference directly (in place
>of
>> all your code
>> in case offset and size are readily available - don't remember
>exactly how
>> complete scalarization "walks" elements). Alternatively use
>> fold_const_aggregate_ref.
>
>That should work for completely_scalarize, yes, i.e. if I can remove
>the other route in create_access.
>
>> 3) You can simplify the stmt SRA generated by simply calling
>fold_stmt on it,
>> that will do a bit more (wasted) work compared to 2) but may be
>easier.
>>
>> I wouldn't bother with the case where we for some reason do not
>simplify
>> the constant pool load.
>
>Hmmm. As above, I'm not quite sure what you mean by "the constant pool
>load" - if that means, substituting the DECL_INITIAL in place of the
>VAR_DECL that is DECL_IN_CONSTANT_POOL, then I didn't find a general
>tree substitution routine, hence writing subst_constant_pool_initial.
>It might be possible to make that simpler/more generic (e.g. just call
>copy_node, recurse on each operand, return the original if nothing
>changed) and then fold at the end.
I tried to suggest creating piecewise loads from the constant pool as opposed
to the aggregate one. But I suppose the difficulty is to determine 'pieces',
not their values (that followup passes and folding can handle quite
efficiently).
As Eric I suggest you simply disqualify the candidate based on its type.
Richard.
>Thanks,
>Alan
