Cleaning up expand optabs code

This patch adds a few helper functions for dealing with optabs:

- insn_operand_matches (ICODE, OPNO, X)

  Return true if X is suitable for operand OPNO of instruction ICODE.

  [ I've deliberately left the mode out here.  We're testing whether
    something matches a match_operand (or suchlike) in the .md file.
    As far as recog is concerned, it's always the mode specified in
    the .md file that should be passed to the predicate, and I think
    that should be the rule for expand as well.

    Of course, in many cases, expand "knows" which mode the .md file
    is supposed to use.  But I think we should honour the .md-file
    mode even then, and abort where there's a mismatch.  We shouldn't
    just ignore what the .md file says.  I suppose this could expose
    a few .md-file bugs though...

    At the moment, some direct predicate calls are careful to use
    the match_operand mode, but others aren't. ]

- (maybe_)legitimize_insn_target (ICODE, OPNO, TARGET, MODE)

  Replaces the idiom:

      if (!TARGET
          || TARGET == const0_rtx
          || GET_MODE (TARGET) != MODE
          || !insn_operand_matches (ICODE, OPNO, TARGET))
        TARGET = gen_reg_rtx (...);

  and the various simplications of it.

  [ I think we should assert here that the new target really is OK,
    rather than passing it to the generator function regardless.
    Passing a bogus value would often trigger a later recog failure,
    but it might not be obvious at that stage where the instruction
    came from. ]

- (maybe_)legitimize_insn_source (ICODE, OPNO, SOURCE, MODE)

  Same idea for:

      if (!insn_operand_matches (ICODE, OPNO, SOURCE))
        SOURCE = copy_to_mode_reg (MODE, SOURCE);

  [ Here too I think we should assert that the new source is OK. ]

The main motivations are:

  * It should be simpler to add new optabs (I want to add a few more...)

  * I hope it'll be easier to avoid the kinds of bug that I came
    across while going through the optabs code (more below).

  * We could be more flexible in future.  E.g. I was told about a port
    in which several named patterns accept restricted memory operands.
    At the moment, I think you have to do something like:

    (define_expand "blah"
      [... (match_operand:M N "memory_operand") ...]
      "..."
      {
        if (!restricted_memory_operand (operands[N], M))
	  operands[N] = replace_equiv_address (operands[N],
					       force_reg (...));
      })

    (define_insn "*blah"
      [... (match_operand:M N "restricted_memory_operand") ...]
      ...

    It'd be nice if expand knew how to handle this automatically.
    It should be fairly easy now that predicates have a more
    declarative form.

A few notes:

- The new functions take insn_code arguments, so various bits of code
  that found it easier to use ints need to be changed to use the
  "proper" type instead.  This includes...

- prepare_operands.  I moved the declaration from expr.h to optabs.h,
  where we can be sure that insn_code is available.  This feels right
  anyway, since the function is defined in optabs.c.

  I've left this as a separate function for now because it can be called
  after reload as well.  I wanted to concentrate on expand in this patch.

- Parts of store_bit_field_1 used sequences rather than the usual
  delete_insns_since approach.  When using the new functions,
  the flow is easier with delete_insns_since.  Using delete_insns_since
  also avoids allocating unnecessary memory.

- If a movstrict expansion failed, the code would rightly try to delete
  any set-up instructions that had been created.  However, it stored the
  results of those set-up instructions in the function-wide "value" variable,
  so the rest of the function could end up using an uninitialised pseudo.

- The followuing functions didn't clean up any set-up instructions if
  the optab couldn't be used:

  - expand_builtin_strlen
  - emit_cstore
  - emit_storent_insn
  - expand_binop_directly (if a mode test failed)

  The insns would probably be deleted as dead by later optimisers,
  but it seems better to get rid of them straight away.

- Arguably the same sort of problem applied to:

  - expand_sync_operation
  - expand_sync_fetch_operation
  - expand_sync_lock_test_and_set

  although the effect here was simply that, if we forced something
  to a register for an insn that we ended up not being able to use,
  we'd keep it in that form anyway for the fallback code.

- The vec_extract code treated an output operand as an input:

      if (! (*insn_data[icode].operand[0].predicate) (dest, mode0))
	dest = copy_to_mode_reg (mode0, dest);

- I first tried to make insn_operand_matches an inline function,
  but I can't think of a good header file that is guaranteed to
  know about both recog.h and insn-codes.h.

The patch only tackles files in gcc/ itself.  If it's OK, I'll do a
follow-up for the backends.

Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?

Richard

gcc/
	* expr.h (prepare_operand): Move to optabs.h.
	* optabs.h (emit_unop_insn): Change insn code parameter from "int"
	to "enum insn_code".
	(maybe_emit_unop_insn): Likewise.
	(prepare_operand): Likewise.  Move from expr.h.
	(insn_operand_matches): Declare.
	(maybe_legitimize_insn_target, legitimize_insn_target): Likewise.
	(maybe_legitimize_insn_source, legitimize_insn_source): Likewise.
	* builtins.c (expand_builtin_prefetch): Call convert_memory_address
	unconditionally.  Use legitimize_insn_source.
	(expand_builtin_interclass_mathfn): Use legitimize_insn_target.
	(expand_movstr): Likewise.   Use GEN_FCN.
	(expand_builtin___clear_cache): Use legitimize_insn_source.
	(expand_builtin_strlen): Likewise.  Clean up if the expander FAILs.
	(expand_builtin_lock_release): Likewise.
	* explow.c (probe_stack_range): Use legitimize_insn_source.
	(allocate_dynamic_stack_space): Likewise.  Call convert_to_mode
	unconditionally beforehand.
	* expmed.c (check_predicate_volatile_ok): Remove mode parameter.
	Use insn_operand_matches.
	(store_bit_field_1): Use insn_operand_matches and
	maybe_legitimize_insn_source.  Don't generate a sequences when
	tentatively expanding an instruction; use delete_insns_since instead.
	Localise the second operand to movstrict.  Update calls to
	check_predicate_volatile_ok.  Use "enum insn_code" for instruction
	codes.
	(extract_bit_field_1): Likewise, except for movstrict.
	Use maybe_legitimize_insn_target.
	(emit_cstore): Use insn_operand_matches and
	maybe_legitimize_insn_target.  Clean up if the expansion fails.
	* expr.c (init_expr_target): Use insn_operand_matches.
	(compress_float_constant): Likewise.
	(emit_block_move_via_movmem): Use insn_operand_matches and
	maybe_legitimize_insn_source.
	(set_storage_via_setmem): Likewise.
	(emit_storent_insn): Likewise.  Clean if up the expansion fails.
	(emit_single_push_insn): Use legitimize_insn_source.
	(expand_assignment): Likewise.
	(try_casesi): Likewise.
	* function.c (safe_insn_predicate): Use insn_operand_matches.
	(assign_parm_setup_reg): Likewise.
	* optabs.c (insn_operand_matches): New function.
	(maybe_legitimize_insn_target, legitimize_insn_target): Likewise.
	(maybe_legitimize_insn_source, legitimize_insn_source): Likewise.
	(expand_vec_shift_expr): Use legitimize_insn_source and
	legitimize_insn_target.
	(expand_vec_cond_expr): Likewise.
	(expand_widen_pattern_expr): Likewise.  Change icode to an insn_code.
	(expand_ternary_op): Likewise.
	(expand_unop_direct): Likewise.
	(maybe_emit_unop_insn): Likewise.
	(expand_binop_directly): Likewise.  Clean up if the mode isn't
	acceptable.
	(expand_twoval_unop): Change icode to an insn_code.
	Use insn_operand_matches and maybe_legitimize_insn_source.
	(expand_twoval_binop): Likewise.
	(expand_copysign_absneg): Change icode to an insn_code.
	(emit_unop_insn): Likewise.
	(emit_unop): Likewise.  Use insn_operand_matches.
	(can_compare_p): Likewise.
	(prepare_operand): Likewise.
	(gen_add2_insn): Likewise.
	(gen_add3_insn): Likewise.
	(have_add2_insn): Likewise.
	(gen_sub2_insn): Likewise.
	(gen_sub3_insn): Likewise.
	(have_sub2_insn): Likewise.
	(prepare_cmp_insn): Use insn_operand_matches.
	(emit_cmp_and_jump_insn_1): Likewise.
	(gen_cond_trap): Likewise.
	(emit_indirect_jump): Use legitimize_insn_source.
	(vector_compare_rtx): Likewise.
	(emit_conditional_move): Use maybe_legitimize_insn_source
	and maybe_legitimize_insn_target.
	(emit_conditional_add): Likewise.
	(expand_val_compare_and_swap_1): Likewise.  Clean up on failure.
	(expand_sync_operation): Likewise.  Use local variables for the
	coerced operands.
	(expand_sync_fetch_operation): Likewise.
	(expand_sync_lock_test_and_set): Likewise.
	* reload.c (find_reloads_address_1): Change icode to an insn_code.
	Use insn_operand_matches.
	* reload1.c (gen_reload): Likewise.
	* targhooks.c (default_secondary_reload): Use insn_operand_matches.

Cleaning up expand optabs code

Commit Message

Comments

Patch