On 08/27/14 23:04, Bin.Cheng wrote:
On Wed, Aug 27, 2014 at 6:34 PM, Richard Earnshaw <rearn...@arm.com> wrote:
On 27/08/14 11:08, Bin Cheng wrote:
Hi
As reported in bug pr62151, combine pass may wrongly delete necessary
instruction in function distribute_notes thus leaving register
uninitialized. This patch is to fix the issue by checking if i2 immediately
modifies the register in REG_DEAD note. If yes, set tem_insn accordingly to
start finding right place for note distribution from i2.
I once sent a RFC patch at
https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01718.html, but got no
comments, here I added some comments in this patch to make it a formal one.
I tested the original patch, and will re-test it against the latest code
later. So is it OK? Any comments will be appreciated.
Isn't this the sort of sequence that combinable_i3pat is supposed to reject?
Hi Richard,
I think it's not. combinable_i3pat checks cases in which i1 feeds to
i3 directly, while i2 kills reg_use in i1. For this case the feeding
chain is "i0->i1->i2->i3", the combination is valid and beneficial.
What needs to be handled is if i2dest is anticipated after i3. If
not, then i2 could be deleted; if yes, i2 should be preserved.
Moreover, following constant propagation could delete i2 after
propagating the new i2src into i4. Note combine pass already handles
this kind of case using variable added_sets_2 in function try_combine.
The problem is in distribute_notes which mis-deleted i2.
I added one test case in the updated patch.
One could argue that this mess is a result of trying to optimize a reg
that is set more than once. Though I guess that might be a bit of a
big hammer.
I haven't thought real hard, but isn't it the case that for a pseudo
with multiple sets that we never want to move a REG_DEAD note across a
set of that pseudo? It would seem that in these cases we want to drop
the REG_DEAD note completely.
Note that i0..i4 need not be consecutive insns, so you'd have to walk
the chain from the location with the death note to the proposed death
note site. If between those locations there's another set of the same
pseudo, then drop the note. Since this may be an expensive check it
should probably be conditionalized on REG_N_SETS (pseudo) > 1
Jeff
