Re: Another case of DCE deleting live code

[kenner at vlsi1 dot ultra dot nyu dot edu (Richard Kenner)]

    Hmm.  I wonder how early the alias set for FRAME is getting set?

    If it is the case that it's getting set before the structure is
    fully constructed, then this would be a bug in tree-nested.c and
    not in the ada front end.

That turns out not to be the bug.

Diego and I have been working on this off-list for a few hours.  There are a
number of things going on here.

The key is the statement

  proc2GP218__b1.1 = (character[1 .. 4] *) proc2GP218__b1.0;

This pointer pun has the pointed-to types in different alias sets.  That's
the key to the problem and expains the data that Diego showed.  There are a
number of issues here.

This came from the gimplification of

  if ("ABCD" != VIEW_CONVERT_EXPR<character[1 .. 4]>(proc2GP218__b1))

where we convert the address of a VIEW_CONVERT_EXPR into pointer-punning.

In this case, the V_C_E was not the result of a user operation, but was
due to Gigi converting between two subtypes.  There is code in Gigi to
give all subtypes of a base type the same alias sets as the base type.
However, this had two bugs.  First, it didn't work correctly when the
base type was an unconstrained array (String here) and secondly, it didn't
even attempt to do this in the case of a string literal subtype.  Fixing
these bugs are a few lines of code in ada/decl.c and fix this case.

But there are still potential problems.

In this case, the V_C_E wasn't the result of an explicit user operation,
but what if it were?  What if we had the same comparison with "ABCD"
but now the other side was an Unchecked_Conversion of a record with
four characters into a string of four characters?  We'll have exactly the
same problem since those two types will not have the same alias set so
the initialization of the record could potentially be deleted as dead.

I think part of the fix is in build_fold_addr_expr.  If the operand is
a V_C_E with its input and output in different alias sets, the pointer type
it should make is one with TYPE_REF_CAN_ALIAS_ALL set.

But then we get into a potential problem in get_tmt_for.  This is
determining the alias set that a dereference of this pointer would get
and doing it with:

  tree tag_type = TREE_TYPE (TREE_TYPE (ptr));
  HOST_WIDE_INT tag_set = get_alias_set (tag_type);

I believe this is wrong.  If TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (ptr)), then
the alias set should be zero, but only this code can know that.  So I think
the following should be there (with the corresponding change to the
gcc_assert at the end of that function).

  /* If this pointer can alias anything, then the alias set of anything
     dereferenced by it is zero.  */
  if (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (ptr)))
    tag_set = 0;

but Diego disagrees.

What an interesting number of bugs that came up from such a tiny test case!