> (define_predicate "call_address_operand"
> (match_code "symbol_ref,const,reg")
> {
> return (symbolic_operand (op, mode) || (GET_CODE (op) == REG));
> })
Nit.
(define_predicate "call_address_operand"
(ior (match_code "reg")
(match_operand 0 "symbolic_operand")))
> (define_special_predicate "any_gpr_operand"
> (match_code "subreg,reg")
> {
> return gpr_operand (op, mode);
> })
(define_special_predicate "any_gpr_operand"
(match_operand 0 "gpr_operand"))
though, I'm not sure what this achieves... while any_gpr_operand
will ignore its mode, passing on that same mode to gpr_operand wont.
> (define_insn_and_split "move_frame"
> [(set (match_operand:SI 0 "gpr_operand" "=r")
> (match_operand:SI 1 "register_operand" "r"))
> (clobber (reg:CC CC_REGNUM))]
> "operands[1] == frame_pointer_rtx || operands[1] == arg_pointer_rtx"
It looks to me that there are several places that could be tidied up
if you have a frame_register_operand predicate to use here instead of
testing operands[] in the extra_constraints field.
> ;; If the frame pointer elimination offset is zero, we'll use this pattern.
> (define_insn_and_split "*move_frame_1"
> [(set (match_operand:SI 0 "gpr_operand" "=r")
> (match_operand:SI 1 "register_operand" "r"))
> (clobber (reg:CC CC_REGNUM))]
> "(reload_in_progress || reload_completed)
> && (operands[1] == stack_pointer_rtx
> || operands[1] == hard_frame_pointer_rtx)"
> "#"
> "&& 1"
> [(set (match_dup 0) (match_dup 1))])
Duplicate of the above? I really don't see how they differ...
> ;; reload uses gen_addsi2 because it doesn't understand the need for
> ;; the clobber.
> (define_peephole2
> [(set (match_operand:SI 0 "gpr_operand" "")
> (match_operand:SI 1 "const_int_operand" ""))
> (parallel [(set (match_dup 0)
> (plus:SI (match_dup 0)
> (match_operand:SI 2 "gpr_operand")))
> (clobber (reg:CC UNKNOWN_REGNUM))])]
I'd like to understand all of this a little more.
Given that reload *would* generate an add3 (with no clobber), and
add-without-clobber isn't ordinarily a valid insn, why not just go
ahead and use that as your reload pattern instead of this
clobber placeholder?
Wouldn't a bare plus pattern with that same reload_in_progress ||
reload_completed
check work just as well for all the pattern matching you want to do
during peephole analysis?
> int scratch = (0x17
> ^ (true_regnum (operands[0]) & 1)
> ^ (true_regnum (operands[1]) & 2)
> ^ (true_regnum (operands[2]) & 4));
> asm_fprintf (asm_out_file, \"\tstr r%d,[sp,#0]\n\", scratch);
> asm_fprintf (asm_out_file, \"\tmovfs r%d,status\n\", scratch);
> output_asm_insn (\"add %0,%1,%2\", operands);
> asm_fprintf (asm_out_file, \"\tmovts status,r%d\n\", scratch);
> asm_fprintf (asm_out_file, \"\tldr r%d,[sp,#0]\n\", scratch);
> return \"\";
It does seem like you'd do well to split this pattern. If you do, then you'll
automatically get the right changes to debugging unwind info across this.
A test for epilogue_completed in the split condition should be sufficient to
wait until after peephole2 has finished, so that you don't interfere with the
transformations you want there.
I'm also interested in hearing about how well this whole scheme works in
practice, as opposed to merely waiting until after reload to split and flags
users. There are certainly lots of other ports that are in the same boat
with respect to only having a flags-clobbering add.
> (define_insn_and_split "*recipsf2_1"
> [(match_parallel 4 "float_operation"
> [(set (match_operand:SF 0 "gpr_operand" "=r,r")
> (div:SF (match_operand:SF 1 "const_float_1_operand" "")
> (match_operand:SF 2 "move_src_operand" "rU16m,rU16mCal")))
> (use (match_operand:SI 3 "move_src_operand" "rU16m,rU16mCal"))
How to you prevent a post-reload copy propagation pass from putting things
back just the way before you split them? It seems to me that's the primary
reason to use specific register constraints here.
> (define_insn "fmadd"
> [(match_parallel 4 "float_operation"
> [(set (match_operand:SF 0 "gpr_operand" "=r")
> (fma:SF (match_operand:SF 2 "gpr_operand" "%r")
> (match_operand:SF 3 "gpr_operand" "r")
> (match_operand:SF 1 "gpr_operand" "0")))
> (clobber (reg:CC_FP CCFP_REGNUM))])]
Presumably the strange operand ordering is left over from your port-specific
builtins? Also, the % is extraneous since the constraints are identical.
> ; combiner pattern, also used by vector combiner pattern
> (define_expand "maddsf"
> [(parallel
> [(set (match_operand:SF 0 "gpr_operand" "=r")
> (plus:SF (mult:SF (match_operand:SF 1 "gpr_operand" "%r")
> (match_operand:SF 2 "gpr_operand" "r"))
> (match_operand:SF 3 "gpr_operand" "0")))
> (clobber (reg:CC_FP CCFP_REGNUM))])]
> "TARGET_FUSED_MADD")
I suspect these aren't needed anymore. Anything the combiner could have
found should be able to be found by the SSA optimizers.
> ; leave to desaster.
lead to disaster
> (and:SI (match_operand:SI 1 "gpr_operand" "%r")
> (match_operand:SI 2 "gpr_operand" "r")))
More useless %. And more in the other logicals.
> (define_expand "one_cmplsi2"
> [(set (match_operand:SI 0 "gpr_operand" "")
> (xor:SI (match_operand:SI 1 "gpr_operand" "")
> (match_dup 2)))]
> ""
> "emit_insn (gen_xorsi3 (operands[0], operands[1],
> force_reg (SImode, GEN_INT (-1))));
> DONE;")
>
> (define_insn "*one_cmplsi2_i"
> [(set (match_operand:SI 0 "gpr_operand" "=r")
> (not:SI (match_operand:SI 1 "gpr_operand" "r")))
> (clobber (reg:CC CC_REGNUM))]
> "epiphany_m1reg >= 0"
> "eor %0,%1,%-")
Why not combine these? I'm pretty sure that expand_binop will try the xor
solution all on its own. I'd think you'd only want to not use that when
m1reg is available.
Of course, if gpr_operand were to include -1 when m1reg is available, and
you swapped all the constraints to something other than "r", you get even
this for free.
> "*
> {
> rtx xop[3];
>
> xop[0] = operands[0];
> xop[1] = operands[1];
> xop[2] = GEN_INT (31-INTVAL (operands[2]));
> output_asm_insn (\"lsl %0,%1,%2\", xop);
> return \"\";
> }")
Please don't use "* in new code; { } is sufficient, and you get to remove
all of the gross \" bits.
> (define_insn "*mov<mode>cc_insn"
> [(set (match_operand:WMODE 0 "gpr_operand" "=r")
> (if_then_else:WMODE (match_operator 3 "proper_comparison_operator"
> [(match_operand 4 "cc_operand") (const_int 0)])
> (match_operand:WMODE 1 "nonmemory_operand" "r")
> (match_operand:WMODE 2 "gpr_operand" "0")))]
If there's a good reason for nonmemory_operand and not gpr_operand here,
you should add a big comment. It looks like a mistake.
> static bool
> epiphany_frame_pointer_required (void)
> {
> return cfun->calls_alloca;
Isn't this automatic?
> epiphany-load-combiner.o : $(srcdir)/config/epiphany/epiphany-load-combiner.c
> \
Missing file?
> #define IMM16(X) ((unsigned)(X) <= 0xFFFF)
> #define IMM5(X) ((unsigned)(X) <= 0x1F)
These need to be unsigned HOST_WIDE_INT, at minimum.
Preferably no cast at all and compare vs 0 as well.
> /* ??? This currently isn't used. Waiting for PIC. */
> #if 0
> #define EXTRA_CONSTRAINT(VALUE, C) \
> ((C) == 'R' ? (SYMBOL_REF_FUNCTION_P (VALUE) || GET_CODE (VALUE) ==
> LABEL_REF) \
Just remove it.
There appear to be quite a number of macros in this file that have been
migrated to target hooks. Please move them.
> /* For DWARF. Marginally different than default so output is "prettier"
> (and consistent with above). */
> #define PUSHSECTION_ASM_OP "\t.section "
Unused?
> /* Tell crtstuff.c we're using ELF. */
> #define OBJECT_FORMAT_ELF
Why wouldn't you get this from config/elfos.h?
r~
