On Sun, Nov 08, 2015 at 07:42:04PM -0500, Michael Meissner wrote:
> - /* Power8 currently will only do the fusion if the top 11 bits of the addis
> - value are all 1's or 0's. */
> value = INTVAL (int_const);
> if ((value & (HOST_WIDE_INT)0xffff) != 0)
Space after cast, like (HOST_WIDE_INT) 0xffff .
> + /* Power8 currently will only do the fusion if the top 11 bits of the addis
> + value are all 1's or 0's. Ignore this restriction if we are testing
> + advanced fusion. */
> + if (TARGET_P9_FUSION)
> + return 1;
This comment seems out of date?
> ;; Match a GPR load (lbz, lhz, lwz, ld) that uses a combined address in the
> ;; memory field with both the addis and the memory offset. Sign extension
> ;; is not handled here, since lha and lwa are not fused.
> -(define_predicate "fusion_gpr_mem_combo"
> - (match_code "mem,zero_extend")
> +;; With extended fusion, also match a FPR load (lfd, lfs) and float_extend
And here?
> --- gcc/config/rs6000/rs6000.c (revision 229975)
> +++ gcc/config/rs6000/rs6000.c (working copy)
> @@ -376,8 +376,18 @@ struct rs6000_reg_addr {
> enum insn_code reload_fpr_gpr; /* INSN to move from FPR to GPR. */
> enum insn_code reload_gpr_vsx; /* INSN to move from GPR to VSX. */
> enum insn_code reload_vsx_gpr; /* INSN to move from VSX to GPR. */
> + enum insn_code fusion_gpr_ld; /* INSN for fusing gpr
> ADDIS/loads. */
> + /* INSNs for fusing addi with loads
> + or stores for each reg. class. */
>
> + enum insn_code fusion_addi_ld[(int)N_RELOAD_REG];
> + enum insn_code fusion_addi_st[(int)N_RELOAD_REG];
> + /* INSNs for fusing addis with loads
> + or stores for each reg. class. */
>
Trailing tabs.
> +/* Return true if the peephole2 can combine a load/store involving a
> + combination of an addis instruction and the memory operation. This was
> + added to the ISA 3.0 (power9) hardware. */
> +
> +bool
> +fusion_p9_p (rtx addis_reg, /* register set via addis. */
> + rtx addis_value, /* addis value. */
> + rtx dest, /* destination (memory or register). */
> + rtx src) /* source (register or memory). */
The function header comment should explain the params, after which you
can use the normal style for the function declaration itself.
> +(define_insn "*toc_fusionload_<mode>"
> + [(set (match_operand:QHSI 0 "int_reg_operand" "=&b,??r")
> + (match_operand:QHSI 1 "toc_fusion_mem_wrapped" "wG,wG"))
> + (unspec [(const_int 0)] UNSPEC_FUSION_ADDIS)
> + (use (match_operand:DI 2 "base_reg_operand" "r,r"))
> + (clobber (match_scratch:DI 3 "=X,&b"))]
> + "TARGET_TOC_FUSION_INT"
Do you need that "??r" alternative? Same for the next define_insn.
Big patch, most looks good :-)
Segher
