Hi Dimitar,
Just some comments, do with it what you want :-)
On Sun, Jun 09, 2019 at 11:01:38PM +0300, Dimitar Dimitrov wrote:
> +; An unfortunate side effect is that quite a few invalid RTL patterns are
> +; generated. For example:
> +; ... (zero_extend:SI (match_operand:SI ...)) ...
You could perhaps use a mode iterator like rs6000's EXTHI and friends.
> +; These patterns are harmless since no pass should generate such RTL. This
> +; shortcut allows us to keep small and concise machine description patterns.
But the generated code (of GCC itself) will be larger. It's also not
harmless in that it can complicate your debugging, problems take longer
to spot.
> +(define_insn
> "add_impl<EQD:mode><EQS0:mode><EQS1:mode>_<alu3_zext><alu3_zext_op1><alu3_zext_op2>"
Many lines are too long. Not that it is clear how to fix that in this
particular case ;-)
> + [(set_attr "type" "alu,alu,alu")])
You can just say
[(set_attr "type" "alu")])
if all alternatives are the same value for an attribute.
> +(define_insn "one_impl<EQD:mode><EQS0:mode>_<alu2_zext>"
The standard pattern is called one_cmpl, so maybe you want one_cmpl_impl
here? one_impl looks like a typo.
> +(define_subst "alu2_zext_subst"
> + [(set (match_operand:EQD 0 "" "")
> + (ALUOP2:EQD (zero_extend:EQD (match_operand:EQD 1 "" ""))))]
I don't know if this actually works for define_subst, but at least in many
other cases you can write this like
(define_subst "alu2_zext_subst"
[(set (match_operand:EQD 0)
(ALUOP2:EQD (zero_extend:EQD (match_operand:EQD 1))))]
(you can omit trailing empty string arguments).
> +(define_predicate "pru_muldst_operand"
> + (match_code "subreg,reg")
> +{
> + if (register_operand (op, mode))
> + {
> + int regno;
> +
> + if (REG_P (op))
> + regno = REGNO (op);
> + else if (GET_CODE (op) == SUBREG && REG_P (SUBREG_REG (op)))
> + regno = REGNO (SUBREG_REG (op));
> + else
> + return 0;
> +
> + return REGNO_REG_CLASS (regno) == MULDST_REGS
> + || regno >= FIRST_PSEUDO_REGISTER;
> + }
> + return 0;
> +})
> +static bool
> +pru_hard_regno_scratch_ok (unsigned int regno)
> +{
> + /* Don't allow hard registers that might be part of the frame pointer.
> + Some places in the compiler just test for [HARD_]FRAME_POINTER_REGNUM
> + and don't handle a frame pointer that spans more than one register.
> + TODO: Fix those faulty places. */
> +
> + if ((!reload_completed || frame_pointer_needed)
> + && ((regno >= HARD_FRAME_POINTER_REGNUM
> + && regno <= HARD_FRAME_POINTER_REGNUM + 3)
> + || (regno >= FRAME_POINTER_REGNUM
> + && regno <= FRAME_POINTER_REGNUM + 3)))
> + return false;
Use IN_RANGE?
> + /* QBxx conditional branching cannot cope with block reordering. */
> + if (flag_reorder_blocks_and_partition)
> + {
> + inform (input_location, "%<-freorder-blocks-and-partition%> "
> + "not supported on this architecture");
> + flag_reorder_blocks_and_partition = 0;
> + flag_reorder_blocks = 1;
> + }
What you cannot cope with is the hot/cold partitioning, I guess --
otherwise you'd have to disable reorder_blocks itself, and that would
result in pretty terrible code.
> +; There is no pipeline, so our scheduling description is simple.
> +(define_automaton "pru")
> +(define_cpu_unit "cpu" "pru")
> +
> +(define_insn_reservation "everything" 1 (match_test "true") "cpu")
Because you have a scheduling description, INSN_SCHEDULING is defined,
and that makes combine not create SUBREGs of MEM. Which is pretty
important :-)
It looks like a quite complete port, congratulations :-)
Segher
