On 09/17/2018 05:09 PM, Jeff Law wrote:
On 9/14/18 4:11 PM, Martin Sebor wrote:
On 09/14/2018 03:35 PM, Jeff Law wrote:
On 9/12/18 11:46 AM, Martin Sebor wrote:
On 08/31/2018 04:07 AM, Richard Biener wrote:
On Thu, Aug 30, 2018 at 7:39 PM Martin Sebor <mse...@gmail.com> wrote:
On 08/30/2018 11:22 AM, Richard Biener wrote:
On August 30, 2018 6:54:21 PM GMT+02:00, Martin Sebor
<mse...@gmail.com> wrote:
On 08/30/2018 02:35 AM, Richard Biener wrote:
On Thu, Aug 30, 2018 at 2:12 AM Martin Sebor <mse...@gmail.com>
wrote:
The attached patch adds code to work harder to determine whether
the destination of an assignment involving MEM_REF is the same
as the destination of a prior strncpy call. The included test
case demonstrates when this situation comes up. During ccp,
dstbase and lhsbase returned by get_addr_base_and_unit_offset()
end up looking like this:
"During CCP" means exactly when? The CCP lattice tracks copies
so CCP should already know that _1 == _8. I suppose during
substitute_and_fold then? But that replaces uses before folding
the stmt.
Yes, when ccp_finalize() performs the final substitution during
substitute_and_fold().
But then you shouldn't need the loop but at most look at the pointer
SSA Def to get at the non-invariant ADDR_EXPR.
I don't follow. Are you suggesting to compare
SSA_NAME_DEF_STMT (dstbase) to SSA_NAME_DEF_STMT (lhsbase) for
equality? They're not equal.
No.
The first loop iterates once and retrieves
1. _8 = &pb_3(D)->a;
The second loop iterates three times and retrieves:
1. _1 = _9
2. _9 = _8
3. _8 = &pb_3(D)->a;
How do I get from _1 to &pb_3(D)->a without iterating? Or are
you saying to still iterate but compare the SSA_NAME_DEF_STMT?
I say you should retrieve _8 = &pb_3(D)->a immediately since the
copies should be
propagated out at this stage.
The warning is issued as the strncpy call is being folded (during
the dom walk in substitute_and_fold_engine::substitute_and_fold)
but before the subsequent statements have been folded (during
the subsequent loop to eliminate statements). So at the point
of the strncpy folding the three assignments above are still
there.
I can't think of a good way to solve this problem that's not
overly intrusive. Unless you have some suggestions for how
to deal with it, is the patch okay as is?
In what pass do you see the the naked copies? In general those should
have been propagated away.
As I said above, this happens during the dom walk in the ccp
pass:
My bad. Sigh. CCP doesn't track copies, just constants, so there's not
going to be any data structure you can exploit. And I don't think
there's a value number you can use to determine the two objects are the
same.
Hmm, let's back up a bit, what is does the relevant part of the IL look
like before CCP? Is the real problem here that we have unpropagated
copies lying around in the IL? Hmm, more likely the IL looksl ike:
_8 = &pb_3(D)->a;
_9 = _8;
_1 = _9;
strncpy (MEM_REF (&pb_3(D)->a), ...);
MEM[(struct S *)_1].a[n_7] = 0;
Yes, that is what the folder sees while the strncpy call is
being transformed/folded by ccp. The MEM_REF is folded just
after the strncpy call and that's when it's transformed into
MEM[(struct S *)_8].a[n_7] = 0;
(The assignments to _1 and _9 don't get removed until after
the dom walk finishes).
If we were to propagate the copies out we'd at best have:
_8 = &pb_3(D)->a;
strncpy (MEM_REF (&pb_3(D)->a), ...);
MEM[(struct S *)_8].a[n_7] = 0;
Is that in a form you can handle? Or would we also need to forward
propagate the address computation into the use of _8?
The above works as long as we look at the def_stmt of _8 in
the MEM_REF (we currently don't). That's also what the last
iteration of the loop does. In this case (with _8) it would
be discovered in the first iteration, so the loop could be
replaced by a simple if statement.
But I'm not sure I understand the concern with the loop. Is
it that we are looping at all, i.e., the cost? Or that ccp
is doing something wrong or suboptimal? (Should have
propagated the value of _8 earlier?)
Martin
