Re: [PATCH 2/2] Reimplementation of build_ref_for_offset

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

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


H.J.