Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-04 Thread Jeff Law
On 09/01/14 23:14, Bin.Cheng wrote: On Tue, Sep 2, 2014 at 11:40 AM, Jeff Law l...@redhat.com wrote: On 08/31/14 22:18, Bin.Cheng wrote: 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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-04 Thread Jeff Law
On 09/01/14 23:17, Bin.Cheng wrote: For this specific case, I think the reuse of r84 comes from coalescing during expanding, and this is necessary to remove redundant reg-moves. Then we need to fix this in coming passes? Hmmm. Yea, I can see how that might be happening. There's a certain

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-03 Thread Segher Boessenkool
On Wed, Sep 03, 2014 at 10:34:39AM +0800, Bin.Cheng wrote: Now I guess this check could be relaxed if somewhere else in combine we'd recognize the substitution into a clobber and simply omit it in that case. Yeah. In the testcase, combine tries combining 76,77 (77 is that clobbering

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-02 Thread Segher Boessenkool
On Tue, Sep 02, 2014 at 10:02:38AM +0800, Bin.Cheng wrote: Archaeology suggests this check is because the clobber might be an earlyclobber. Which seems silly: how can it be a valid insn at all in that case? It seems to me the check can just be removed. That will hide your issue, maybe

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-02 Thread Segher Boessenkool
On Mon, Sep 01, 2014 at 09:28:09PM -0600, Jeff Law wrote: Note that in this case we're talking about a hard register, not a pseudo. I was referring to r84 in Bin's message, not the condition code register. Unless I missed something it's set at the start of the sequence to the value 0, then

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-02 Thread Ulrich Weigand
Segher Boessenkool wreote: On Mon, Sep 01, 2014 at 10:39:10AM -0600, Jeff Law wrote: On 09/01/14 05:38, Segher Boessenkool wrote: On Mon, Sep 01, 2014 at 11:36:07AM +0800, Bin.Cheng wrote: In the testcase (and comment in the proposed patch), why is combine combining four insns at all?

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-02 Thread Segher Boessenkool
On Tue, Sep 02, 2014 at 02:10:32PM +0200, Ulrich Weigand wrote: In any case, this test in can_combine_p rejects a combination for *two* different issues. One is the earlyclobber problem, which is what that 2004 thread was about, and which my patch back then relaxed for fixed hard register.

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-02 Thread Bin.Cheng
On Tue, Sep 2, 2014 at 9:40 PM, Segher Boessenkool seg...@kernel.crashing.org wrote: On Tue, Sep 02, 2014 at 02:10:32PM +0200, Ulrich Weigand wrote: In any case, this test in can_combine_p rejects a combination for *two* different issues. One is the earlyclobber problem, which is what that

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Segher Boessenkool
On Mon, Sep 01, 2014 at 11:36:07AM +0800, Bin.Cheng wrote: In the testcase (and comment in the proposed patch), why is combine combining four insns at all? That means it rejected combining just the first three. Why did it do that? It is explicitly reject by below code in can_combine_p.

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Jeff Law
On 09/01/14 05:38, Segher Boessenkool wrote: On Mon, Sep 01, 2014 at 11:36:07AM +0800, Bin.Cheng wrote: In the testcase (and comment in the proposed patch), why is combine combining four insns at all? That means it rejected combining just the first three. Why did it do that? It is explicitly

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Jeff Law
On 08/31/14 06:18, Segher Boessenkool wrote: On Fri, Aug 29, 2014 at 11:58:37PM -0600, Jeff Law wrote: 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. It works fine in other cases, and is

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Segher Boessenkool
On Mon, Sep 01, 2014 at 10:39:10AM -0600, Jeff Law wrote: On 09/01/14 05:38, Segher Boessenkool wrote: On Mon, Sep 01, 2014 at 11:36:07AM +0800, Bin.Cheng wrote: In the testcase (and comment in the proposed patch), why is combine combining four insns at all? That means it rejected combining

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Segher Boessenkool
On Mon, Sep 01, 2014 at 10:41:43AM -0600, Jeff Law wrote: On 08/31/14 06:18, Segher Boessenkool wrote: On Fri, Aug 29, 2014 at 11:58:37PM -0600, Jeff Law wrote: 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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Bin.Cheng
On Tue, Sep 2, 2014 at 1:50 AM, Segher Boessenkool seg...@kernel.crashing.org wrote: On Mon, Sep 01, 2014 at 10:39:10AM -0600, Jeff Law wrote: On 09/01/14 05:38, Segher Boessenkool wrote: On Mon, Sep 01, 2014 at 11:36:07AM +0800, Bin.Cheng wrote: In the testcase (and comment in the proposed

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Jeff Law
On 09/01/14 13:15, Segher Boessenkool wrote: On Mon, Sep 01, 2014 at 10:41:43AM -0600, Jeff Law wrote: On 08/31/14 06:18, Segher Boessenkool wrote: On Fri, Aug 29, 2014 at 11:58:37PM -0600, Jeff Law wrote: One could argue that this mess is a result of trying to optimize a reg that is set more

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Jeff Law
On 09/01/14 11:50, Segher Boessenkool wrote: Another question is why is r84 set twice in the first place? Various transformations can set that kind of situation up. Sure -- but also lazy expanders can reuse a register instead of doing gen_reg_rtx. Which is why I asked :-) Which comes back

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Jeff Law
On 08/31/14 22:18, Bin.Cheng wrote: 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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Bin.Cheng
On Tue, Sep 2, 2014 at 11:40 AM, Jeff Law l...@redhat.com wrote: On 08/31/14 22:18, Bin.Cheng wrote: 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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-09-01 Thread Bin.Cheng
On Tue, Sep 2, 2014 at 11:28 AM, Jeff Law l...@redhat.com wrote: On 09/01/14 13:15, Segher Boessenkool wrote: On Mon, Sep 01, 2014 at 10:41:43AM -0600, Jeff Law wrote: On 08/31/14 06:18, Segher Boessenkool wrote: On Fri, Aug 29, 2014 at 11:58:37PM -0600, Jeff Law wrote: One could argue

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-08-31 Thread Segher Boessenkool
On Fri, Aug 29, 2014 at 11:58:37PM -0600, Jeff Law wrote: 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. It works fine in other cases, and is quite beneficial for e.g. optimising

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-08-31 Thread Bin.Cheng
On Sun, Aug 31, 2014 at 8:18 PM, Segher Boessenkool seg...@kernel.crashing.org wrote: On Fri, Aug 29, 2014 at 11:58:37PM -0600, Jeff Law wrote: 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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-08-31 Thread Bin.Cheng
On Sat, Aug 30, 2014 at 1:58 PM, Jeff Law l...@redhat.com wrote: 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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-08-29 Thread Jeff Law
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

[PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-08-27 Thread Bin Cheng
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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-08-27 Thread Richard Earnshaw
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

Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass

2014-08-27 Thread Bin.Cheng
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