Jeff Law <l...@redhat.com> writes: > On 07/13/2017 02:40 AM, Richard Sandiford wrote: >> GET_MODE_WIDER previously returned VOIDmode if no wider mode existed. >> That would cause problems with stricter mode classes, since VOIDmode >> isn't for example a valid scalar integer or floating-point mode. >> This patch instead makes it return a new opt_mode<T> class, which >> holds either a T or nothing. >> >> 2017-07-13 Richard Sandiford <richard.sandif...@linaro.org> >> Alan Hayward <alan.hayw...@arm.com> >> David Sherwood <david.sherw...@arm.com> >> >> gcc/ >> * coretypes.h (opt_mode): New class. >> * machmode.h (opt_mode): Likewise. >> (opt_mode::else_void): New function. >> (opt_mode::operator *): Likewise. >> (opt_mode::exists): Likewise. >> (GET_MODE_WIDER_MODE): Turn into a function and return an opt_mode. >> (GET_MODE_2XWIDER_MODE): Likewise. >> (mode_iterator::get_wider): Update accordingly. >> (mode_iterator::get_2xwider): Likewise. >> (mode_iterator::get_known_wider): Likewise, turning into a template. >> * combine.c (make_extraction): Update use of GET_MODE_WIDER_MODE, >> forcing a wider mode to exist. >> * config/cr16/cr16.h (LONG_REG_P): Likewise. >> * rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise. >> * config/c6x/c6x.c (c6x_rtx_costs): Update use of >> GET_MODE_2XWIDER_MODE, forcing a wider mode to exist. >> * lower-subreg.c (init_lower_subreg): Likewise. >> * optabs-libfuncs.c (init_sync_libfuncs_1): Likewise, but not >> on the final iteration. >> * config/i386/i386.c (ix86_expand_set_or_movmem): Check whether >> a wider mode exists before asking for a move pattern. >> (get_mode_wider_vector): Update use of GET_MODE_WIDER_MODE, >> forcing a wider mode to exist. >> (expand_vselect_vconcat): Update use of GET_MODE_2XWIDER_MODE, >> returning false if no such mode exists. >> * config/ia64/ia64.c (expand_vselect_vconcat): Likewise. >> * config/mips/mips.c (mips_expand_vselect_vconcat): Likewise. >> * expmed.c (init_expmed_one_mode): Update use of GET_MODE_WIDER_MODE. >> Avoid checking for a MODE_INT if we already know the mode is not a >> SCALAR_INT_MODE_P. >> (extract_high_half): Update use of GET_MODE_WIDER_MODE, >> forcing a wider mode to exist. >> (expmed_mult_highpart_optab): Likewise. >> (expmed_mult_highpart): Likewise. >> * expr.c (expand_expr_real_2): Update use of GET_MODE_WIDER_MODE, >> using else_void. >> * lto-streamer-in.c (lto_input_mode_table): Likewise. >> * optabs-query.c (find_widening_optab_handler_and_mode): Likewise. >> * stor-layout.c (bit_field_mode_iterator::next_mode): Likewise. >> * internal-fn.c (expand_mul_overflow): Update use of >> GET_MODE_2XWIDER_MODE. >> * omp-low.c (omp_clause_aligned_alignment): Likewise. >> * tree-ssa-math-opts.c (convert_mult_to_widen): Update use of >> GET_MODE_WIDER_MODE. >> (convert_plusminus_to_widen): Likewise. >> * tree-switch-conversion.c (array_value_type): Likewise. >> * var-tracking.c (emit_note_insn_var_location): Likewise. >> * tree-vrp.c (simplify_float_conversion_using_ranges): Likewise. >> Return false inside rather than outside the loop if no wider mode >> exists >> * optabs.c (expand_binop): Update use of GET_MODE_WIDER_MODE >> and GET_MODE_2XWIDER_MODE >> (can_compare_p): Use else_void. >> * gdbhooks.py (OptMachineModePrinter): New class. >> (build_pretty_printer): Use it for opt_mode. >> >> gcc/ada/ >> * gcc-interface/decl.c (validate_size): Update use of >> GET_MODE_WIDER_MODE, forcing a wider mode to exist. > I'm not a big fan of the API here, particularly using operator* to > handle asserting the mode exists. I'd prefer to just use a member > function rather than overloading operator*. > > What's the rationale behind using operator* to imply the assertion? > > THe changes themsleves look fine, it's really just a question of the API > we present.
The original idea was to make opt_mode look pointer-ish, so that the dyn_cast <...> result could be used in the same way as for dyn_cast <gassign *> etc. The first cut therefore had operator bool () to test whether there was a mode and operator * to dereference it. However, operator bool () created various subtle problems (as it always seems to) so we dropped it in favour of exists (). I was neutral on whether we should keep '*' or switch to a function, so in the end the status quo won out. I'm happy to change it to a named accessor though. Any better ideas than "get ()" for the name? Maybe something to emphasis that it is asserting for non-nullness/non-emptiness (which '*' does implicitly)? Thanks, Richard
