Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-04 Thread James Greenhalgh
On Wed, Aug 03, 2016 at 04:08:30PM -0600, Jeff Law wrote: > On 08/03/2016 11:41 AM, Bernd Edlinger wrote: > >On 08/03/16 17:38, Jeff Law wrote: > >>cse.c changes look good, but I'd really like to see a testcase for each > >>issue in the dejagnu framework. Extra points if you tried to build a >

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-03 Thread Jeff Law
On 08/03/2016 11:41 AM, Bernd Edlinger wrote: On 08/03/16 17:38, Jeff Law wrote: cse.c changes look good, but I'd really like to see a testcase for each issue in the dejagnu framework. Extra points if you tried to build a unit test using David M's framework, but that isn't required. The

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-03 Thread Bernd Edlinger
On 08/03/16 17:38, Jeff Law wrote: > cse.c changes look good, but I'd really like to see a testcase for each > issue in the dejagnu framework. Extra points if you tried to build a > unit test using David M's framework, but that isn't required. > > The testcase from 70903 ought to be trivial to

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-03 Thread Jeff Law
On 08/01/2016 12:52 PM, Bernd Edlinger wrote: Hi Jeff, On 08/01/16 19:54, Jeff Law wrote: > Looks like you've probably nailed it. It'll be interesting see if > there's any fallout (though our RTL optimizer testing is pretty weak, so > even if there were, I doubt we'd catch it). > If there

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-03 Thread Jeff Law
On 08/02/2016 02:16 PM, Bernd Schmidt wrote: On 08/02/2016 06:46 PM, Segher Boessenkool wrote: On Tue, Aug 02, 2016 at 09:21:34AM -0600, Jeff Law wrote: However I think there are more paradoxical subregs generated all over, but the aarch64 insv code pattern did trigger more hidden bugs than

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-03 Thread Bernd Edlinger
Hi, Is it OK for the trunk? I guess so, but need an explicit OK. Thanks Bernd. On 08/01/16 20:52, Bernd Edlinger wrote: > Hi Jeff, > > On 08/01/16 19:54, Jeff Law wrote: >> Looks like you've probably nailed it. It'll be interesting see if >> there's any fallout (though our RTL optimizer

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-02 Thread Bernd Schmidt
On 08/02/2016 06:46 PM, Segher Boessenkool wrote: On Tue, Aug 02, 2016 at 09:21:34AM -0600, Jeff Law wrote: However I think there are more paradoxical subregs generated all over, but the aarch64 insv code pattern did trigger more hidden bugs than any other port. It is certainly unfortunate

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-02 Thread Jeff Law
On 08/02/2016 11:46 AM, Richard Biener wrote: But we love to exploit undefined behavior elsewhere, too. Now the init-regs pass comes to my mind again (papering over issues elsewhere).. True. I just haven't seen that the don't care bits created by paradoxical subregs has actually been all that

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-02 Thread Richard Biener
On August 2, 2016 5:21:34 PM GMT+02:00, Jeff Law wrote: >On 08/01/2016 05:31 PM, Segher Boessenkool wrote: >> Hi, >> >> On Mon, Aug 01, 2016 at 06:52:54PM +, Bernd Edlinger wrote: >>> On 08/01/16 19:54, Jeff Law wrote: Looks like you've probably nailed it. It'll be

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-02 Thread Segher Boessenkool
On Tue, Aug 02, 2016 at 09:21:34AM -0600, Jeff Law wrote: > >>However I think there are more paradoxical subregs generated all over, > >>but the aarch64 insv code pattern did trigger more hidden bugs than > >>any other port. It is certainly unfortunate that the major source > >>of paradoxical

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-02 Thread Jeff Law
On 08/01/2016 05:31 PM, Segher Boessenkool wrote: Hi, On Mon, Aug 01, 2016 at 06:52:54PM +, Bernd Edlinger wrote: On 08/01/16 19:54, Jeff Law wrote: Looks like you've probably nailed it. It'll be interesting see if there's any fallout (though our RTL optimizer testing is pretty weak, so

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-01 Thread Segher Boessenkool
Hi, On Mon, Aug 01, 2016 at 06:52:54PM +, Bernd Edlinger wrote: > On 08/01/16 19:54, Jeff Law wrote: > > Looks like you've probably nailed it. It'll be interesting see if > > there's any fallout (though our RTL optimizer testing is pretty weak, so > > even if there were, I doubt we'd catch

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-01 Thread Bernd Edlinger
Hi Jeff, On 08/01/16 19:54, Jeff Law wrote: > Looks like you've probably nailed it. It'll be interesting see if > there's any fallout (though our RTL optimizer testing is pretty weak, so > even if there were, I doubt we'd catch it). > If there is, it will probably a performance regression...

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-01 Thread Jeff Law
On 07/31/2016 04:44 AM, Bernd Edlinger wrote: like this? Index: emit-rtl.c === --- emit-rtl.c (revision 238891) +++ emit-rtl.c (working copy) @@ -1156,7 +1156,11 @@ { #if defined(POINTERS_EXTEND_UNSIGNED) if

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-08-01 Thread Jeff Law
On 07/30/2016 02:17 AM, Bernd Edlinger wrote: In your first mail you showed reg 481 as _not_ being REG_POINTER: (insn 1047 1046 1048 (set (reg:DI 481) (subreg:DI (reg/f:SI 479) 0)) y.c:12702 -1 (nil)) (note the lack of /f). So which is it? REG_POINTER here is not correct as

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-31 Thread Bernd Edlinger
On 07/31/16 12:43, Bernd Edlinger wrote: > I found out that the trouble starts one instruction earlier: > > (insn 19 40 20 2 (set (subreg:DI (reg:QI 93) 0) > ) pr.c:8 50 {*movdi_aarch64} > (expr_list:REG_DEAD (reg:DI 110 [ D.3037 ]) > (nil))) > oops, my e-mail missed one

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-31 Thread Bernd Edlinger
On 07/30/16 13:39, Segher Boessenkool wrote: > On Sat, Jul 30, 2016 at 08:17:59AM +, Bernd Edlinger wrote: >> Ok, I the incorrect REG_POINTER is done here: >> >> cse_main -> reg_scan -> reg_scan_mark_refs -> set_reg_attrs_from_value >> >> and here I see a bug, because if

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-30 Thread Segher Boessenkool
On Sat, Jul 30, 2016 at 08:17:59AM +, Bernd Edlinger wrote: > Ok, I the incorrect REG_POINTER is done here: > > cse_main -> reg_scan -> reg_scan_mark_refs -> set_reg_attrs_from_value > > and here I see a bug, because if POINTERS_EXTEND_UNSIGNED > can_be_reg_pointer is only set to false if x

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-30 Thread Bernd Edlinger
On 07/29/16 15:03, Bernd Edlinger wrote: > On 07/29/16 09:02, Segher Boessenkool wrote: >> On Thu, Jul 28, 2016 at 08:59:44PM +, Bernd Edlinger wrote: >>> (insn 1047 1044 1048 101 (set (reg/f:DI 481) >>> (subreg:DI (reg/f:SI 545) 0)) isl_input.c:2496 50 >>> {*movdi_aarch64} >>>

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-29 Thread Bernd Edlinger
On 07/29/16 09:02, Segher Boessenkool wrote: > On Thu, Jul 28, 2016 at 08:59:44PM +, Bernd Edlinger wrote: >> (insn 1047 1044 1048 101 (set (reg/f:DI 481) >> (subreg:DI (reg/f:SI 545) 0)) isl_input.c:2496 50 {*movdi_aarch64} >>(nil)) > > In your first mail you showed reg 481

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-29 Thread Segher Boessenkool
On Thu, Jul 28, 2016 at 08:59:44PM +, Bernd Edlinger wrote: > (insn 1047 1044 1048 101 (set (reg/f:DI 481) > (subreg:DI (reg/f:SI 545) 0)) isl_input.c:2496 50 {*movdi_aarch64} > (nil)) In your first mail you showed reg 481 as _not_ being REG_POINTER: (insn 1047 1046 1048 (set

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-28 Thread Bernd Edlinger
Hi, On 07/27/16 23:31, Jeff Law wrote: > So you're stumbling into another really interesting area. > Absolutely, I am just too curious what's going on here ;-) > I can hazard a guess that the reason we create the paradoxical SUBREG > rather than a zero or sign extension is because various

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-27 Thread Bernd Edlinger
Am 27.07.2016 um 23:31 schrieb Jeff Law: > On 07/26/2016 11:32 AM, Bernd Edlinger wrote: >> Hi! >> >> As described in PR 71779 and PR 70903 we have a wrong code issue on >> aarch64 >> which is triggered by a paradoxical subreg that can be created in >> store_bit_field_using_insv when the target

Re: [PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-27 Thread Jeff Law
On 07/26/2016 11:32 AM, Bernd Edlinger wrote: Hi! As described in PR 71779 and PR 70903 we have a wrong code issue on aarch64 which is triggered by a paradoxical subreg that can be created in store_bit_field_using_insv when the target has an EP_insv bitfield instruction. The attached patch

[PATCH] Fix wrong code on aarch64 due to paradoxical subreg

2016-07-26 Thread Bernd Edlinger
Hi! As described in PR 71779 and PR 70903 we have a wrong code issue on aarch64 which is triggered by a paradoxical subreg that can be created in store_bit_field_using_insv when the target has an EP_insv bitfield instruction. The attached patch changes this insn... (insn 1047 1046 1048 (set