Re: [PATCH 2/2] Reimplementation of build_ref_for_offset

["H.J. Lu" <hjl.tools@gmail.com>]

On Sat, Oct 23, 2010 at 10:12 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Sep 8, 2010 at 9:43 AM, Martin Jambor <mjambor@suse.cz> wrote:
>> Hi,
>>
>> this patch reimplements build_ref_for_offset so that it simply creates
>> a MEM_REF rather than trying to figure out what combination of
>> component and array refs are necessary.  The main advantage of this
>> approach is that this can never fail, allowing us to be more
>> aggressive and remove a number of checks.
>>
>> There were two main problems with this, though.  First is that
>> MEM_REFs are not particularly readable to by users.  This would be a
>> problem when we are creating a reference that might be displayed to
>> them in a warning or a debugger which is what we do with
>> DECL_DEBUG_EXPR expressions.  We sometimes construct these
>> artificially when propagating accesses across assignments.  So for
>> those cases I retained the old implementation and only simplified it a
>> bit - it is now called build_user_friendly_ref_for_offset.
>>
>> The other problem was bit-fields.  Constructing accesses to them was
>> difficult enough but then I realized that I was not even able to
>> detect the cases when I was accessing a bit field if their offset
>> happened to be on a byte boundary.  I thought I would be able to
>> figure this out from TYPE_SIZE and TYPE_PRECISION of exp_type but
>> combinations that signal a bit-field in one language may not be
>> applied in another (in C, small TYPE_PRECISION denotes bit-fields and
>> TYPE_SIZE is big, but for example Fortran booleans have the precision
>> set to one even though they are not bit-fields).
>>
>> So in the end I based the detection on the access structures that
>> represented the thing being loaded or stored which I knew had their
>> sizes correct because they are based on field sizes.  Since I use the
>> access, the simplest way to actually create the reference to the bit
>> field is to re-use the last component ref of its expression - that is
>> what build_ref_for_model (meaning a model access) does.  Separating
>> this from build_ref_for_offset (which cannot handle bit-fields) makes
>> the code a bit cleaner and keeps the latter function for other users
>> which know nothing about SRA access structures.
>>
>> I hope that you'll find these approaches reasonable.  The patch was
>> bootstrapped and tested on x86_64-linux without any issues.  I'd like
>> to commit it to trunk but I'm sure there will be comments and
>> suggestions.
>>
>> Thanks,
>>
>> Martin
>>
>>
>>
>> 2010-09-08  Martin Jambor  <mjambor@suse.cz>
>>
>>        PR tree-optimization/44972
>>        * tree-sra.c: Include toplev.h.
>>        (build_ref_for_offset): Entirely reimplemented.
>>        (build_ref_for_model): New function.
>>        (build_user_friendly_ref_for_offset): New function.
>>        (analyze_access_subtree): Removed build_ref_for_offset check.
>>        (propagate_subaccesses_across_link): Likewise.
>>        (create_artificial_child_access): Use
>>        build_user_friendly_ref_for_offset.
>>        (propagate_subaccesses_across_link): Likewise.
>>        (ref_expr_for_all_replacements_p): Removed.
>>        (generate_subtree_copies): Updated comment.  Use build_ref_for_model.
>>        (sra_modify_expr): Use build_ref_for_model.
>>        (load_assign_lhs_subreplacements): Likewise.
>>        (sra_modify_assign): Removed ref_expr_for_all_replacements_p checks,
>>        checks for return values of build_ref_for_offset.
>>        * ipa-cp.c (ipcp_lattice_from_jfunc): No need to check return value of
>>        build_ref_for_offset.
>>        * ipa-prop.h: Include gimple.h
>>        * ipa-prop.c (ipa_compute_jump_functions): Update to look for MEM_REFs.
>>        (ipa_analyze_indirect_call_uses): Update comment.
>>        * Makefile.in (tree-sra.o): Add $(GIMPLE_H) to dependencies.
>>        (IPA_PROP_H): Likewise.
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46150
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49039

-- 
H.J.