| Message ID | 5639D3A8.5040606@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 11/04/2015 01:45 AM, Jiong Wang wrote: > So as Jim Wilson commented on the bugzilla, instead of "return !strict", > we need to only do the check if strict be true, and only does rejection > which means return FALSE, for all other cases, we need to go through > those normal checks below. I was just about to submit the same patch myself; my testsuite run finished last night. This testsuite run was with a toolchain configured "--with-arch=armv8-a --with-fpu=neon-fp-armv8 --with-float=hard --with-mode=thumb --enable-languages=c,c++". I see 10 fewer unexpected failures on the gcc testsuite with the patch, and no changes to the other testsuite results. Jim
On 04/11/15 09:45, Jiong Wang wrote: > As discussed at the bugzilla > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67305 > > neon_vector_mem_operand is broken. As the comments says > "/* Reject eliminable registers. */", the code block at the head > of this function which checks eliminable registers is designed to do > early reject only, there shouldn't be any early accept. > > If this code hunk doesn't reject the incoming rtx, then the rtx pattern > should still go through all default checks below. All other similar > functions, thumb1_legitimate_address_p, arm_coproc_mem_operand, > neon_struct_mem_operand etc are exactly follow this check flow. > > So as Jim Wilson commented on the bugzilla, instead of "return !strict", > we need to only do the check if strict be true, and only does rejection > which means return FALSE, for all other cases, we need to go through > those normal checks below. > > neon_vector_mem_operand is only used by several misalign pattern, I > guess that's why this bug is not exposed for long time. > > boostrap & regression OK on armv8 aarch32, ok for trunk? > > 2015-11-04 Jiong Wang <jiong.wang@arm.com> > Jim Wilson <wilson@gcc.gnu.org> > > gcc/ > PR target/67305 > * config/arm/arm.md (neon_vector_mem_operand): Return FALSE if strict > be true and eliminable registers mentioned. > Ping ~
On 04/11/15 09:45, Jiong Wang wrote: > As discussed at the bugzilla > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67305 > > neon_vector_mem_operand is broken. As the comments says > "/* Reject eliminable registers. */", the code block at the head > of this function which checks eliminable registers is designed to do > early reject only, there shouldn't be any early accept. > > If this code hunk doesn't reject the incoming rtx, then the rtx pattern > should still go through all default checks below. All other similar > functions, thumb1_legitimate_address_p, arm_coproc_mem_operand, > neon_struct_mem_operand etc are exactly follow this check flow. > > So as Jim Wilson commented on the bugzilla, instead of "return !strict", > we need to only do the check if strict be true, and only does rejection > which means return FALSE, for all other cases, we need to go through > those normal checks below. > > neon_vector_mem_operand is only used by several misalign pattern, I > guess that's why this bug is not exposed for long time. > > boostrap & regression OK on armv8 aarch32, ok for trunk? > > 2015-11-04 Jiong Wang <jiong.wang@arm.com> > Jim Wilson <wilson@gcc.gnu.org> > > gcc/ > PR target/67305 > * config/arm/arm.md (neon_vector_mem_operand): Return FALSE if strict > be true and eliminable registers mentioned. > This has been lurking for a long time ... Sorry about the delay in reviewing. This is OK for trunk regards Ramana > > neon-mem.patch > > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index 87e55e9..7fbf897 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -12957,14 +12957,14 @@ neon_vector_mem_operand (rtx op, int type, bool strict) > rtx ind; > > /* Reject eliminable registers. */ > - if (! (reload_in_progress || reload_completed) > - && ( reg_mentioned_p (frame_pointer_rtx, op) > + if (strict && ! (reload_in_progress || reload_completed) > + && (reg_mentioned_p (frame_pointer_rtx, op) > || reg_mentioned_p (arg_pointer_rtx, op) > || reg_mentioned_p (virtual_incoming_args_rtx, op) > || reg_mentioned_p (virtual_outgoing_args_rtx, op) > || reg_mentioned_p (virtual_stack_dynamic_rtx, op) > || reg_mentioned_p (virtual_stack_vars_rtx, op))) > - return !strict; > + return FALSE; > > /* Constants are converted into offsets from labels. */ > if (!MEM_P (op)) >
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 87e55e9..7fbf897 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -12957,14 +12957,14 @@ neon_vector_mem_operand (rtx op, int type, bool strict) rtx ind; /* Reject eliminable registers. */ - if (! (reload_in_progress || reload_completed) - && ( reg_mentioned_p (frame_pointer_rtx, op) + if (strict && ! (reload_in_progress || reload_completed) + && (reg_mentioned_p (frame_pointer_rtx, op) || reg_mentioned_p (arg_pointer_rtx, op) || reg_mentioned_p (virtual_incoming_args_rtx, op) || reg_mentioned_p (virtual_outgoing_args_rtx, op) || reg_mentioned_p (virtual_stack_dynamic_rtx, op) || reg_mentioned_p (virtual_stack_vars_rtx, op))) - return !strict; + return FALSE; /* Constants are converted into offsets from labels. */ if (!MEM_P (op))
As discussed at the bugzilla https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67305 neon_vector_mem_operand is broken. As the comments says "/* Reject eliminable registers. */", the code block at the head of this function which checks eliminable registers is designed to do early reject only, there shouldn't be any early accept. If this code hunk doesn't reject the incoming rtx, then the rtx pattern should still go through all default checks below. All other similar functions, thumb1_legitimate_address_p, arm_coproc_mem_operand, neon_struct_mem_operand etc are exactly follow this check flow. So as Jim Wilson commented on the bugzilla, instead of "return !strict", we need to only do the check if strict be true, and only does rejection which means return FALSE, for all other cases, we need to go through those normal checks below. neon_vector_mem_operand is only used by several misalign pattern, I guess that's why this bug is not exposed for long time. boostrap & regression OK on armv8 aarch32, ok for trunk? 2015-11-04 Jiong Wang <jiong.wang@arm.com> Jim Wilson <wilson@gcc.gnu.org> gcc/ PR target/67305 * config/arm/arm.md (neon_vector_mem_operand): Return FALSE if strict be true and eliminable registers mentioned.