RE: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
Hi, Sorry, I forgot to mention I tested this on: aarch64 and aarch64_be - no regressions in gcc testsuite x86_64 - bootstrap build, no testsuite regressions arm-none-eabi - no regressions in gcc testsuite I will also make sure that I can do cross builds on a variety of different targets. Regards, David. > -Original Message- > From: David Sherwood [mailto:david.sherw...@arm.com] > Sent: 27 July 2015 11:25 > To: gcc-patches@gcc.gnu.org > Subject: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode > > Hi, > > Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER > (m) > so that it returns m in cases where there is no inner mode. This simplifies > some > of the calling code by removing the need to check for VOIDmode and allows > calling it unconditionally. I also removed element_precision () as it was only > called in one place and thought it neater to call GET_MODE_PRECISION > explicitly. > > Parts 2-4 will include further tidy-ups and optimisations based on [1/N]. > > Good to go? > > Regards, > David Sherwood. > > 2015-07-17 David Sherwood > > gcc/ > * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call > GET_MODE_INNER unconditionally. > * config/spu/spu.c (arith_immediate_p): Likewise. > * config/i386/i386.c (ix86_build_signbit_mask): Likewise. New > variable. > * expmed.c (synth_mult): Remove check for VOIDmode result from > GET_MODE_INNER. > (expand_mult_const): Likewise. > * fold-const.c (): Replace call to element_precision with call to > GET_MODE_PRECISION. > * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with > m->name. > (emit_mode_inner): Likewise. > * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER > result check. > * machmode.h (GET_MODE_UNIT_SIZE): Simplify. > (GET_MODE_UNIT_PRECISION): Likewise. > * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally. > * simplify-rtx.c (simplify_immed_subreg): Likewise. > * stor-layout.c (bitwise_type_for_mode): Update assert. > (element_precision): Remove.
Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
On 07/27/2015 04:25 AM, David Sherwood wrote: Hi, Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m) so that it returns m in cases where there is no inner mode. This simplifies some of the calling code by removing the need to check for VOIDmode and allows calling it unconditionally. I also removed element_precision () as it was only called in one place and thought it neater to call GET_MODE_PRECISION explicitly. Parts 2-4 will include further tidy-ups and optimisations based on [1/N]. Good to go? Regards, David Sherwood. 2015-07-17 David Sherwood gcc/ * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call GET_MODE_INNER unconditionally. * config/spu/spu.c (arith_immediate_p): Likewise. * config/i386/i386.c (ix86_build_signbit_mask): Likewise. New variable. * expmed.c (synth_mult): Remove check for VOIDmode result from GET_MODE_INNER. (expand_mult_const): Likewise. * fold-const.c (): Replace call to element_precision with call to GET_MODE_PRECISION. * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with m->name. (emit_mode_inner): Likewise. * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER result check. * machmode.h (GET_MODE_UNIT_SIZE): Simplify. (GET_MODE_UNIT_PRECISION): Likewise. * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally. * simplify-rtx.c (simplify_immed_subreg): Likewise. * stor-layout.c (bitwise_type_for_mode): Update assert. (element_precision): Remove. Somehow my brain kept translating INNER into NARROWER. Naturally I was having considerable trouble seeing how the patch could be correct ;-) Looking at insn-modes.h cleared things up quickly. In a lot of ways this makes GET_INNER_MODE act more like GET_MODE_NUNITS, which is probably good. You need to update the comment for GET_MODE_INNER in machmode.h to reflect the change in its return value for non-vector modes. With that update, this patch is fine. jeff
RE: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
> > On 07/27/2015 04:25 AM, David Sherwood wrote: > > Hi, > > > > Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER > > (m) > > so that it returns m in cases where there is no inner mode. This simplifies > > some > > of the calling code by removing the need to check for VOIDmode and allows > > calling it unconditionally. I also removed element_precision () as it was > > only > > called in one place and thought it neater to call GET_MODE_PRECISION > > explicitly. > > > > Parts 2-4 will include further tidy-ups and optimisations based on [1/N]. > > > > Good to go? > > > > Regards, > > David Sherwood. > > > > 2015-07-17 David Sherwood > > > > gcc/ > > * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call > > GET_MODE_INNER unconditionally. > > * config/spu/spu.c (arith_immediate_p): Likewise. > > * config/i386/i386.c (ix86_build_signbit_mask): Likewise. New > > variable. > > * expmed.c (synth_mult): Remove check for VOIDmode result from > > GET_MODE_INNER. > > (expand_mult_const): Likewise. > > * fold-const.c (): Replace call to element_precision with call to > > GET_MODE_PRECISION. > > * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with > > m->name. > > (emit_mode_inner): Likewise. > > * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER > > result check. > > * machmode.h (GET_MODE_UNIT_SIZE): Simplify. > > (GET_MODE_UNIT_PRECISION): Likewise. > > * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally. > > * simplify-rtx.c (simplify_immed_subreg): Likewise. > > * stor-layout.c (bitwise_type_for_mode): Update assert. > > (element_precision): Remove. > Somehow my brain kept translating INNER into NARROWER. Naturally I was > having considerable trouble seeing how the patch could be correct ;-) > Looking at insn-modes.h cleared things up quickly. > > In a lot of ways this makes GET_INNER_MODE act more like > GET_MODE_NUNITS, which is probably good. > > You need to update the comment for GET_MODE_INNER in machmode.h to > reflect the change in its return value for non-vector modes. Thanks for the quick response! Before I post a new patch, does this new comment seem ok? /* Where MODE represents a vector return the mode of the inner elements, otherwise just return MODE. */ Dave. > > With that update, this patch is fine. > > jeff
RE: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
Hi, I have updated the comment above GET_MODE_INNER and while there I have fixed a spelling mistake in the comment above GET_MODE_UNIT_SIZE. Tested: aarch64 and aarch64_be - no regressions in gcc testsuite x86_64 - bootstrap build, no testsuite regressions arm-none-eabi - no regressions in gcc testsuite Run contrib/config-list.mk - only build failures are ones that fail anyway with warnings being treated as errors. Hope this is ok. Cheers, Dave. 2015-07-28 David Sherwood gcc/ * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call GET_MODE_INNER unconditionally. * config/spu/spu.c (arith_immediate_p): Likewise. * config/i386/i386.c (ix86_build_signbit_mask): Likewise. New variable. * expmed.c (synth_mult): Remove check for VOIDmode result from GET_MODE_INNER. (expand_mult_const): Likewise. * fold-const.c (): Replace call to element_precision with call to GET_MODE_PRECISION. * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with m->name. (emit_mode_inner): Likewise. * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER result check. * machmode.h (GET_MODE_INNER): Update comment. (GET_MODE_UNIT_SIZE): Simplify and fix spelling mistake in comment. (GET_MODE_UNIT_PRECISION): Simplify. (element_precision): Remove. * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally. * simplify-rtx.c (simplify_immed_subreg): Likewise. * stor-layout.c (bitwise_type_for_mode): Update assert. (element_precision): Remove. > > On 07/27/2015 04:25 AM, David Sherwood wrote: > > Hi, > > > > Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER > > (m) > > so that it returns m in cases where there is no inner mode. This simplifies > > some > > of the calling code by removing the need to check for VOIDmode and allows > > calling it unconditionally. I also removed element_precision () as it was > > only > > called in one place and thought it neater to call GET_MODE_PRECISION > > explicitly. > > > > Parts 2-4 will include further tidy-ups and optimisations based on [1/N]. > > > > Good to go? > > > > Regards, > > David Sherwood. > > > > 2015-07-17 David Sherwood > > > > gcc/ > > * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call > > GET_MODE_INNER unconditionally. > > * config/spu/spu.c (arith_immediate_p): Likewise. > > * config/i386/i386.c (ix86_build_signbit_mask): Likewise. New > > variable. > > * expmed.c (synth_mult): Remove check for VOIDmode result from > > GET_MODE_INNER. > > (expand_mult_const): Likewise. > > * fold-const.c (): Replace call to element_precision with call to > > GET_MODE_PRECISION. > > * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with > > m->name. > > (emit_mode_inner): Likewise. > > * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER > > result check. > > * machmode.h (GET_MODE_UNIT_SIZE): Simplify. > > (GET_MODE_UNIT_PRECISION): Likewise. > > * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally. > > * simplify-rtx.c (simplify_immed_subreg): Likewise. > > * stor-layout.c (bitwise_type_for_mode): Update assert. > > (element_precision): Remove. > Somehow my brain kept translating INNER into NARROWER. Naturally I was > having considerable trouble seeing how the patch could be correct ;-) > Looking at insn-modes.h cleared things up quickly. > > In a lot of ways this makes GET_INNER_MODE act more like > GET_MODE_NUNITS, which is probably good. > > You need to update the comment for GET_MODE_INNER in machmode.h to > reflect the change in its return value for non-vector modes. > > With that update, this patch is fine. > > jeff mode_inner1.patch Description: Binary data
Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
"David Sherwood" writes: > Hi, > > I have updated the comment above GET_MODE_INNER and while there I have > fixed a spelling mistake in the comment above GET_MODE_UNIT_SIZE. > > Tested: > aarch64 and aarch64_be - no regressions in gcc testsuite > x86_64 - bootstrap build, no testsuite regressions > arm-none-eabi - no regressions in gcc testsuite > Run contrib/config-list.mk - only build failures are ones that fail anyway > with > warnings being treated as errors. > > Hope this is ok. > > Cheers, > Dave. Since Jeff conditionally approved the patch, I went ahead and applied it. Thanks, Richard
Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
On 07/28/2015 03:35 AM, David Sherwood wrote: On 07/27/2015 04:25 AM, David Sherwood wrote: Hi, Part 1 of this change is a clean-up. I have changed calls to GET_MODE_INNER (m) so that it returns m in cases where there is no inner mode. This simplifies some of the calling code by removing the need to check for VOIDmode and allows calling it unconditionally. I also removed element_precision () as it was only called in one place and thought it neater to call GET_MODE_PRECISION explicitly. Parts 2-4 will include further tidy-ups and optimisations based on [1/N]. Good to go? Regards, David Sherwood. 2015-07-17 David Sherwood gcc/ * config/arm/arm.c (neon_element_bits, neon_valid_immediate): Call GET_MODE_INNER unconditionally. * config/spu/spu.c (arith_immediate_p): Likewise. * config/i386/i386.c (ix86_build_signbit_mask): Likewise. New variable. * expmed.c (synth_mult): Remove check for VOIDmode result from GET_MODE_INNER. (expand_mult_const): Likewise. * fold-const.c (): Replace call to element_precision with call to GET_MODE_PRECISION. * genmodes.c (emit_mode_inner_inline): Replace void_mode->name with m->name. (emit_mode_inner): Likewise. * lto-streamer-out.c (lto_write_mode_table): Update GET_MODE_INNER result check. * machmode.h (GET_MODE_UNIT_SIZE): Simplify. (GET_MODE_UNIT_PRECISION): Likewise. * rtlanal.c (subreg_get_info): Call GET_MODE_INNER unconditionally. * simplify-rtx.c (simplify_immed_subreg): Likewise. * stor-layout.c (bitwise_type_for_mode): Update assert. (element_precision): Remove. Somehow my brain kept translating INNER into NARROWER. Naturally I was having considerable trouble seeing how the patch could be correct ;-) Looking at insn-modes.h cleared things up quickly. In a lot of ways this makes GET_INNER_MODE act more like GET_MODE_NUNITS, which is probably good. You need to update the comment for GET_MODE_INNER in machmode.h to reflect the change in its return value for non-vector modes. Thanks for the quick response! Before I post a new patch, does this new comment seem ok? /* Where MODE represents a vector return the mode of the inner elements, otherwise just return MODE. */ Yes, that's fine. Thanks, jeff
Re: [PATCH][1/N] Change GET_MODE_INNER to always return a non-void mode
On 07/28/2015 03:47 AM, David Sherwood wrote: Sorry, the comment should reflect complex types too. How about this? /* Return the mode of the basic parts of MODE. For vector modes this is the mode of the vector elements. For complex modes it is the mode of the real and imaginary parts. For other modes it is MODE itself. */ Even better :-) jeff