mbox series

[00/77] Add wrapper classes for machine_modes

Message ID 8760ewohsv.fsf@linaro.org
Headers show
Series Add wrapper classes for machine_modes | expand

Message

Richard Sandiford July 13, 2017, 8:35 a.m. UTC 
This series is an update of:

    https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00766.html

It adds a group of wrapper classes around machine_mode for modes that
are known to belong to, or need to belong to, a particular mode_class.
For example, it adds a scalar_int_mode wrapper for MODE_INT and
MODE_PARTIAL_INT modes, a scalar_float_mode wrapper for MODE_FLOAT and
MODE_DECIMAL_FLOAT modes, and so on.

These wrapper classes aren't meant to form an inheritance tree in the
way that rtx_insn or gimple do.  They really just bundle a machine_mode
with a static assertion that a particular condition holds.  In principle
there could be a wrapper class for any useful condition.

The reason for using wrapper classes is that they provide more
build-time checking that modes have the right class.  In the best
cases they avoid the need for any runtime assertions, but "at worst" they
force the assertion further up to the place that derives the mode, often
a TYPE_MODE or a GET_MODE.  Enforcing the condition at this level should
catch mistakes earlier and with less sensitivity to the following code
paths.  It should also make the basis for the assertion more obvious;
e.g. scalar_int_mode uses of TYPE_MODEs are often guarded at some level
by an INTEGRAL_TYPE_P test.

I know of three specific cases in which the static type checking
forced fixes for things that turned out to be real bugs (although
we didn't know that at the time, otherwise we'd have posted patches).
They were later fixed for trunk by:

    https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01783.html
    https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02983.html
    https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02896.html

It also found problems that seemed to be latent, but the fixes for those
went into GCC 7, so there are none to go with this iteration of the series.
The most common sources of problems seemed to be using VOIDmode or BLKmode
where a scalar integer was expected (e.g. when getting the number of bits
in the value), and simplifying vector operations in ways that only make
sense for scalars.

The series is part of the SVE submission and includes most of the
changes in group C from:

    https://gcc.gnu.org/ml/gcc/2016-11/msg00033.html

Since SVE has variable-length vectors, the SVE series as a whole needs
to change the size of a vector mode from being a known compile-time
constant to being (possibly) a run-time invariant.  It then needs to
do the same for unconstrained machine_modes, which might or might not be
vectors.  Introducing these new constrained types for scalar and complex
modes means that we can continue to treat them as having a constant size.

Mostly this is just a tweaked version of the original series,
rebased onto current trunk.  There is one significant difference
though: the first iteration treated machine_mode as an all-encompassing
wrapper class whereas this version keeps it as an enum.  Treating it as
a wrapper class made it consistent with the other classes and meant that
it would be easier in future to use member functions to access mode
properties (e.g. "mode.size ()" rather than "GET_MODE_SIZE (mode)")
if that was thought to be a good thing.  However, Martin's recent
memset/memcpy warning shows that there are still many places that
expect machine_modes to be POD-ish, so in the end it seemed better
to leave machine_mode as an enum for now.

Also, I've now split the scalar_mode patch into 11 separate patches,
since it was very difficult to review/reread in its old form.

In terms of compile time, the series is actually a very slight win
for --enable-checking=release builds and a more marked win for
--enable-checking=yes,rtl.  That might seem unlikely, but there are
several possible reasons:

(1) The series makes all the machmode.h macros that used:

      __builtin_constant_p (M) ? foo_inline (M) : foo_array[M]

    forward to an ALWAYS_INLINE function, so that (a) M is only evaluated
    once and (b) __builtin_constant_p is applied to a variable, and so is
    deferred until later passes.  This helped the optimisation to fire in
    more cases and to continue firing when M is a class rather than a
    raw enum.

(2) The series has a tendency to evaluate modes once, rather than
    continually fetching them from (sometimes quite deep) rtx nests.
    Refetching a mode is a particular problem if a call comes between
    two uses, since the compiler then has to re-evaluate the whole thing.

(3) The series introduces many uses of new SCALAR_*TYPE_MODE macros,
    as alternatives to TYPE_MODE.  The new macros avoid the usual:

      (VECTOR_TYPE_P (TYPE_CHECK (NODE)) \
       ? vector_type_mode (NODE) : (NODE)->type_common.mode)

    and become direct field accesses in release builds.

    VECTOR_TYPE_P would be consistently false for these uses, and thus
    predictable at runtime.  However, we can't predict them to be false
    at compile time without profile feedback, since in other uses of
    TYPE_MODE the VECTOR_TYPE_P condition can be quite likely or even
    guaranteed.  This means (for example) that call-clobbered registers
    would usually be treated as clobbered by the condition as a whole.

I tested this by compiling the testsuite for:

    aarch64-linux-gnu alpha-linux-gnu arc-elf arm-linux-gnueabi
    arm-linux-gnueabihf avr-elf bfin-elf c6x-elf cr16-elf cris-elf
    epiphany-elf fr30-elf frv-linux-gnu ft32-elf h8300-elf
    hppa64-hp-hpux11.23 ia64-linux-gnu i686-pc-linux-gnu
    i686-apple-darwin iq2000-elf lm32-elf m32c-elf m32r-elf
    m68k-linux-gnu mcore-elf microblaze-elf mips-linux-gnu
    mipsisa64-linux-gnu mmix mn10300-elf moxie-rtems msp430-elf
    nds32le-elf nios2-linux-gnu nvptx-none pdp11 powerpc-linux-gnuspe
    powerpc-eabispe powerpc64-linux-gnu powerpc-ibm-aix7.0 riscv64-elf
    rl78-elf rx-elf s390-linux-gnu s390x-linux-gnu sh-linux-gnu
    sparc-linux-gnu sparc64-linux-gnu sparc-wrs-vxworks spu-elf
    tilegx-elf tilepro-elf xstormy16-elf v850-elf vax-netbsdelf
    visium-elf x86_64-darwin x86_64-linux-gnu xtensa-elf

and checking that there were no changes in assembly.  Also tested
in the normal way on aarch64-linux-gnu, powerc64-linux-gnu and
x86_64-linux-gnu.

Thanks,
Richard

Comments

Richard Sandiford July 13, 2017, 8:38 a.m. UTC | #1 
All case statements need to be updated to use the prefixed names,
since the unprefixed names will eventually not be integer constant
expressions.  This patch does a mechanical substitution over the whole
codebase.

2017-07-13  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* config/aarch64/aarch64-builtins.c (aarch64_simd_builtin_std_type):
	Prefix mode names with E_ in case statements.
	* config/aarch64/aarch64-elf.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.
	* config/aarch64/aarch64.c (aarch64_split_simd_combine): Likewise.
	(aarch64_split_simd_move): Likewise.
	(aarch64_gen_storewb_pair): Likewise.
	(aarch64_gen_loadwb_pair): Likewise.
	(aarch64_gen_store_pair): Likewise.
	(aarch64_gen_load_pair): Likewise.
	(aarch64_get_condition_code_1): Likewise.
	(aarch64_constant_pool_reload_icode): Likewise.
	(get_rsqrte_type): Likewise.
	(get_rsqrts_type): Likewise.
	(get_recpe_type): Likewise.
	(get_recps_type): Likewise.
	(aarch64_gimplify_va_arg_expr): Likewise.
	(aarch64_simd_container_mode): Likewise.
	(aarch64_emit_load_exclusive): Likewise.
	(aarch64_emit_store_exclusive): Likewise.
	(aarch64_expand_compare_and_swap): Likewise.
	(aarch64_gen_atomic_cas): Likewise.
	(aarch64_emit_bic): Likewise.
	(aarch64_emit_atomic_swap): Likewise.
	(aarch64_emit_atomic_load_op): Likewise.
	(aarch64_evpc_trn): Likewise.
	(aarch64_evpc_uzp): Likewise.
	(aarch64_evpc_zip): Likewise.
	(aarch64_evpc_ext): Likewise.
	(aarch64_evpc_rev): Likewise.
	(aarch64_evpc_dup): Likewise.
	(aarch64_gen_ccmp_first): Likewise.
	(aarch64_gen_ccmp_next): Likewise.
	* config/alpha/alpha.c (alpha_scalar_mode_supported_p): Likewise.
	(alpha_emit_xfloating_libcall): Likewise.
	(emit_insxl): Likewise.
	(alpha_arg_type): Likewise.
	* config/arc/arc.c (arc_vector_mode_supported_p): Likewise.
	(arc_preferred_simd_mode): Likewise.
	(arc_secondary_reload): Likewise.
	(get_arc_condition_code): Likewise.
	(arc_print_operand): Likewise.
	* config/arc/arc.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.
	* config/arc/arc.md (casesi_load): Likewise.
	(casesi_compact_jump): Likewise.
	* config/arc/predicates.md (proper_comparison_operator): Likewise.
	(cc_use_register): Likewise.
	* config/arm/aout.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.
	* config/arm/arm-builtins.c (arm_simd_builtin_std_type): Likewise.
	(arm_init_iwmmxt_builtins): Likewise.
	* config/arm/arm.c (thumb1_size_rtx_costs): Likewise.
	(neon_expand_vector_init): Likewise.
	(arm_attr_length_move_neon): Likewise.
	(maybe_get_arm_condition_code): Likewise.
	(arm_emit_vector_const): Likewise.
	(arm_preferred_simd_mode): Likewise.
	(arm_output_iwmmxt_tinsr): Likewise.
	(thumb1_output_casesi): Likewise.
	(thumb2_output_casesi): Likewise.
	(arm_emit_load_exclusive): Likewise.
	(arm_emit_store_exclusive): Likewise.
	(arm_expand_compare_and_swap): Likewise.
	(arm_evpc_neon_vuzp): Likewise.
	(arm_evpc_neon_vzip): Likewise.
	(arm_evpc_neon_vrev): Likewise.
	(arm_evpc_neon_vtrn): Likewise.
	(arm_evpc_neon_vext): Likewise.
	(arm_validize_comparison): Likewise.
	* config/arm/neon.md (neon_vc<cmp_op><mode>): Likewise.
	* config/avr/avr-c.c (avr_resolve_overloaded_builtin): Likewise.
	* config/avr/avr.c (avr_rtx_costs_1): Likewise.
	* config/c6x/c6x.c (c6x_vector_mode_supported_p): Likewise.
	(c6x_preferred_simd_mode): Likewise.
	* config/epiphany/epiphany.c (get_epiphany_condition_code): Likewise.
	(epiphany_rtx_costs): Likewise.
	* config/epiphany/predicates.md (proper_comparison_operator):
	Likewise.
	* config/frv/frv.c (condexec_memory_operand): Likewise.
	(frv_emit_move): Likewise.
	(output_move_single): Likewise.
	(output_condmove_single): Likewise.
	(frv_hard_regno_mode_ok): Likewise.
	(frv_matching_accg_mode): Likewise.
	* config/h8300/h8300.c (split_adds_subs): Likewise.
	(h8300_rtx_costs): Likewise.
	(h8300_print_operand): Likewise.
	(compute_mov_length): Likewise.
	(output_logical_op): Likewise.
	(compute_logical_op_length): Likewise.
	(compute_logical_op_cc): Likewise.
	(h8300_shift_needs_scratch_p): Likewise.
	(output_a_shift): Likewise.
	(compute_a_shift_length): Likewise.
	(compute_a_shift_cc): Likewise.
	(expand_a_rotate): Likewise.
	(output_a_rotate): Likewise.
	* config/i386/i386.c (classify_argument): Likewise.
	(function_arg_advance_32): Likewise.
	(function_arg_32): Likewise.
	(function_arg_64): Likewise.
	(function_value_64): Likewise.
	(ix86_gimplify_va_arg): Likewise.
	(ix86_legitimate_constant_p): Likewise.
	(put_condition_code): Likewise.
	(split_double_mode): Likewise.
	(ix86_avx256_split_vector_move_misalign): Likewise.
	(ix86_expand_vector_logical_operator): Likewise.
	(ix86_split_idivmod): Likewise.
	(ix86_expand_adjust_ufix_to_sfix_si): Likewise.
	(ix86_build_const_vector): Likewise.
	(ix86_build_signbit_mask): Likewise.
	(ix86_match_ccmode): Likewise.
	(ix86_cc_modes_compatible): Likewise.
	(ix86_expand_branch): Likewise.
	(ix86_expand_sse_cmp): Likewise.
	(ix86_expand_sse_movcc): Likewise.
	(ix86_expand_int_sse_cmp): Likewise.
	(ix86_expand_vec_perm_vpermi2): Likewise.
	(ix86_expand_vec_perm): Likewise.
	(ix86_expand_sse_unpack): Likewise.
	(ix86_expand_int_addcc): Likewise.
	(ix86_split_to_parts): Likewise.
	(ix86_vectorize_builtin_gather): Likewise.
	(ix86_vectorize_builtin_scatter): Likewise.
	(avx_vpermilp_parallel): Likewise.
	(inline_memory_move_cost): Likewise.
	(ix86_tieable_integer_mode_p): Likewise.
	(x86_maybe_negate_const_int): Likewise.
	(ix86_expand_vector_init_duplicate): Likewise.
	(ix86_expand_vector_init_one_nonzero): Likewise.
	(ix86_expand_vector_init_one_var): Likewise.
	(ix86_expand_vector_init_concat): Likewise.
	(ix86_expand_vector_init_interleave): Likewise.
	(ix86_expand_vector_init_general): Likewise.
	(ix86_expand_vector_set): Likewise.
	(ix86_expand_vector_extract): Likewise.
	(emit_reduc_half): Likewise.
	(ix86_emit_i387_round): Likewise.
	(ix86_mangle_type): Likewise.
	(ix86_expand_round_sse4): Likewise.
	(expand_vec_perm_blend): Likewise.
	(canonicalize_vector_int_perm): Likewise.
	(ix86_expand_vec_one_operand_perm_avx512): Likewise.
	(expand_vec_perm_1): Likewise.
	(expand_vec_perm_interleave3): Likewise.
	(expand_vec_perm_even_odd_pack): Likewise.
	(expand_vec_perm_even_odd_1): Likewise.
	(expand_vec_perm_broadcast_1): Likewise.
	(ix86_vectorize_vec_perm_const_ok): Likewise.
	(ix86_expand_vecop_qihi): Likewise.
	(ix86_expand_mul_widen_hilo): Likewise.
	(ix86_expand_sse2_abs): Likewise.
	(ix86_expand_pextr): Likewise.
	(ix86_expand_pinsr): Likewise.
	(ix86_preferred_simd_mode): Likewise.
	(ix86_simd_clone_compute_vecsize_and_simdlen): Likewise.
	* config/i386/sse.md (*andnot<mode>3): Likewise.
	(<mask_codefor><code><mode>3<mask_name>): Likewise.
	(*<code><mode>3): Likewise.
	* config/ia64/ia64.c (ia64_expand_vecint_compare): Likewise.
	(ia64_expand_atomic_op): Likewise.
	(ia64_arg_type): Likewise.
	(ia64_mode_to_int): Likewise.
	(ia64_scalar_mode_supported_p): Likewise.
	(ia64_vector_mode_supported_p): Likewise.
	(expand_vec_perm_broadcast): Likewise.
	* config/iq2000/iq2000.c (iq2000_move_1word): Likewise.
	(iq2000_function_arg_advance): Likewise.
	(iq2000_function_arg): Likewise.
	* config/m32c/m32c.c (m32c_preferred_reload_class): Likewise.
	* config/m68k/m68k.c (output_dbcc_and_branch): Likewise.
	(m68k_libcall_value): Likewise.
	(m68k_function_value): Likewise.
	(sched_attr_op_type): Likewise.
	* config/mcore/mcore.c (mcore_output_move): Likewise.
	* config/microblaze/microblaze.c (microblaze_function_arg_advance):
	Likewise.
	(microblaze_function_arg): Likewise.
	* config/mips/mips.c (mips16_build_call_stub): Likewise.
	(mips_print_operand): Likewise.
	(mips_mode_ok_for_mov_fmt_p): Likewise.
	(mips_vector_mode_supported_p): Likewise.
	(mips_preferred_simd_mode): Likewise.
	(mips_expand_vpc_loongson_even_odd): Likewise.
	(mips_expand_vec_unpack): Likewise.
	(mips_expand_vi_broadcast): Likewise.
	(mips_expand_vector_init): Likewise.
	(mips_expand_vec_reduc): Likewise.
	(mips_expand_msa_cmp): Likewise.
	* config/mips/mips.md (casesi_internal_mips16_<mode>): Likewise.
	* config/mn10300/mn10300.c (mn10300_print_operand): Likewise.
	(cc_flags_for_mode): Likewise.
	* config/msp430/msp430.c (msp430_print_operand): Likewise.
	* config/nds32/nds32-md-auxiliary.c (nds32_mem_format): Likewise.
	(nds32_output_casesi_pc_relative): Likewise.
	* config/nds32/nds32.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.
	* config/nvptx/nvptx.c (nvptx_ptx_type_from_mode): Likewise.
	(nvptx_gen_unpack): Likewise.
	(nvptx_gen_pack): Likewise.
	(nvptx_gen_shuffle): Likewise.
	(nvptx_gen_wcast): Likewise.
	* config/pa/pa.c (pa_secondary_reload): Likewise.
	* config/pa/predicates.md (base14_operand): Likewise.
	* config/powerpcspe/powerpcspe-c.c
	(altivec_resolve_overloaded_builtin): Likewise.
	* config/powerpcspe/powerpcspe.c (rs6000_setup_reg_addr_masks):
	Likewise.
	(rs6000_preferred_simd_mode): Likewise.
	(output_vec_const_move): Likewise.
	(rs6000_expand_vector_extract): Likewise.
	(rs6000_split_vec_extract_var): Likewise.
	(reg_offset_addressing_ok_p): Likewise.
	(rs6000_legitimate_offset_address_p): Likewise.
	(rs6000_legitimize_address): Likewise.
	(rs6000_emit_set_const): Likewise.
	(rs6000_const_vec): Likewise.
	(rs6000_emit_move): Likewise.
	(spe_build_register_parallel): Likewise.
	(rs6000_darwin64_record_arg_recurse): Likewise.
	(swap_selector_for_mode): Likewise.
	(spe_init_builtins): Likewise.
	(paired_init_builtins): Likewise.
	(altivec_init_builtins): Likewise.
	(do_load_for_compare): Likewise.
	(rs6000_generate_compare): Likewise.
	(rs6000_expand_float128_convert): Likewise.
	(emit_load_locked): Likewise.
	(emit_store_conditional): Likewise.
	(rs6000_output_function_epilogue): Likewise.
	(rs6000_handle_altivec_attribute): Likewise.
	(rs6000_function_value): Likewise.
	(emit_fusion_gpr_load): Likewise.
	(emit_fusion_p9_load): Likewise.
	(emit_fusion_p9_store): Likewise.
	* config/powerpcspe/predicates.md (easy_fp_constant): Likewise.
	(fusion_gpr_mem_load): Likewise.
	(fusion_addis_mem_combo_load): Likewise.
	(fusion_addis_mem_combo_store): Likewise.
	* config/rs6000/predicates.md (easy_fp_constant): Likewise.
	(fusion_gpr_mem_load): Likewise.
	(fusion_addis_mem_combo_load): Likewise.
	(fusion_addis_mem_combo_store): Likewise.
	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
	Likewise.
	* config/rs6000/rs6000-string.c (do_load_for_compare): Likewise.
	* config/rs6000/rs6000.c (rs6000_setup_reg_addr_masks): Likewise.
	(rs6000_preferred_simd_mode): Likewise.
	(output_vec_const_move): Likewise.
	(rs6000_expand_vector_extract): Likewise.
	(rs6000_split_vec_extract_var): Likewise.
	(reg_offset_addressing_ok_p): Likewise.
	(rs6000_legitimate_offset_address_p): Likewise.
	(rs6000_legitimize_address): Likewise.
	(rs6000_emit_set_const): Likewise.
	(rs6000_const_vec): Likewise.
	(rs6000_emit_move): Likewise.
	(rs6000_darwin64_record_arg_recurse): Likewise.
	(swap_selector_for_mode): Likewise.
	(paired_init_builtins): Likewise.
	(altivec_init_builtins): Likewise.
	(rs6000_expand_float128_convert): Likewise.
	(emit_load_locked): Likewise.
	(emit_store_conditional): Likewise.
	(rs6000_output_function_epilogue): Likewise.
	(rs6000_handle_altivec_attribute): Likewise.
	(rs6000_function_value): Likewise.
	(emit_fusion_gpr_load): Likewise.
	(emit_fusion_p9_load): Likewise.
	(emit_fusion_p9_store): Likewise.
	* config/rx/rx.c (rx_gen_move_template): Likewise.
	(flags_from_mode): Likewise.
	* config/s390/predicates.md (s390_alc_comparison): Likewise.
	(s390_slb_comparison): Likewise.
	* config/s390/s390.c (s390_handle_vectorbool_attribute): Likewise.
	(s390_vector_mode_supported_p): Likewise.
	(s390_cc_modes_compatible): Likewise.
	(s390_match_ccmode_set): Likewise.
	(s390_canonicalize_comparison): Likewise.
	(s390_emit_compare_and_swap): Likewise.
	(s390_branch_condition_mask): Likewise.
	(s390_rtx_costs): Likewise.
	(s390_secondary_reload): Likewise.
	(__SECONDARY_RELOAD_CASE): Likewise.
	(s390_expand_cs): Likewise.
	(s390_preferred_simd_mode): Likewise.
	* config/s390/vx-builtins.md (vec_packsu_u<mode>): Likewise.
	* config/sh/sh.c (sh_print_operand): Likewise.
	(dump_table): Likewise.
	(sh_secondary_reload): Likewise.
	* config/sh/sh.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.
	* config/sh/sh.md (casesi_worker_1): Likewise.
	(casesi_worker_2): Likewise.
	* config/sparc/predicates.md (icc_comparison_operator): Likewise.
	(fcc_comparison_operator): Likewise.
	* config/sparc/sparc.c (sparc_expand_move): Likewise.
	(emit_soft_tfmode_cvt): Likewise.
	(sparc_preferred_simd_mode): Likewise.
	(output_cbranch): Likewise.
	(sparc_print_operand): Likewise.
	(sparc_expand_vec_perm_bmask): Likewise.
	(vector_init_bshuffle): Likewise.
	* config/spu/spu.c (spu_scalar_mode_supported_p): Likewise.
	(spu_vector_mode_supported_p): Likewise.
	(spu_expand_insv): Likewise.
	(spu_emit_branch_or_set): Likewise.
	(spu_handle_vector_attribute): Likewise.
	(spu_builtin_splats): Likewise.
	(spu_builtin_extract): Likewise.
	(spu_builtin_promote): Likewise.
	(spu_expand_sign_extend): Likewise.
	* config/tilegx/tilegx.c (tilegx_scalar_mode_supported_p): Likewise.
	(tilegx_simd_int): Likewise.
	* config/tilepro/tilepro.c (tilepro_scalar_mode_supported_p): Likewise.
	(tilepro_simd_int): Likewise.
	* config/v850/v850.c (const_double_split): Likewise.
	(v850_print_operand): Likewise.
	(ep_memory_offset): Likewise.
	* config/vax/vax.c (vax_rtx_costs): Likewise.
	(vax_output_int_move): Likewise.
	(vax_output_int_add): Likewise.
	(vax_output_int_subtract): Likewise.
	* config/visium/predicates.md (visium_branch_operator): Likewise.
	* config/visium/visium.c (rtx_ok_for_offset_p): Likewise.
	(visium_print_operand_address): Likewise.
	* config/visium/visium.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.
	* config/xtensa/xtensa.c (xtensa_mem_offset): Likewise.
	(xtensa_expand_conditional_branch): Likewise.
	(xtensa_copy_incoming_a7): Likewise.
	(xtensa_output_literal): Likewise.
	* dfp.c (decimal_real_maxval): Likewise.
	* targhooks.c (default_libgcc_floating_mode_supported_p): Likewise.

gcc/c-family/
	* c-cppbuiltin.c (mode_has_fma): Prefix mode names with E_ in
	case statements.

gcc/objc/
	* objc-encoding.c (encode_gnu_bitfield): Prefix mode names with E_ in
	case statements.

libobjc/
	* encoding.c (_darwin_rs6000_special_round_type_align): Prefix mode
	names with E_ in case statements.
Richard Sandiford July 13, 2017, 9 a.m. UTC | #2 
This patch changes the types of various things from machine_mode
to scalar_int_mode, in cases where (after previous patches)
simply changing the type is enough on its own.  The patch does
nothing other than that.

2017-07-13  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* builtins.h (builtin_strncpy_read_str): Take a scalar_int_mode
	instead of a machine_mode.
	(builtin_memset_read_str): Likewise.
	* builtins.c (c_readstr): Likewise.
	(builtin_memcpy_read_str): Likewise.
	(builtin_strncpy_read_str): Likewise.
	(builtin_memset_read_str): Likewise.
	(builtin_memset_gen_str): Likewise.
	(expand_builtin_signbit): Use scalar_int_mode for local variables.
	* cfgexpand.c (convert_debug_memory_address): Take a scalar_int_mode
	instead of a machine_mode.
	* combine.c (simplify_if_then_else): Use scalar_int_mode for local
	variables.
	(make_extraction): Likewise.
	(try_widen_shift_mode): Take and return scalar_int_modes instead
	of machine_modes.
	* config/aarch64/aarch64.c (aarch64_libgcc_cmp_return_mode): Return
	a scalar_int_mode instead of a machine_mode.
	* config/avr/avr.c (avr_addr_space_address_mode): Likewise.
	(avr_addr_space_pointer_mode): Likewise.
	* config/cr16/cr16.c (cr16_unwind_word_mode): Likewise.
	* config/msp430/msp430.c (msp430_addr_space_pointer_mode): Likewise.
	(msp430_unwind_word_mode): Likewise.
	* config/spu/spu.c (spu_unwind_word_mode): Likewise.
	(spu_addr_space_pointer_mode): Likewise.
	(spu_addr_space_address_mode): Likewise.
	(spu_libgcc_cmp_return_mode): Likewise.
	(spu_libgcc_shift_count_mode): Likewise.
	* config/rl78/rl78.c (rl78_addr_space_address_mode): Likewise.
	(rl78_addr_space_pointer_mode): Likewise.
	(fl78_unwind_word_mode): Likewise.
	(rl78_valid_pointer_mode): Take a scalar_int_mode instead of a
	machine_mode.
	* config/alpha/alpha.c (vms_valid_pointer_mode): Likewise.
	* config/ia64/ia64.c (ia64_vms_valid_pointer_mode): Likewise.
	* config/mips/mips.c (mips_mode_rep_extended): Likewise.
	(mips_valid_pointer_mode): Likewise.
	* config/tilegx/tilegx.c (tilegx_mode_rep_extended): Likewise.
	* config/ft32/ft32.c (ft32_valid_pointer_mode): Likewise.
	(ft32_addr_space_pointer_mode): Return a scalar_int_mode instead
	of a machine_mode.
	(ft32_addr_space_address_mode): Likewise.
	* config/m32c/m32c.c (m32c_valid_pointer_mode): Take a
	scalar_int_mode instead of a machine_mode.
	(m32c_addr_space_pointer_mode): Return a scalar_int_mode instead
	of a machine_mode.
	(m32c_addr_space_address_mode): Likewise.
	* config/powerpcspe/powerpcspe.c (rs6000_abi_word_mode): Likewise.
	(rs6000_eh_return_filter_mode): Likewise.
	* config/rs6000/rs6000.c (rs6000_abi_word_mode): Likewise.
	(rs6000_eh_return_filter_mode): Likewise.
	* config/s390/s390.c (s390_libgcc_cmp_return_mode): Likewise.
	(s390_libgcc_shift_count_mode): Likewise.
	(s390_unwind_word_mode): Likewise.
	(s390_valid_pointer_mode): Take a scalar_int_mode rather than a
	machine_mode.
	* target.def (mode_rep_extended): Likewise.
	(valid_pointer_mode): Likewise.
	(addr_space.valid_pointer_mode): Likewise.
	(eh_return_filter_mode): Return a scalar_int_mode rather than
	a machine_mode.
	(libgcc_cmp_return_mode): Likewise.
	(libgcc_shift_count_mode): Likewise.
	(unwind_word_mode): Likewise.
	(addr_space.pointer_mode): Likewise.
	(addr_space.address_mode): Likewise.
	* doc/tm.texi: Regenerate.
	* dojump.c (prefer_and_bit_test): Take a scalar_int_mode rather than
	a machine_mode.
	(do_jump): Use scalar_int_mode for local variables.
	* dwarf2cfi.c (init_return_column_size): Take a scalar_int_mode
	rather than a machine_mode.
	* dwarf2out.c (convert_descriptor_to_mode): Likewise.
	(scompare_loc_descriptor_wide): Likewise.
	(scompare_loc_descriptor_narrow): Likewise.
	* emit-rtl.c (adjust_address_1): Use scalar_int_mode for local
	variables.
	* except.c (sjlj_emit_dispatch_table): Likewise.
	(expand_builtin_eh_copy_values): Likewise.
	* explow.c (convert_memory_address_addr_space_1): Likewise.
	Take a scalar_int_mode rather than a machine_mode.
	(convert_memory_address_addr_space): Take a scalar_int_mode rather
	than a machine_mode.
	(memory_address_addr_space): Use scalar_int_mode for local variables.
	* expmed.h (expand_mult_highpart_adjust): Take a scalar_int_mode
	rather than a machine_mode.
	* expmed.c (mask_rtx): Likewise.
	(init_expmed_one_conv): Likewise.
	(expand_mult_highpart_adjust): Likewise.
	(extract_high_half): Likewise.
	(expmed_mult_highpart_optab): Likewise.
	(expmed_mult_highpart): Likewise.
	(expand_smod_pow2): Likewise.
	(expand_sdiv_pow2): Likewise.
	(emit_store_flag_int): Likewise.
	(adjust_bit_field_mem_for_reg): Use scalar_int_mode for local
	variables.
	(extract_low_bits): Likewise.
	* expr.h (by_pieces_constfn): Take a scalar_int_mode rather than
	a machine_mode.
	* expr.c (pieces_addr::adjust):  Likewise.
	(can_store_by_pieces): Likewise.
	(store_by_pieces): Likewise.
	(clear_by_pieces_1): Likewise.
	(expand_expr_addr_expr_1): Likewise.
	(expand_expr_addr_expr): Use scalar_int_mode for local variables.
	(expand_expr_real_1): Likewise.
	(try_casesi): Likewise.
	* final.c (shorten_branches): Likewise.
	* fold-const.c (fold_convert_const_int_from_fixed): Change the
	type of "mode" to machine_mode.
	* internal-fn.c (expand_arith_overflow_result_store): Take a
	scalar_int_mode rather than a machine_mode.
	(expand_mul_overflow): Use scalar_int_mode for local variables.
	* loop-doloop.c (doloop_modify): Likewise.
	(doloop_optimize): Likewise.
	* optabs.c (expand_subword_shift): Take a scalar_int_mode rather
	than a machine_mode.
	(expand_doubleword_shift_condmove): Likewise.
	(expand_doubleword_shift): Likewise.
	(expand_doubleword_clz): Likewise.
	(expand_doubleword_popcount): Likewise.
	(expand_doubleword_parity): Likewise.
	(expand_absneg_bit): Use scalar_int_mode for local variables.
	(prepare_float_lib_cmp): Likewise.
	* rtl.h (convert_memory_address_addr_space_1): Take a scalar_int_mode
	rather than a machine_mode.
	(convert_memory_address_addr_space): Likewise.
	(get_mode_bounds): Likewise.
	(get_address_mode): Return a scalar_int_mode rather than a
	machine_mode.
	* rtlanal.c (get_address_mode): Likewise.
	* stor-layout.c (get_mode_bounds): Take a scalar_int_mode rather
	than a machine_mode.
	* targhooks.c (default_mode_rep_extended): Likewise.
	(default_valid_pointer_mode): Likewise.
	(default_addr_space_valid_pointer_mode): Likewise.
	(default_eh_return_filter_mode): Return a scalar_int_mode rather
	than a machine_mode.
	(default_libgcc_cmp_return_mode): Likewise.
	(default_libgcc_shift_count_mode): Likewise.
	(default_unwind_word_mode): Likewise.
	(default_addr_space_pointer_mode): Likewise.
	(default_addr_space_address_mode): Likewise.
	* targhooks.h (default_eh_return_filter_mode): Likewise.
	(default_libgcc_cmp_return_mode): Likewise.
	(default_libgcc_shift_count_mode): Likewise.
	(default_unwind_word_mode): Likewise.
	(default_addr_space_pointer_mode): Likewise.
	(default_addr_space_address_mode): Likewise.
	(default_mode_rep_extended): Take a scalar_int_mode rather than
	a machine_mode.
	(default_valid_pointer_mode): Likewise.
	(default_addr_space_valid_pointer_mode): Likewise.
	* tree-ssa-address.c (addr_for_mem_ref): Use scalar_int_mode for
	local variables.
	* tree-ssa-loop-ivopts.c (get_shiftadd_cost): Take a scalar_int_mode
	rather than a machine_mode.
	* tree-switch-conversion.c (array_value_type): Use scalar_int_mode
	for local variables.
	* tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.
	* var-tracking.c (use_narrower_mode): Take a scalar_int_mode rather
	than a machine_mode.
Segher Boessenkool July 22, 2017, 9:02 p.m. UTC | #3 
Hi Richard,

On Thu, Jul 13, 2017 at 09:35:44AM +0100, Richard Sandiford wrote:
> This series is an update of:

> 

>     https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00766.html

> 

> It adds a group of wrapper classes around machine_mode for modes that

> are known to belong to, or need to belong to, a particular mode_class.

> For example, it adds a scalar_int_mode wrapper for MODE_INT and

> MODE_PARTIAL_INT modes, a scalar_float_mode wrapper for MODE_FLOAT and

> MODE_DECIMAL_FLOAT modes, and so on.


<huge snip>

So what does it change in the interfaces we use?  I couldn't find an
update of documentation, maybe I missed it (it's a huge series :-) )
An overview of the new interfaces (and how they are used) would help.

From what I can tell so far it makes things much harder to read.
Perhaps that is just because this is all new.

> I tested this by compiling the testsuite for:

> 

>     aarch64-linux-gnu alpha-linux-gnu arc-elf arm-linux-gnueabi

>     arm-linux-gnueabihf avr-elf bfin-elf c6x-elf cr16-elf cris-elf

>     epiphany-elf fr30-elf frv-linux-gnu ft32-elf h8300-elf

>     hppa64-hp-hpux11.23 ia64-linux-gnu i686-pc-linux-gnu

>     i686-apple-darwin iq2000-elf lm32-elf m32c-elf m32r-elf

>     m68k-linux-gnu mcore-elf microblaze-elf mips-linux-gnu

>     mipsisa64-linux-gnu mmix mn10300-elf moxie-rtems msp430-elf

>     nds32le-elf nios2-linux-gnu nvptx-none pdp11 powerpc-linux-gnuspe

>     powerpc-eabispe powerpc64-linux-gnu powerpc-ibm-aix7.0 riscv64-elf

>     rl78-elf rx-elf s390-linux-gnu s390x-linux-gnu sh-linux-gnu

>     sparc-linux-gnu sparc64-linux-gnu sparc-wrs-vxworks spu-elf

>     tilegx-elf tilepro-elf xstormy16-elf v850-elf vax-netbsdelf

>     visium-elf x86_64-darwin x86_64-linux-gnu xtensa-elf

> 

> and checking that there were no changes in assembly.  Also tested

> in the normal way on aarch64-linux-gnu, powerc64-linux-gnu and

> x86_64-linux-gnu.


Could you also test powerpc64le-linux please?  It is a primary
platform.  gcc112 is a nice fast machine.


Segher
Richard Sandiford July 24, 2017, 9:28 a.m. UTC | #4 
Segher Boessenkool <segher@kernel.crashing.org> writes:
> Hi Richard,

>

> On Thu, Jul 13, 2017 at 09:35:44AM +0100, Richard Sandiford wrote:

>> This series is an update of:

>> 

>>     https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00766.html

>> 

>> It adds a group of wrapper classes around machine_mode for modes that

>> are known to belong to, or need to belong to, a particular mode_class.

>> For example, it adds a scalar_int_mode wrapper for MODE_INT and

>> MODE_PARTIAL_INT modes, a scalar_float_mode wrapper for MODE_FLOAT and

>> MODE_DECIMAL_FLOAT modes, and so on.

>

> <huge snip>

>

> So what does it change in the interfaces we use?  I couldn't find an

> update of documentation, maybe I missed it (it's a huge series :-) )

> An overview of the new interfaces (and how they are used) would help.


You didn't miss one.  I was hoping the function comments would be enough,
but on reflection, they're not.  I've attached a patch for rtl.texi below.

> From what I can tell so far it makes things much harder to read.

> Perhaps that is just because this is all new.


Which parts specifically?  E.g. is it mostly the is_a <T> (x, &y) changes?
Or the as_a <T> (x) changes too?  Do you think the FOR_EACH_* iterators
also make things harder to read?  Or is machine_mode->scalar_int_mode
itself a problem?

>> I tested this by compiling the testsuite for:

>> 

>>     aarch64-linux-gnu alpha-linux-gnu arc-elf arm-linux-gnueabi

>>     arm-linux-gnueabihf avr-elf bfin-elf c6x-elf cr16-elf cris-elf

>>     epiphany-elf fr30-elf frv-linux-gnu ft32-elf h8300-elf

>>     hppa64-hp-hpux11.23 ia64-linux-gnu i686-pc-linux-gnu

>>     i686-apple-darwin iq2000-elf lm32-elf m32c-elf m32r-elf

>>     m68k-linux-gnu mcore-elf microblaze-elf mips-linux-gnu

>>     mipsisa64-linux-gnu mmix mn10300-elf moxie-rtems msp430-elf

>>     nds32le-elf nios2-linux-gnu nvptx-none pdp11 powerpc-linux-gnuspe

>>     powerpc-eabispe powerpc64-linux-gnu powerpc-ibm-aix7.0 riscv64-elf

>>     rl78-elf rx-elf s390-linux-gnu s390x-linux-gnu sh-linux-gnu

>>     sparc-linux-gnu sparc64-linux-gnu sparc-wrs-vxworks spu-elf

>>     tilegx-elf tilepro-elf xstormy16-elf v850-elf vax-netbsdelf

>>     visium-elf x86_64-darwin x86_64-linux-gnu xtensa-elf

>> 

>> and checking that there were no changes in assembly.  Also tested

>> in the normal way on aarch64-linux-gnu, powerc64-linux-gnu and

>> x86_64-linux-gnu.

>

> Could you also test powerpc64le-linux please?  It is a primary

> platform.  gcc112 is a nice fast machine.


Sorry, I meant powerpc64le-linux-gnu rather than powerpc64-linux-gnu.
It was indeed gcc112 that I used.

Thanks,
Richard


2017-07-24  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* doc/rtl.texi: Document machine_mode wrapper classes.Index: gcc/doc/rtl.texi
===================================================================
--- gcc/doc/rtl.texi	2017-07-24 10:11:36.155904178 +0100
+++ gcc/doc/rtl.texi	2017-07-24 10:11:40.484732386 +0100
@@ -1402,6 +1402,108 @@ classes.  Currently @code{VOIDmode} and
 @code{MODE_RANDOM}.
 @end table
 
+@cindex machine mode wrapper classes
+@code{machmode.h} also defines various wrapper classes that combine a
+@code{machine_mode} with a static assertion that a particular
+condition holds.  The classes are:
+
+@table @code
+@findex scalar_int_mode
+@item scalar_int_mode
+A mode that has class @code{MODE_INT} or @code{MODE_PARTIAL_INT}.
+
+@findex scalar_float_mode
+@item scalar_float_mode
+A mode that has class @code{MODE_FLOAT} or @code{MODE_DECIMAL_FLOAT}.
+
+@findex scalar_mode
+@item scalar_mode
+A mode that holds a single numerical value.  In practice this means
+that the mode is a @code{scalar_int_mode}, is a @code{scalar_float_mode},
+or has class @code{MODE_FRACT}, @code{MODE_UFRACT}, @code{MODE_ACCUM},
+@code{MODE_UACCUM} or @code{MODE_POINTER_BOUNDS}.
+
+@findex complex_mode
+@item complex_mode
+A mode that has class @code{MODE_COMPLEX_INT} or @code{MODE_COMPLEX_FLOAT}.
+@end table
+
+Named modes use the most constrained of the available wrapper classes,
+if one exists, otherwise they use @code{machine_mode}.  For example,
+@code{QImode} is a @code{scalar_int_mode}, @code{SFmode} is a
+@code{scalar_float_mode} and @code{BLKmode} is a plain
+@code{machine_mode}.  It is possible to refer to any mode as a raw
+@code{machine_mode} by adding the @code{E_} prefix, where @code{E}
+stands for ``enumeration''.  For example, the raw @code{machine_mode}
+names of the modes just mentioned are @code{E_QImode}, @code{E_SFmode}
+and @code{E_BLKmode} respectively.
+
+The wrapper classes implicitly convert to @code{machine_mode} and to any
+wrapper class that represents a more general condition; for example
+@code{scalar_int_mode} and @code{scalar_float_mode} both convert
+to @code{scalar_mode}.  The classes act like @code{machine_mode}s
+that accept only certain named modes.
+
+@findex opt_mode
+@file{machmode.h} also defines a template class @code{opt_mode<@var{T}>}
+that holds a @code{T} or nothing, where @code{T} can be either
+@code{machine_mode} or one of the wrapper classes above.  The main
+operations on an @code{opt_mode<@var{T}>} @var{x} are as follows:
+
+@table @samp
+@item @var{x}.exists ()
+Return true if @var{x} holds a mode rather than nothing.
+
+@item @var{x}.exists (&@var{y})
+Return true if @var{x} holds a mode rather than nothing, storing the
+mode in @var{y} if so.  @var{y} must be assignment-compatible with @var{T}.
+
+@item *@var{x}
+Assert that @var{x} holds a mode rather than nothing and return that mode.
+
+@item @var{x} = @var{y}
+Set @var{x} to @var{y}, where @var{y} is a @var{T} or implicitly converts
+to a @var{T}.
+@end table
+
+The default constructor sets an @code{opt_mode<@var{T}>} to nothing.
+There is also a constructor that takes an initial value of type @var{T}.
+
+It is possible to use the @file{is-a.h} accessors on a @code{machine_mode}
+or machine mode wrapper @var{x}:
+
+@table @samp
+@findex is_a
+@item is_a <@var{T}> (@var{x})
+Return true if @var{x} meets the conditions for wrapper class @var{T}.
+
+@item is_a <@var{T}> (@var{x}, &@var{y})
+Return true if @var{x} meets the conditions for wrapper class @var{T},
+storing it in @var{y} if so.  @var{y} must be assignment-compatible with
+@var{T}.
+
+@item as_a <@var{T}> (@var{x})
+Assert that @var{x} meets the conditions for wrapper class @var{T}
+and return it as a @var{T}.
+
+@item dyn_cast <@var{T}> (@var{x})
+Return an @code{opt_mode<@var{T}>} that holds @var{x} if @var{x} meets
+the conditions for wrapper class @var{T} and that holds nothing otherwise.
+@end table
+
+The purpose of these wrapper classes is to give stronger static type
+checking.  For example, if a function takes a @code{scalar_int_mode},
+a caller that has a general @code{machine_mode} must either check or
+assert that the code is indeed a scalar integer first, using one of
+the functions above.
+
+The wrapper classes are normal C++ classes, with user-defined
+constructors.  Sometimes it is useful to have a POD version of
+the same type, particularly if the type appears in a @code{union}.
+The template class @code{pod_mode<@var{T}>} provides a POD version
+of wrapper class @var{T}.  It is assignment-compatible with @var{T}
+and implicitly converts to both @code{machine_mode} and @var{T}.
+
 Here are some C macros that relate to machine modes:
 
 @table @code
Segher Boessenkool July 24, 2017, 10:56 a.m. UTC | #5 
On Mon, Jul 24, 2017 at 10:28:06AM +0100, Richard Sandiford wrote:
> > So what does it change in the interfaces we use?  I couldn't find an

> > update of documentation, maybe I missed it (it's a huge series :-) )

> > An overview of the new interfaces (and how they are used) would help.

> 

> You didn't miss one.  I was hoping the function comments would be enough,

> but on reflection, they're not.  I've attached a patch for rtl.texi below.


Thanks!

> > From what I can tell so far it makes things much harder to read.

> > Perhaps that is just because this is all new.

> 

> Which parts specifically?  E.g. is it mostly the is_a <T> (x, &y) changes?

> Or the as_a <T> (x) changes too?  Do you think the FOR_EACH_* iterators

> also make things harder to read?  Or is machine_mode->scalar_int_mode

> itself a problem?


All the  as_a <T> (x)  etc. looks like cuneiform to me (not just in your
patch); and I cannot read cuneiform :-)

One day I might understand why we need all this C++ inverted syntax,
needless abstraction, needless generalisation, data hiding and everything
else hiding.  Until then, I rant.  Sorry.

The main purpose of abstraction is to make code easier to understand and
to write and change, but with C++ it usually makes it harder it seems :-(


Segher
Richard Sandiford July 24, 2017, 12:52 p.m. UTC | #6 
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Mon, Jul 24, 2017 at 10:28:06AM +0100, Richard Sandiford wrote:

>> > From what I can tell so far it makes things much harder to read.

>> > Perhaps that is just because this is all new.

>> 

>> Which parts specifically?  E.g. is it mostly the is_a <T> (x, &y) changes?

>> Or the as_a <T> (x) changes too?  Do you think the FOR_EACH_* iterators

>> also make things harder to read?  Or is machine_mode->scalar_int_mode

>> itself a problem?

>

> All the  as_a <T> (x)  etc. looks like cuneiform to me (not just in your

> patch); and I cannot read cuneiform :-)

>

> One day I might understand why we need all this C++ inverted syntax,

> needless abstraction, needless generalisation, data hiding and everything

> else hiding.  Until then, I rant.  Sorry.

>

> The main purpose of abstraction is to make code easier to understand and

> to write and change, but with C++ it usually makes it harder it seems :-(


Well, as someone who was/is on the fence about the C++ thing, I definitely
sympathise :-)  But in this particular case it really isn't (supposed to be)
"hey, we're using C++ now, let's add more layers!" abstraction.
The same principle would have worked in C and IMO been useful in C.
It sounds like you don't necessarily object to SCALAR_INT_TYPE_MODE etc.
as asserting forms of TYPE_MODE, so if the syntax is a problem, I think:

   as_a <scalar_int_mode> (x)   => force_scalar_int (x)
   is_a <scalar_int> (x, &y)    => is_scalar_int (x, &y)

would be fine too, and shorter to write.  Would that be better?

Like I say, one advantage of the new wrappers is that they force mode
assumptions to be explicit (otherwise the compiler won't build).  That
caught several real bugs before they had been found and fixed upstream.
But that argument might be too weak to support this on its own.

The other -- original, and IMO more compelling -- motivation is to make
it easier to add variable-sized modes without having to worry about the
possibility that scalar or complex modes could be variable-sized
(because that would be much more invasive).  We could instead have kept
everything as machine_mode, made GET_MODE_SIZE always be variable, and
forced the result to a constant whenever it's "known" to be constant
for some reason.  The difficulty with that is that it would be very hard
to get right if not testing SVE, and even with SVE you would need just
the right input code to trigger the problem.  So what we wanted to do
was make it "easy" for people to know when variable-sized modes were a
concern even if they weren't thinking about or testing SVE.  This
scalar/not scalar or complex/not complex choices aren't architecture-
specific in the same way.

With this approach we only needed to force a variable size or offset to
a constant in a few places, and those cases were easy to justify from
context.

Another alternative would have been to add functions like
GET_SCALAR_MODE_SIZE that first assert that the mode is scalar and then
return a constant size.  However, that would have led to a lot more
asserts in an enable-checking build and IMO wouldn't have been any
more readable than an explicit "is this a scalar?" check followed
by the normal GET_MODE_SIZE accessors.

Thanks,
Richard
Segher Boessenkool July 24, 2017, 1:42 p.m. UTC | #7 
On Mon, Jul 24, 2017 at 01:52:49PM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:

> > On Mon, Jul 24, 2017 at 10:28:06AM +0100, Richard Sandiford wrote:

> >> > From what I can tell so far it makes things much harder to read.

> >> > Perhaps that is just because this is all new.

> >> 

> >> Which parts specifically?  E.g. is it mostly the is_a <T> (x, &y) changes?

> >> Or the as_a <T> (x) changes too?  Do you think the FOR_EACH_* iterators

> >> also make things harder to read?  Or is machine_mode->scalar_int_mode

> >> itself a problem?

> >

> > All the  as_a <T> (x)  etc. looks like cuneiform to me (not just in your

> > patch); and I cannot read cuneiform :-)

> >

> > One day I might understand why we need all this C++ inverted syntax,

> > needless abstraction, needless generalisation, data hiding and everything

> > else hiding.  Until then, I rant.  Sorry.

> >

> > The main purpose of abstraction is to make code easier to understand and

> > to write and change, but with C++ it usually makes it harder it seems :-(

> 

> Well, as someone who was/is on the fence about the C++ thing, I definitely

> sympathise :-)  But in this particular case it really isn't (supposed to be)

> "hey, we're using C++ now, let's add more layers!" abstraction.


:-)

> The same principle would have worked in C and IMO been useful in C.

> It sounds like you don't necessarily object to SCALAR_INT_TYPE_MODE etc.

> as asserting forms of TYPE_MODE, so if the syntax is a problem, I think:

> 

>    as_a <scalar_int_mode> (x)   => force_scalar_int (x)

>    is_a <scalar_int> (x, &y)    => is_scalar_int (x, &y)

> 

> would be fine too, and shorter to write.  Would that be better?


Yeah that looks better, to me at least.

> Like I say, one advantage of the new wrappers is that they force mode

> assumptions to be explicit (otherwise the compiler won't build).  That

> caught several real bugs before they had been found and fixed upstream.

> But that argument might be too weak to support this on its own.


It also helps compile time you said.  I like that, too :-)

> The other -- original, and IMO more compelling -- motivation is to make

> it easier to add variable-sized modes without having to worry about the

> possibility that scalar or complex modes could be variable-sized

> (because that would be much more invasive).  We could instead have kept

> everything as machine_mode, made GET_MODE_SIZE always be variable, and

> forced the result to a constant whenever it's "known" to be constant

> for some reason.  The difficulty with that is that it would be very hard

> to get right if not testing SVE, and even with SVE you would need just

> the right input code to trigger the problem.  So what we wanted to do

> was make it "easy" for people to know when variable-sized modes were a

> concern even if they weren't thinking about or testing SVE.  This

> scalar/not scalar or complex/not complex choices aren't architecture-

> specific in the same way.

> 

> With this approach we only needed to force a variable size or offset to

> a constant in a few places, and those cases were easy to justify from

> context.


Yes, it is clear some changes are needed -- I just hope the changes
can make the code easier to understand, instead of more complex.


Segher
Richard Biener July 24, 2017, 6:31 p.m. UTC | #8 
On July 24, 2017 3:42:30 PM GMT+02:00, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>On Mon, Jul 24, 2017 at 01:52:49PM +0100, Richard Sandiford wrote:

>> Segher Boessenkool <segher@kernel.crashing.org> writes:

>> > On Mon, Jul 24, 2017 at 10:28:06AM +0100, Richard Sandiford wrote:

>> >> > From what I can tell so far it makes things much harder to read.

>> >> > Perhaps that is just because this is all new.

>> >> 

>> >> Which parts specifically?  E.g. is it mostly the is_a <T> (x, &y)

>changes?

>> >> Or the as_a <T> (x) changes too?  Do you think the FOR_EACH_*

>iterators

>> >> also make things harder to read?  Or is

>machine_mode->scalar_int_mode

>> >> itself a problem?

>> >

>> > All the  as_a <T> (x)  etc. looks like cuneiform to me (not just in

>your

>> > patch); and I cannot read cuneiform :-)

>> >

>> > One day I might understand why we need all this C++ inverted

>syntax,

>> > needless abstraction, needless generalisation, data hiding and

>everything

>> > else hiding.  Until then, I rant.  Sorry.

>> >

>> > The main purpose of abstraction is to make code easier to

>understand and

>> > to write and change, but with C++ it usually makes it harder it

>seems :-(

>> 

>> Well, as someone who was/is on the fence about the C++ thing, I

>definitely

>> sympathise :-)  But in this particular case it really isn't (supposed

>to be)

>> "hey, we're using C++ now, let's add more layers!" abstraction.

>

>:-)

>

>> The same principle would have worked in C and IMO been useful in C.

>> It sounds like you don't necessarily object to SCALAR_INT_TYPE_MODE

>etc.

>> as asserting forms of TYPE_MODE, so if the syntax is a problem, I

>think:

>> 

>>    as_a <scalar_int_mode> (x)   => force_scalar_int (x)

>>    is_a <scalar_int> (x, &y)    => is_scalar_int (x, &y)

>> 

>> would be fine too, and shorter to write.  Would that be better?

>

>Yeah that looks better, to me at least.


I don't like that.  We've settled on one style so please adhere to that.  Force_ also suggests some magic semantics that are not there.

So if any then try to improve the as-is stuff in general.  For example it's quite awkward in switches.

Richard.

>> Like I say, one advantage of the new wrappers is that they force mode

>> assumptions to be explicit (otherwise the compiler won't build). 

>That

>> caught several real bugs before they had been found and fixed

>upstream.

>> But that argument might be too weak to support this on its own.

>

>It also helps compile time you said.  I like that, too :-)

>

>> The other -- original, and IMO more compelling -- motivation is to

>make

>> it easier to add variable-sized modes without having to worry about

>the

>> possibility that scalar or complex modes could be variable-sized

>> (because that would be much more invasive).  We could instead have

>kept

>> everything as machine_mode, made GET_MODE_SIZE always be variable,

>and

>> forced the result to a constant whenever it's "known" to be constant

>> for some reason.  The difficulty with that is that it would be very

>hard

>> to get right if not testing SVE, and even with SVE you would need

>just

>> the right input code to trigger the problem.  So what we wanted to do

>> was make it "easy" for people to know when variable-sized modes were

>a

>> concern even if they weren't thinking about or testing SVE.  This

>> scalar/not scalar or complex/not complex choices aren't architecture-

>> specific in the same way.

>> 

>> With this approach we only needed to force a variable size or offset

>to

>> a constant in a few places, and those cases were easy to justify from

>> context.

>

>Yes, it is clear some changes are needed -- I just hope the changes

>can make the code easier to understand, instead of more complex.

>

>

>Segher
Jeff Law Aug. 11, 2017, 4:46 p.m. UTC | #9 
On 07/13/2017 02:38 AM, Richard Sandiford wrote:
> All case statements need to be updated to use the prefixed names,

> since the unprefixed names will eventually not be integer constant

> expressions.  This patch does a mechanical substitution over the whole

> codebase.

> 

> 2017-07-13  Richard Sandiford  <richard.sandiford@linaro.org>

> 	    Alan Hayward  <alan.hayward@arm.com>

> 	    David Sherwood  <david.sherwood@arm.com>

> 

> gcc/

> 	* config/aarch64/aarch64-builtins.c (aarch64_simd_builtin_std_type):

> 	Prefix mode names with E_ in case statements.

> 	* config/aarch64/aarch64-elf.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.

> 	* config/aarch64/aarch64.c (aarch64_split_simd_combine): Likewise.

> 	(aarch64_split_simd_move): Likewise.

> 	(aarch64_gen_storewb_pair): Likewise.

> 	(aarch64_gen_loadwb_pair): Likewise.

> 	(aarch64_gen_store_pair): Likewise.

> 	(aarch64_gen_load_pair): Likewise.

> 	(aarch64_get_condition_code_1): Likewise.

> 	(aarch64_constant_pool_reload_icode): Likewise.

> 	(get_rsqrte_type): Likewise.

> 	(get_rsqrts_type): Likewise.

> 	(get_recpe_type): Likewise.

> 	(get_recps_type): Likewise.

> 	(aarch64_gimplify_va_arg_expr): Likewise.

> 	(aarch64_simd_container_mode): Likewise.

> 	(aarch64_emit_load_exclusive): Likewise.

> 	(aarch64_emit_store_exclusive): Likewise.

> 	(aarch64_expand_compare_and_swap): Likewise.

> 	(aarch64_gen_atomic_cas): Likewise.

> 	(aarch64_emit_bic): Likewise.

> 	(aarch64_emit_atomic_swap): Likewise.

> 	(aarch64_emit_atomic_load_op): Likewise.

> 	(aarch64_evpc_trn): Likewise.

> 	(aarch64_evpc_uzp): Likewise.

> 	(aarch64_evpc_zip): Likewise.

> 	(aarch64_evpc_ext): Likewise.

> 	(aarch64_evpc_rev): Likewise.

> 	(aarch64_evpc_dup): Likewise.

> 	(aarch64_gen_ccmp_first): Likewise.

> 	(aarch64_gen_ccmp_next): Likewise.

> 	* config/alpha/alpha.c (alpha_scalar_mode_supported_p): Likewise.

> 	(alpha_emit_xfloating_libcall): Likewise.

> 	(emit_insxl): Likewise.

> 	(alpha_arg_type): Likewise.

> 	* config/arc/arc.c (arc_vector_mode_supported_p): Likewise.

> 	(arc_preferred_simd_mode): Likewise.

> 	(arc_secondary_reload): Likewise.

> 	(get_arc_condition_code): Likewise.

> 	(arc_print_operand): Likewise.

> 	* config/arc/arc.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.

> 	* config/arc/arc.md (casesi_load): Likewise.

> 	(casesi_compact_jump): Likewise.

> 	* config/arc/predicates.md (proper_comparison_operator): Likewise.

> 	(cc_use_register): Likewise.

> 	* config/arm/aout.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.

> 	* config/arm/arm-builtins.c (arm_simd_builtin_std_type): Likewise.

> 	(arm_init_iwmmxt_builtins): Likewise.

> 	* config/arm/arm.c (thumb1_size_rtx_costs): Likewise.

> 	(neon_expand_vector_init): Likewise.

> 	(arm_attr_length_move_neon): Likewise.

> 	(maybe_get_arm_condition_code): Likewise.

> 	(arm_emit_vector_const): Likewise.

> 	(arm_preferred_simd_mode): Likewise.

> 	(arm_output_iwmmxt_tinsr): Likewise.

> 	(thumb1_output_casesi): Likewise.

> 	(thumb2_output_casesi): Likewise.

> 	(arm_emit_load_exclusive): Likewise.

> 	(arm_emit_store_exclusive): Likewise.

> 	(arm_expand_compare_and_swap): Likewise.

> 	(arm_evpc_neon_vuzp): Likewise.

> 	(arm_evpc_neon_vzip): Likewise.

> 	(arm_evpc_neon_vrev): Likewise.

> 	(arm_evpc_neon_vtrn): Likewise.

> 	(arm_evpc_neon_vext): Likewise.

> 	(arm_validize_comparison): Likewise.

> 	* config/arm/neon.md (neon_vc<cmp_op><mode>): Likewise.

> 	* config/avr/avr-c.c (avr_resolve_overloaded_builtin): Likewise.

> 	* config/avr/avr.c (avr_rtx_costs_1): Likewise.

> 	* config/c6x/c6x.c (c6x_vector_mode_supported_p): Likewise.

> 	(c6x_preferred_simd_mode): Likewise.

> 	* config/epiphany/epiphany.c (get_epiphany_condition_code): Likewise.

> 	(epiphany_rtx_costs): Likewise.

> 	* config/epiphany/predicates.md (proper_comparison_operator):

> 	Likewise.

> 	* config/frv/frv.c (condexec_memory_operand): Likewise.

> 	(frv_emit_move): Likewise.

> 	(output_move_single): Likewise.

> 	(output_condmove_single): Likewise.

> 	(frv_hard_regno_mode_ok): Likewise.

> 	(frv_matching_accg_mode): Likewise.

> 	* config/h8300/h8300.c (split_adds_subs): Likewise.

> 	(h8300_rtx_costs): Likewise.

> 	(h8300_print_operand): Likewise.

> 	(compute_mov_length): Likewise.

> 	(output_logical_op): Likewise.

> 	(compute_logical_op_length): Likewise.

> 	(compute_logical_op_cc): Likewise.

> 	(h8300_shift_needs_scratch_p): Likewise.

> 	(output_a_shift): Likewise.

> 	(compute_a_shift_length): Likewise.

> 	(compute_a_shift_cc): Likewise.

> 	(expand_a_rotate): Likewise.

> 	(output_a_rotate): Likewise.

> 	* config/i386/i386.c (classify_argument): Likewise.

> 	(function_arg_advance_32): Likewise.

> 	(function_arg_32): Likewise.

> 	(function_arg_64): Likewise.

> 	(function_value_64): Likewise.

> 	(ix86_gimplify_va_arg): Likewise.

> 	(ix86_legitimate_constant_p): Likewise.

> 	(put_condition_code): Likewise.

> 	(split_double_mode): Likewise.

> 	(ix86_avx256_split_vector_move_misalign): Likewise.

> 	(ix86_expand_vector_logical_operator): Likewise.

> 	(ix86_split_idivmod): Likewise.

> 	(ix86_expand_adjust_ufix_to_sfix_si): Likewise.

> 	(ix86_build_const_vector): Likewise.

> 	(ix86_build_signbit_mask): Likewise.

> 	(ix86_match_ccmode): Likewise.

> 	(ix86_cc_modes_compatible): Likewise.

> 	(ix86_expand_branch): Likewise.

> 	(ix86_expand_sse_cmp): Likewise.

> 	(ix86_expand_sse_movcc): Likewise.

> 	(ix86_expand_int_sse_cmp): Likewise.

> 	(ix86_expand_vec_perm_vpermi2): Likewise.

> 	(ix86_expand_vec_perm): Likewise.

> 	(ix86_expand_sse_unpack): Likewise.

> 	(ix86_expand_int_addcc): Likewise.

> 	(ix86_split_to_parts): Likewise.

> 	(ix86_vectorize_builtin_gather): Likewise.

> 	(ix86_vectorize_builtin_scatter): Likewise.

> 	(avx_vpermilp_parallel): Likewise.

> 	(inline_memory_move_cost): Likewise.

> 	(ix86_tieable_integer_mode_p): Likewise.

> 	(x86_maybe_negate_const_int): Likewise.

> 	(ix86_expand_vector_init_duplicate): Likewise.

> 	(ix86_expand_vector_init_one_nonzero): Likewise.

> 	(ix86_expand_vector_init_one_var): Likewise.

> 	(ix86_expand_vector_init_concat): Likewise.

> 	(ix86_expand_vector_init_interleave): Likewise.

> 	(ix86_expand_vector_init_general): Likewise.

> 	(ix86_expand_vector_set): Likewise.

> 	(ix86_expand_vector_extract): Likewise.

> 	(emit_reduc_half): Likewise.

> 	(ix86_emit_i387_round): Likewise.

> 	(ix86_mangle_type): Likewise.

> 	(ix86_expand_round_sse4): Likewise.

> 	(expand_vec_perm_blend): Likewise.

> 	(canonicalize_vector_int_perm): Likewise.

> 	(ix86_expand_vec_one_operand_perm_avx512): Likewise.

> 	(expand_vec_perm_1): Likewise.

> 	(expand_vec_perm_interleave3): Likewise.

> 	(expand_vec_perm_even_odd_pack): Likewise.

> 	(expand_vec_perm_even_odd_1): Likewise.

> 	(expand_vec_perm_broadcast_1): Likewise.

> 	(ix86_vectorize_vec_perm_const_ok): Likewise.

> 	(ix86_expand_vecop_qihi): Likewise.

> 	(ix86_expand_mul_widen_hilo): Likewise.

> 	(ix86_expand_sse2_abs): Likewise.

> 	(ix86_expand_pextr): Likewise.

> 	(ix86_expand_pinsr): Likewise.

> 	(ix86_preferred_simd_mode): Likewise.

> 	(ix86_simd_clone_compute_vecsize_and_simdlen): Likewise.

> 	* config/i386/sse.md (*andnot<mode>3): Likewise.

> 	(<mask_codefor><code><mode>3<mask_name>): Likewise.

> 	(*<code><mode>3): Likewise.

> 	* config/ia64/ia64.c (ia64_expand_vecint_compare): Likewise.

> 	(ia64_expand_atomic_op): Likewise.

> 	(ia64_arg_type): Likewise.

> 	(ia64_mode_to_int): Likewise.

> 	(ia64_scalar_mode_supported_p): Likewise.

> 	(ia64_vector_mode_supported_p): Likewise.

> 	(expand_vec_perm_broadcast): Likewise.

> 	* config/iq2000/iq2000.c (iq2000_move_1word): Likewise.

> 	(iq2000_function_arg_advance): Likewise.

> 	(iq2000_function_arg): Likewise.

> 	* config/m32c/m32c.c (m32c_preferred_reload_class): Likewise.

> 	* config/m68k/m68k.c (output_dbcc_and_branch): Likewise.

> 	(m68k_libcall_value): Likewise.

> 	(m68k_function_value): Likewise.

> 	(sched_attr_op_type): Likewise.

> 	* config/mcore/mcore.c (mcore_output_move): Likewise.

> 	* config/microblaze/microblaze.c (microblaze_function_arg_advance):

> 	Likewise.

> 	(microblaze_function_arg): Likewise.

> 	* config/mips/mips.c (mips16_build_call_stub): Likewise.

> 	(mips_print_operand): Likewise.

> 	(mips_mode_ok_for_mov_fmt_p): Likewise.

> 	(mips_vector_mode_supported_p): Likewise.

> 	(mips_preferred_simd_mode): Likewise.

> 	(mips_expand_vpc_loongson_even_odd): Likewise.

> 	(mips_expand_vec_unpack): Likewise.

> 	(mips_expand_vi_broadcast): Likewise.

> 	(mips_expand_vector_init): Likewise.

> 	(mips_expand_vec_reduc): Likewise.

> 	(mips_expand_msa_cmp): Likewise.

> 	* config/mips/mips.md (casesi_internal_mips16_<mode>): Likewise.

> 	* config/mn10300/mn10300.c (mn10300_print_operand): Likewise.

> 	(cc_flags_for_mode): Likewise.

> 	* config/msp430/msp430.c (msp430_print_operand): Likewise.

> 	* config/nds32/nds32-md-auxiliary.c (nds32_mem_format): Likewise.

> 	(nds32_output_casesi_pc_relative): Likewise.

> 	* config/nds32/nds32.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.

> 	* config/nvptx/nvptx.c (nvptx_ptx_type_from_mode): Likewise.

> 	(nvptx_gen_unpack): Likewise.

> 	(nvptx_gen_pack): Likewise.

> 	(nvptx_gen_shuffle): Likewise.

> 	(nvptx_gen_wcast): Likewise.

> 	* config/pa/pa.c (pa_secondary_reload): Likewise.

> 	* config/pa/predicates.md (base14_operand): Likewise.

> 	* config/powerpcspe/powerpcspe-c.c

> 	(altivec_resolve_overloaded_builtin): Likewise.

> 	* config/powerpcspe/powerpcspe.c (rs6000_setup_reg_addr_masks):

> 	Likewise.

> 	(rs6000_preferred_simd_mode): Likewise.

> 	(output_vec_const_move): Likewise.

> 	(rs6000_expand_vector_extract): Likewise.

> 	(rs6000_split_vec_extract_var): Likewise.

> 	(reg_offset_addressing_ok_p): Likewise.

> 	(rs6000_legitimate_offset_address_p): Likewise.

> 	(rs6000_legitimize_address): Likewise.

> 	(rs6000_emit_set_const): Likewise.

> 	(rs6000_const_vec): Likewise.

> 	(rs6000_emit_move): Likewise.

> 	(spe_build_register_parallel): Likewise.

> 	(rs6000_darwin64_record_arg_recurse): Likewise.

> 	(swap_selector_for_mode): Likewise.

> 	(spe_init_builtins): Likewise.

> 	(paired_init_builtins): Likewise.

> 	(altivec_init_builtins): Likewise.

> 	(do_load_for_compare): Likewise.

> 	(rs6000_generate_compare): Likewise.

> 	(rs6000_expand_float128_convert): Likewise.

> 	(emit_load_locked): Likewise.

> 	(emit_store_conditional): Likewise.

> 	(rs6000_output_function_epilogue): Likewise.

> 	(rs6000_handle_altivec_attribute): Likewise.

> 	(rs6000_function_value): Likewise.

> 	(emit_fusion_gpr_load): Likewise.

> 	(emit_fusion_p9_load): Likewise.

> 	(emit_fusion_p9_store): Likewise.

> 	* config/powerpcspe/predicates.md (easy_fp_constant): Likewise.

> 	(fusion_gpr_mem_load): Likewise.

> 	(fusion_addis_mem_combo_load): Likewise.

> 	(fusion_addis_mem_combo_store): Likewise.

> 	* config/rs6000/predicates.md (easy_fp_constant): Likewise.

> 	(fusion_gpr_mem_load): Likewise.

> 	(fusion_addis_mem_combo_load): Likewise.

> 	(fusion_addis_mem_combo_store): Likewise.

> 	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):

> 	Likewise.

> 	* config/rs6000/rs6000-string.c (do_load_for_compare): Likewise.

> 	* config/rs6000/rs6000.c (rs6000_setup_reg_addr_masks): Likewise.

> 	(rs6000_preferred_simd_mode): Likewise.

> 	(output_vec_const_move): Likewise.

> 	(rs6000_expand_vector_extract): Likewise.

> 	(rs6000_split_vec_extract_var): Likewise.

> 	(reg_offset_addressing_ok_p): Likewise.

> 	(rs6000_legitimate_offset_address_p): Likewise.

> 	(rs6000_legitimize_address): Likewise.

> 	(rs6000_emit_set_const): Likewise.

> 	(rs6000_const_vec): Likewise.

> 	(rs6000_emit_move): Likewise.

> 	(rs6000_darwin64_record_arg_recurse): Likewise.

> 	(swap_selector_for_mode): Likewise.

> 	(paired_init_builtins): Likewise.

> 	(altivec_init_builtins): Likewise.

> 	(rs6000_expand_float128_convert): Likewise.

> 	(emit_load_locked): Likewise.

> 	(emit_store_conditional): Likewise.

> 	(rs6000_output_function_epilogue): Likewise.

> 	(rs6000_handle_altivec_attribute): Likewise.

> 	(rs6000_function_value): Likewise.

> 	(emit_fusion_gpr_load): Likewise.

> 	(emit_fusion_p9_load): Likewise.

> 	(emit_fusion_p9_store): Likewise.

> 	* config/rx/rx.c (rx_gen_move_template): Likewise.

> 	(flags_from_mode): Likewise.

> 	* config/s390/predicates.md (s390_alc_comparison): Likewise.

> 	(s390_slb_comparison): Likewise.

> 	* config/s390/s390.c (s390_handle_vectorbool_attribute): Likewise.

> 	(s390_vector_mode_supported_p): Likewise.

> 	(s390_cc_modes_compatible): Likewise.

> 	(s390_match_ccmode_set): Likewise.

> 	(s390_canonicalize_comparison): Likewise.

> 	(s390_emit_compare_and_swap): Likewise.

> 	(s390_branch_condition_mask): Likewise.

> 	(s390_rtx_costs): Likewise.

> 	(s390_secondary_reload): Likewise.

> 	(__SECONDARY_RELOAD_CASE): Likewise.

> 	(s390_expand_cs): Likewise.

> 	(s390_preferred_simd_mode): Likewise.

> 	* config/s390/vx-builtins.md (vec_packsu_u<mode>): Likewise.

> 	* config/sh/sh.c (sh_print_operand): Likewise.

> 	(dump_table): Likewise.

> 	(sh_secondary_reload): Likewise.

> 	* config/sh/sh.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.

> 	* config/sh/sh.md (casesi_worker_1): Likewise.

> 	(casesi_worker_2): Likewise.

> 	* config/sparc/predicates.md (icc_comparison_operator): Likewise.

> 	(fcc_comparison_operator): Likewise.

> 	* config/sparc/sparc.c (sparc_expand_move): Likewise.

> 	(emit_soft_tfmode_cvt): Likewise.

> 	(sparc_preferred_simd_mode): Likewise.

> 	(output_cbranch): Likewise.

> 	(sparc_print_operand): Likewise.

> 	(sparc_expand_vec_perm_bmask): Likewise.

> 	(vector_init_bshuffle): Likewise.

> 	* config/spu/spu.c (spu_scalar_mode_supported_p): Likewise.

> 	(spu_vector_mode_supported_p): Likewise.

> 	(spu_expand_insv): Likewise.

> 	(spu_emit_branch_or_set): Likewise.

> 	(spu_handle_vector_attribute): Likewise.

> 	(spu_builtin_splats): Likewise.

> 	(spu_builtin_extract): Likewise.

> 	(spu_builtin_promote): Likewise.

> 	(spu_expand_sign_extend): Likewise.

> 	* config/tilegx/tilegx.c (tilegx_scalar_mode_supported_p): Likewise.

> 	(tilegx_simd_int): Likewise.

> 	* config/tilepro/tilepro.c (tilepro_scalar_mode_supported_p): Likewise.

> 	(tilepro_simd_int): Likewise.

> 	* config/v850/v850.c (const_double_split): Likewise.

> 	(v850_print_operand): Likewise.

> 	(ep_memory_offset): Likewise.

> 	* config/vax/vax.c (vax_rtx_costs): Likewise.

> 	(vax_output_int_move): Likewise.

> 	(vax_output_int_add): Likewise.

> 	(vax_output_int_subtract): Likewise.

> 	* config/visium/predicates.md (visium_branch_operator): Likewise.

> 	* config/visium/visium.c (rtx_ok_for_offset_p): Likewise.

> 	(visium_print_operand_address): Likewise.

> 	* config/visium/visium.h (ASM_OUTPUT_ADDR_DIFF_ELT): Likewise.

> 	* config/xtensa/xtensa.c (xtensa_mem_offset): Likewise.

> 	(xtensa_expand_conditional_branch): Likewise.

> 	(xtensa_copy_incoming_a7): Likewise.

> 	(xtensa_output_literal): Likewise.

> 	* dfp.c (decimal_real_maxval): Likewise.

> 	* targhooks.c (default_libgcc_floating_mode_supported_p): Likewise.

> 

> gcc/c-family/

> 	* c-cppbuiltin.c (mode_has_fma): Prefix mode names with E_ in

> 	case statements.

> 

> gcc/objc/

> 	* objc-encoding.c (encode_gnu_bitfield): Prefix mode names with E_ in

> 	case statements.

> 

> libobjc/

> 	* encoding.c (_darwin_rs6000_special_round_type_align): Prefix mode

> 	names with E_ in case statements.

> 

OK.
jeff
Jeff Law Aug. 24, 2017, 9:37 p.m. UTC | #10 
On 07/13/2017 03:00 AM, Richard Sandiford wrote:
> This patch changes the types of various things from machine_mode

> to scalar_int_mode, in cases where (after previous patches)

> simply changing the type is enough on its own.  The patch does

> nothing other than that.

> 

> 2017-07-13  Richard Sandiford  <richard.sandiford@linaro.org>

> 	    Alan Hayward  <alan.hayward@arm.com>

> 	    David Sherwood  <david.sherwood@arm.com>

> 

> gcc/

> 	* builtins.h (builtin_strncpy_read_str): Take a scalar_int_mode

> 	instead of a machine_mode.

> 	(builtin_memset_read_str): Likewise.

> 	* builtins.c (c_readstr): Likewise.

> 	(builtin_memcpy_read_str): Likewise.

> 	(builtin_strncpy_read_str): Likewise.

> 	(builtin_memset_read_str): Likewise.

> 	(builtin_memset_gen_str): Likewise.

> 	(expand_builtin_signbit): Use scalar_int_mode for local variables.

> 	* cfgexpand.c (convert_debug_memory_address): Take a scalar_int_mode

> 	instead of a machine_mode.

> 	* combine.c (simplify_if_then_else): Use scalar_int_mode for local

> 	variables.

> 	(make_extraction): Likewise.

> 	(try_widen_shift_mode): Take and return scalar_int_modes instead

> 	of machine_modes.

> 	* config/aarch64/aarch64.c (aarch64_libgcc_cmp_return_mode): Return

> 	a scalar_int_mode instead of a machine_mode.

> 	* config/avr/avr.c (avr_addr_space_address_mode): Likewise.

> 	(avr_addr_space_pointer_mode): Likewise.

> 	* config/cr16/cr16.c (cr16_unwind_word_mode): Likewise.

> 	* config/msp430/msp430.c (msp430_addr_space_pointer_mode): Likewise.

> 	(msp430_unwind_word_mode): Likewise.

> 	* config/spu/spu.c (spu_unwind_word_mode): Likewise.

> 	(spu_addr_space_pointer_mode): Likewise.

> 	(spu_addr_space_address_mode): Likewise.

> 	(spu_libgcc_cmp_return_mode): Likewise.

> 	(spu_libgcc_shift_count_mode): Likewise.

> 	* config/rl78/rl78.c (rl78_addr_space_address_mode): Likewise.

> 	(rl78_addr_space_pointer_mode): Likewise.

> 	(fl78_unwind_word_mode): Likewise.

> 	(rl78_valid_pointer_mode): Take a scalar_int_mode instead of a

> 	machine_mode.

> 	* config/alpha/alpha.c (vms_valid_pointer_mode): Likewise.

> 	* config/ia64/ia64.c (ia64_vms_valid_pointer_mode): Likewise.

> 	* config/mips/mips.c (mips_mode_rep_extended): Likewise.

> 	(mips_valid_pointer_mode): Likewise.

> 	* config/tilegx/tilegx.c (tilegx_mode_rep_extended): Likewise.

> 	* config/ft32/ft32.c (ft32_valid_pointer_mode): Likewise.

> 	(ft32_addr_space_pointer_mode): Return a scalar_int_mode instead

> 	of a machine_mode.

> 	(ft32_addr_space_address_mode): Likewise.

> 	* config/m32c/m32c.c (m32c_valid_pointer_mode): Take a

> 	scalar_int_mode instead of a machine_mode.

> 	(m32c_addr_space_pointer_mode): Return a scalar_int_mode instead

> 	of a machine_mode.

> 	(m32c_addr_space_address_mode): Likewise.

> 	* config/powerpcspe/powerpcspe.c (rs6000_abi_word_mode): Likewise.

> 	(rs6000_eh_return_filter_mode): Likewise.

> 	* config/rs6000/rs6000.c (rs6000_abi_word_mode): Likewise.

> 	(rs6000_eh_return_filter_mode): Likewise.

> 	* config/s390/s390.c (s390_libgcc_cmp_return_mode): Likewise.

> 	(s390_libgcc_shift_count_mode): Likewise.

> 	(s390_unwind_word_mode): Likewise.

> 	(s390_valid_pointer_mode): Take a scalar_int_mode rather than a

> 	machine_mode.

> 	* target.def (mode_rep_extended): Likewise.

> 	(valid_pointer_mode): Likewise.

> 	(addr_space.valid_pointer_mode): Likewise.

> 	(eh_return_filter_mode): Return a scalar_int_mode rather than

> 	a machine_mode.

> 	(libgcc_cmp_return_mode): Likewise.

> 	(libgcc_shift_count_mode): Likewise.

> 	(unwind_word_mode): Likewise.

> 	(addr_space.pointer_mode): Likewise.

> 	(addr_space.address_mode): Likewise.

> 	* doc/tm.texi: Regenerate.

> 	* dojump.c (prefer_and_bit_test): Take a scalar_int_mode rather than

> 	a machine_mode.

> 	(do_jump): Use scalar_int_mode for local variables.

> 	* dwarf2cfi.c (init_return_column_size): Take a scalar_int_mode

> 	rather than a machine_mode.

> 	* dwarf2out.c (convert_descriptor_to_mode): Likewise.

> 	(scompare_loc_descriptor_wide): Likewise.

> 	(scompare_loc_descriptor_narrow): Likewise.

> 	* emit-rtl.c (adjust_address_1): Use scalar_int_mode for local

> 	variables.

> 	* except.c (sjlj_emit_dispatch_table): Likewise.

> 	(expand_builtin_eh_copy_values): Likewise.

> 	* explow.c (convert_memory_address_addr_space_1): Likewise.

> 	Take a scalar_int_mode rather than a machine_mode.

> 	(convert_memory_address_addr_space): Take a scalar_int_mode rather

> 	than a machine_mode.

> 	(memory_address_addr_space): Use scalar_int_mode for local variables.

> 	* expmed.h (expand_mult_highpart_adjust): Take a scalar_int_mode

> 	rather than a machine_mode.

> 	* expmed.c (mask_rtx): Likewise.

> 	(init_expmed_one_conv): Likewise.

> 	(expand_mult_highpart_adjust): Likewise.

> 	(extract_high_half): Likewise.

> 	(expmed_mult_highpart_optab): Likewise.

> 	(expmed_mult_highpart): Likewise.

> 	(expand_smod_pow2): Likewise.

> 	(expand_sdiv_pow2): Likewise.

> 	(emit_store_flag_int): Likewise.

> 	(adjust_bit_field_mem_for_reg): Use scalar_int_mode for local

> 	variables.

> 	(extract_low_bits): Likewise.

> 	* expr.h (by_pieces_constfn): Take a scalar_int_mode rather than

> 	a machine_mode.

> 	* expr.c (pieces_addr::adjust):  Likewise.

> 	(can_store_by_pieces): Likewise.

> 	(store_by_pieces): Likewise.

> 	(clear_by_pieces_1): Likewise.

> 	(expand_expr_addr_expr_1): Likewise.

> 	(expand_expr_addr_expr): Use scalar_int_mode for local variables.

> 	(expand_expr_real_1): Likewise.

> 	(try_casesi): Likewise.

> 	* final.c (shorten_branches): Likewise.

> 	* fold-const.c (fold_convert_const_int_from_fixed): Change the

> 	type of "mode" to machine_mode.

> 	* internal-fn.c (expand_arith_overflow_result_store): Take a

> 	scalar_int_mode rather than a machine_mode.

> 	(expand_mul_overflow): Use scalar_int_mode for local variables.

> 	* loop-doloop.c (doloop_modify): Likewise.

> 	(doloop_optimize): Likewise.

> 	* optabs.c (expand_subword_shift): Take a scalar_int_mode rather

> 	than a machine_mode.

> 	(expand_doubleword_shift_condmove): Likewise.

> 	(expand_doubleword_shift): Likewise.

> 	(expand_doubleword_clz): Likewise.

> 	(expand_doubleword_popcount): Likewise.

> 	(expand_doubleword_parity): Likewise.

> 	(expand_absneg_bit): Use scalar_int_mode for local variables.

> 	(prepare_float_lib_cmp): Likewise.

> 	* rtl.h (convert_memory_address_addr_space_1): Take a scalar_int_mode

> 	rather than a machine_mode.

> 	(convert_memory_address_addr_space): Likewise.

> 	(get_mode_bounds): Likewise.

> 	(get_address_mode): Return a scalar_int_mode rather than a

> 	machine_mode.

> 	* rtlanal.c (get_address_mode): Likewise.

> 	* stor-layout.c (get_mode_bounds): Take a scalar_int_mode rather

> 	than a machine_mode.

> 	* targhooks.c (default_mode_rep_extended): Likewise.

> 	(default_valid_pointer_mode): Likewise.

> 	(default_addr_space_valid_pointer_mode): Likewise.

> 	(default_eh_return_filter_mode): Return a scalar_int_mode rather

> 	than a machine_mode.

> 	(default_libgcc_cmp_return_mode): Likewise.

> 	(default_libgcc_shift_count_mode): Likewise.

> 	(default_unwind_word_mode): Likewise.

> 	(default_addr_space_pointer_mode): Likewise.

> 	(default_addr_space_address_mode): Likewise.

> 	* targhooks.h (default_eh_return_filter_mode): Likewise.

> 	(default_libgcc_cmp_return_mode): Likewise.

> 	(default_libgcc_shift_count_mode): Likewise.

> 	(default_unwind_word_mode): Likewise.

> 	(default_addr_space_pointer_mode): Likewise.

> 	(default_addr_space_address_mode): Likewise.

> 	(default_mode_rep_extended): Take a scalar_int_mode rather than

> 	a machine_mode.

> 	(default_valid_pointer_mode): Likewise.

> 	(default_addr_space_valid_pointer_mode): Likewise.

> 	* tree-ssa-address.c (addr_for_mem_ref): Use scalar_int_mode for

> 	local variables.

> 	* tree-ssa-loop-ivopts.c (get_shiftadd_cost): Take a scalar_int_mode

> 	rather than a machine_mode.

> 	* tree-switch-conversion.c (array_value_type): Use scalar_int_mode

> 	for local variables.

> 	* tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.

> 	* var-tracking.c (use_narrower_mode): Take a scalar_int_mode rather

> 	than a machine_mode.

> 

Going to ACK as obvious.

Jeff