This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix PR middle-end/17746
[Thanks again for looking into this.]
> Again, my question is: what behavior do we expect from this code?
> Given the quoted code
> VIEW_CONVERT_EXPR <record_type1 ... align-ok>
> INDIRECT_REF <record_type2>
> VAR_DECL <pointer_type <record_type2>>
> where the VIEW_CONVERT_EXPR is increasing the alignment, either
> 1) we know that the pointer already has the proper alignment, or
> 2) the pointer might not have the proper alignment, so we might need to
> copy into a record_type1 temporary with the appropriate alignment.
I think it's 1). My understanding is that the polymorphic pointer is
guaranteed to be sufficiently aligned by the front-end through the setting of
the TYPE_ALIGN_OK flag on the VIEW_CONVERT_EXPR.
> I suspect that the get_inner_reference change was needed because #2 is
> true. In that case, the gimplifier should allocate a temporary for the
> VIEW_CONVERT_EXPR on platforms where a temporary is needed.
Reading the audit trail for TN BA08-006, it is clear that the change was made
to *prevent* a temporary from being allocated. A few excerpts:
"I'll take a look, if nothing else to understand the semantics here! This is
a protected object, so it's certainly wrong to copy it."
"The alignments are fine, the discrepancy is expected, this should work. I
rather suspect that the mechanism for the Object that is the implicit
parameter of a protected operation is not labelled as By_Reference, and
the back-end may decide that it has the permission to pass it by
copy. Let me suggest a patch to try on gnat5. If this doesn't work, it's
a deeper back-end problem."
Richard's final comment:
"Ed and I worked on this and it turns out that because of the tagged
types, the alignment is OK and the appropriate flags were set. But
because of the use of a component of the type, they were ignored. I
have a fix for it."
In light of this, my interpretation is that Richard's strategy was to force
get_inner_reference to "temporarily" exit, so that the VIEW_CONVERT_EXPR
expander can take control and step up the alignment:
op0 = expand_expr (TREE_OPERAND (exp, 0), NULL_RTX, mode, modifier);
if (MEM_P (op0))
op0 = copy_rtx (op0);
if (TYPE_ALIGN_OK (type))
set_mem_align (op0, MAX (MEM_ALIGN (op0), TYPE_ALIGN (type)));
To sum up, I think the gimplifier must not allocate a temporary for
VIEW_CONVERT_EXPR in the TYPE_ALIGN_OK case, but could do so in other
cases. However, only its expander currently knows the exact condition.
About the handled_component_p/get_inner_reference discrepancy: a widely used
while (handled_component_p (t))
t = TREE_OPERAND (t, 0);
So if we made handled_component_p reject VIEW_CONVERT_EXPRs with the
TYPE_ALIGN_OK flag, I suspect we would probably need to add explicit
|| ((TREE_CODE (t) == VIEW_CONVERT_EXPR && TYPE_ALIGN_OK (t) && ...)
all over the place.
> Incidentally, the expand_expr_real/VIEW_CONVERT_EXPR code doesn't seem to
> try to use 'target' in the alignment mismatch case; it seems to me that it
> should do that before it allocates a temporary.
Indeed, that could be a useful improvement.