| Message ID | CA+4CFy50YiTyPe24p9CyR6__q18q5cCZLQaBRx7fKehkNzGZkw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 01/21/15 15:32, Wei Mi wrote: > Hi, > > The patch is to address the bug here: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64557 > > It is to call get_addr for VALUE before forming a mem_addr expr with > the VALUE and an offset. This is to avoid the problem that get_addr > can only handle VALUE but cannot handle an expr like: (VALUE + > offset). With the fix, find_base_term can always get the base of the > original addr. > > bootstrap and regression test on x86_64-linux-gnu are ok. regression > tests on aarch64-linux-gnu and powerpc64-linux-gnu are also ok. Is it > ok for trunk? > > Thanks, > Wei. > > gcc/ChangeLog: > > 2015-01-21 Wei Mi <wmi@google.com> > > * dse.c (record_store): Call get_addr for mem_addr. > (check_mem_read_rtx): Likewise. Please add a PR marker to the ChangeLog entry. A testcase would be great, but from reading the PR that doesn't seem possible without some heroic efforts. OK with the PR marker and a comment before the two calls indicating why those two calls are necessary. jeff
Thanks for the review. Comments addressed and patch committed. The problem exists on gcc-4_9 too. Is it ok for gcc-4_9-branch? Will wait another day to commit it to gcc-4_9 if it is ok. Thanks, Wei. On Thu, Jan 22, 2015 at 9:39 AM, Jeff Law <law@redhat.com> wrote: > On 01/21/15 15:32, Wei Mi wrote: >> >> Hi, >> >> The patch is to address the bug here: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64557 >> >> It is to call get_addr for VALUE before forming a mem_addr expr with >> the VALUE and an offset. This is to avoid the problem that get_addr >> can only handle VALUE but cannot handle an expr like: (VALUE + >> offset). With the fix, find_base_term can always get the base of the >> original addr. >> >> bootstrap and regression test on x86_64-linux-gnu are ok. regression >> tests on aarch64-linux-gnu and powerpc64-linux-gnu are also ok. Is it >> ok for trunk? >> >> Thanks, >> Wei. >> >> gcc/ChangeLog: >> >> 2015-01-21 Wei Mi <wmi@google.com> >> >> * dse.c (record_store): Call get_addr for mem_addr. >> (check_mem_read_rtx): Likewise. > > Please add a PR marker to the ChangeLog entry. A testcase would be great, > but from reading the PR that doesn't seem possible without some heroic > efforts. > > OK with the PR marker and a comment before the two calls indicating why > those two calls are necessary. > > jeff >
On 01/22/15 11:00, Wei Mi wrote: > Thanks for the review. Comments addressed and patch committed. The > problem exists on gcc-4_9 too. Is it ok for gcc-4_9-branch? Will wait > another day to commit it to gcc-4_9 if it is ok. Yes, if the patch from Uros was backported to 4.9, then this patch should get backported as well. The failure mode if this bug gets triggered will likely be hard to identify, so I'd rather not have to do that :-) jeff
On Thu, Jan 22, 2015 at 7:04 PM, Jeff Law <law@redhat.com> wrote: >> Thanks for the review. Comments addressed and patch committed. The >> problem exists on gcc-4_9 too. Is it ok for gcc-4_9-branch? Will wait >> another day to commit it to gcc-4_9 if it is ok. > > Yes, if the patch from Uros was backported to 4.9, then this patch should > get backported as well. The failure mode if this bug gets triggered will > likely be hard to identify, so I'd rather not have to do that :-) Thanks also from my side. I have to say to my defense, that my patch patched horrible bitrotten code that unnecessarily marked many operands as aliasing. The patches were extensively tested and I hope that this was the only fallout. Uros.
Index: gcc/dse.c =================================================================== --- gcc/dse.c (revision 219975) +++ gcc/dse.c (working copy) @@ -1575,6 +1575,7 @@ record_store (rtx body, bb_info_t bb_inf = rtx_group_vec[group_id]; mem_addr = group->canon_base_addr; } + mem_addr = get_addr (mem_addr); if (offset) mem_addr = plus_constant (get_address_mode (mem), mem_addr, offset); } @@ -2188,6 +2189,7 @@ check_mem_read_rtx (rtx *loc, bb_info_t = rtx_group_vec[group_id]; mem_addr = group->canon_base_addr; } + mem_addr = get_addr (mem_addr); if (offset) mem_addr = plus_constant (get_address_mode (mem), mem_addr, offset); }
Hi, The patch is to address the bug here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64557 It is to call get_addr for VALUE before forming a mem_addr expr with the VALUE and an offset. This is to avoid the problem that get_addr can only handle VALUE but cannot handle an expr like: (VALUE + offset). With the fix, find_base_term can always get the base of the original addr. bootstrap and regression test on x86_64-linux-gnu are ok. regression tests on aarch64-linux-gnu and powerpc64-linux-gnu are also ok. Is it ok for trunk? Thanks, Wei. gcc/ChangeLog: 2015-01-21 Wei Mi <wmi@google.com> * dse.c (record_store): Call get_addr for mem_addr. (check_mem_read_rtx): Likewise.