Re: PR target/23303 (worse instruction choice)

[Steven Bosscher <stevenb at suse dot de>]

On Wednesday 02 November 2005 21:03, Jan Hubicka wrote:
> 2005-10-31  Jan Hubicka  <jh@suse.cz>
> 	PR target/23303
> 	* i386.md: Add peep2 for simplyfing array accesses.
> Index: config/i386/i386.md
> ===================================================================
> --- config/i386/i386.md	(revision 106395)
> +++ config/i386/i386.md	(working copy)
> @@ -19785,6 +19785,64 @@
>    if (!rtx_equal_p (operands[0], operands[1]))
>      emit_move_insn (operands[0], operands[1]);
>  })
> +
> +;; After splitting up read-modify operations, array accesses with memory
> +;; operands might end up in form:
> +;;  sall    $2, %eax
> +;;  movl    4(%esp), %edx
> +;;  addl    %edx, %eax
> +;; instead of pre-splitting:
> +;;  sall    $2, %eax
> +;;  addl    4(%esp), %eax
> +;; Turn it into:
> +;;  movl    4(%esp), %edx
> +;;  leal    (%edx,%eax,4), %eax
> +
> +(define_peephole2
> +  [(parallel [(set (match_operand 0 "register_operand" "")
> +		   (ashift (match_operand 1 "register_operand" "")
> +			   (match_operand 2 "const_int_operand" "")))
> +	       (clobber (reg:CC FLAGS_REG))])
> +   (set (match_operand 3 "register_operand")
> +        (match_operand 4 "x86_64_general_operand" ""))
> +   (parallel [(set (match_operand 5 "register_operand" "")
> +		   (plus (match_operand 6 "register_operand" "")
> +			 (match_operand 7 "register_operand" "")))
> +		   (clobber (reg:CC FLAGS_REG))])]
> +  "INTVAL (operands[2]) >= 0 && INTVAL (operands[2]) <= 3
> +   /* Validate MODE for lea.  */
> +   && ((!TARGET_PARTIAL_REG_STALL
> +	&& (GET_MODE (operands[0]) == QImode
> +	    || GET_MODE (operands[0]) == HImode))
> +       || GET_MODE (operands[0]) == SImode
> +       || (TARGET_64BIT && GET_MODE (operands[0]) == DImode))
> +   /* We reorder load and the shift.  */
> +   && !rtx_equal_p (operands[1], operands[3])
> +   && !reg_overlap_mentioned_p (operands[0], operands[4])
> +   /* Last PLUS must consist of operand 0 and 3.  */
> +   && !rtx_equal_p (operands[0], operands[3])
> +   && (rtx_equal_p (operands[3], operands[6])
> +       || rtx_equal_p (operands[3], operands[7]))
> +   && (rtx_equal_p (operands[0], operands[6])
> +       || rtx_equal_p (operands[0], operands[7]))
> +   /* The intermediate operand 0 must die or be same as output.  */
> +   && (rtx_equal_p (operands[0], operands[5])
> +       || peep2_reg_dead_p (3, operands[0]))"
> +  [(set (match_dup 3) (match_dup 4))
> +   (set (match_dup 0) (match_dup 1))]
> +{
> +  enum machine_mode mode = GET_MODE (operands[5]) == DImode ? DImode : SImode; +  int scale = 1 << INTVAL (operands[2]);
> +  rtx index = gen_lowpart (Pmode, operands[1]);
> +  rtx base = gen_lowpart (Pmode, operands[3]);
> +  rtx dest = gen_lowpart (mode, operands[5]);
> +
> +  operands[1] = gen_rtx_PLUS (Pmode, base,
> +  			      gen_rtx_MULT (Pmode, index, GEN_INT (scale)));
> +  if (mode != Pmode)
> +    operands[1] = gen_rtx_SUBREG (mode, operands[1], 0);
> +  operands[0] = dest;
> +})
>  
>  ;; Call-value patterns last so that the wildcard operand does not
>  ;; disrupt insn-recog's switch tables.

Is this whole huge complicated peephole really worth it?  I'd expect
the pattern to match quite a few times, but all the additional checks
probably make it not trigger very often.  Honza, do you have numbers
about the effects of this patch??

Gr.
Steven