"Alexander Monakov" <[EMAIL PROTECTED]> writes:
> The implementation follows the outline presented in my initial message
> on the project, here: http://gcc.gnu.org/ml/gcc/2007-03/msg00900.html
> . Data references and data dependency relations obtained via
> compute_dependencies_for_loop are copied into containing struct (which
> is a member of struct function). This containing struct is marked
> GTY((skip)), because its lifetime and lifetime of its components is
> known (from the moment of export until destruction late in the RTL
> pipeline). However, I need to preserve trees that are referenced in
> the exported datarefs (to be able to bind MEMs to datarefs later), so
> I need to put them into a GC-visible array. For now that array is
> global. Can it be done better?
It seems to me that this array could also be in the function struct.
I don't see why not. Given the way it is used, it also seems to me
that this array should be a pointer_map.
> MEMs are bound to datarefs by means of saving original trees for MEMs
> during expand. This is a large part of the attached patch with
> changes to emit-rtl.[ch], which adds new field to struct mem_attrs
> (mem_orig_expr), initializes it and propagates through various RTL
> transformations. Notice that it has stricter hashing (as compared to
> orig_expr part of the same struct), so it will reduce sharing of
> mem_attrs (and also increase size of the struct, obviously). Can it
> be a problem? Please also note that this patch is a part of aliasing
> information exporting patch by Dmitry Melnik, which available now in
> alias-export branch.
Adding a new field to mem_attrs could be a problem. You'll have to
measure to see how much the memory usage changes.
> After original tree is found, a corresponding dataref is looked up in
> exported info. I guess I will need here hashtabs for fast lookup
> (also for searching ddrs for pairs of datarefs), am I right?
Yes, hash tables or pointer maps.
> The verifier also frequently breaks on passes that create unreachable
> basic blocks (because dominator analysis called from flow_loops_find
> asserts there are none). For now I just place
> delete_unreachable_blocks in such passes, is that ok?
Not for the final patch, but it's OK for debugging.
> +/* If EXPR contains conversions at the root of the tree, all of them
> + will be removed. */
> +
> +static tree
> +skip_conversions (tree expr)
> +{
> + tree inner = expr;
> + /* Remove any conversions: they don't change what the underlying
> + object is. Likewise for SAVE_EXPR. */
> + while (TREE_CODE (inner) == NOP_EXPR || TREE_CODE (inner) == CONVERT_EXPR
> + || TREE_CODE (inner) == NON_LVALUE_EXPR
> + || TREE_CODE (inner) == VIEW_CONVERT_EXPR
> + || TREE_CODE (inner) == SAVE_EXPR)
> + inner = TREE_OPERAND (inner, 0);
> + return inner;
> +}
I don't understand why it's OK to throw all these away. Some of them
change the meaning of the expression. I also don't understand why you
want to throw them away.
> @@ -1401,27 +1456,36 @@ component_ref_for_mem_expr (tree ref)
> {
> tree inner = TREE_OPERAND (ref, 0);
>
> - if (TREE_CODE (inner) == COMPONENT_REF)
> + /* Remove any conversions: they don't change what the underlying
> + object is. Likewise for SAVE_EXPR. */
> + while (TREE_CODE (inner) == NOP_EXPR || TREE_CODE (inner) == CONVERT_EXPR
> + || TREE_CODE (inner) == NON_LVALUE_EXPR
> + || TREE_CODE (inner) == VIEW_CONVERT_EXPR
> + || TREE_CODE (inner) == SAVE_EXPR)
> + inner = TREE_OPERAND (inner, 0);
> +
> + if (TREE_CODE (inner) == COMPONENT_REF || TREE_CODE (inner) ==
> INDIRECT_REF)
> inner = component_ref_for_mem_expr (inner);
> else
> {
> - /* Now remove any conversions: they don't change what the underlying
> - object is. Likewise for SAVE_EXPR. */
> - while (TREE_CODE (inner) == NOP_EXPR || TREE_CODE (inner) ==
> CONVERT_EXPR
> - || TREE_CODE (inner) == NON_LVALUE_EXPR
> - || TREE_CODE (inner) == VIEW_CONVERT_EXPR
> - || TREE_CODE (inner) == SAVE_EXPR)
> - inner = TREE_OPERAND (inner, 0);
> -
> if (! DECL_P (inner))
> inner = NULL_TREE;
> }
>
> if (inner == TREE_OPERAND (ref, 0))
> return ref;
> - else
> - return build3 (COMPONENT_REF, TREE_TYPE (ref), inner,
> - TREE_OPERAND (ref, 1), NULL_TREE);
> + else if (TREE_CODE (ref) == COMPONENT_REF)
> + {
> + return build3 (COMPONENT_REF, TREE_TYPE (ref), inner,
> + TREE_OPERAND (ref, 1), NULL_TREE);
> + }
> + else /* INDIRECT_REF. */
> + {
> + if (inner == NULL_TREE)
> + return NULL_TREE;
> + else
> + return build1 (INDIRECT_REF, TREE_TYPE (ref), inner);
> + }
> }
The handling of INDIRECT_REF here seems clearly wrong. You definitely
can't discard conversions for an INDIRECT_REF.
Hope this helps a bit. I will have more luck understanding what is
going on when I see how you use the data.
Ian
