[i386] Fix PR64003

Hi,

This patch fixes PR target/64003 by avoiding functions calls during computations of "length" attribute for short jump instructions.  It is achieved by having separate templates for prefixed and not prefixed instructions.  Please see discussion in bugzilla for reasoning.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  Valgrind run for reproducer shows problem is fixed.  OK for trunk?

Thanks,
Ilya
--
2014-12-05  Ilya Enkovich  <ilya.enkovich@intel.com>

	* config/i386/i386.md (*jcc_1_bnd): New.
	(*jcc_2_bnd): New.
	(jump_bnd): New.
	(*jcc_1): Remove bnd prefix.
	(*jcc_2): Likewise.
	(jump): Likewise.

On 12/05/14 07:57, Ilya Enkovich wrote:
> Hi,
>
> This patch fixes PR target/64003 by avoiding functions calls during computations of "length" attribute for short jump instructions.  It is achieved by having separate templates for prefixed and not prefixed instructions.  Please see discussion in bugzilla for reasoning.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  Valgrind run for reproducer shows problem is fixed.  OK for trunk?
>
> Thanks,
> Ilya
> --
> 2014-12-05  Ilya Enkovich  <ilya.enkovich@intel.com>
>
> 	* config/i386/i386.md (*jcc_1_bnd): New.
> 	(*jcc_2_bnd): New.
> 	(jump_bnd): New.
> 	(*jcc_1): Remove bnd prefix.
> 	(*jcc_2): Likewise.
> 	(jump): Likewise.
Just wanted to say thanks for taking care of this.  The obscure and 
undocumented rules about what can appear in these length computations 
is, umm, bad and I don't think anyone could reasonably have expected you 
to know about them.

jeff

Ilya Enkovich <enkovich.gnu@gmail.com> writes:
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 88435d6..9019ed8 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -10958,6 +10958,24 @@
>  ;; Basic conditional jump instructions.
>  ;; We ignore the overflow flag for signed branch instructions.
>  
> +(define_insn "*jcc_1_bnd"
> +  [(set (pc)
> +	(if_then_else (match_operator 1 "ix86_comparison_operator"
> +				      [(reg FLAGS_REG) (const_int 0)])
> +		      (label_ref (match_operand 0))
> +		      (pc)))]
> +  "TARGET_MPX && ix86_bnd_prefixed_insn_p (insn)"
> +  "bnd %+j%C1\t%l0"
> +  [(set_attr "type" "ibr")
> +   (set_attr "modrm" "0")
> +   (set (attr "length")
> +	   (if_then_else (and (ge (minus (match_dup 0) (pc))
> +				  (const_int -126))
> +			      (lt (minus (match_dup 0) (pc))
> +				  (const_int 128)))
> +	     (const_int 3)
> +	     (const_int 7)))])
> +

Sorry, looking at this again, shouldn't the offset be -125 rather
than -126?  The pc in:

   (ge (minus (match_dup 0) (pc))
       (const_int -126))

is the start of the instruction, so we need to add the length
of the jump itself to the real minimum range of -128.

Thanks,
Richard

2014-12-07 12:51 GMT+03:00 Richard Sandiford <rdsandiford@googlemail.com>:
> Ilya Enkovich <enkovich.gnu@gmail.com> writes:
>> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
>> index 88435d6..9019ed8 100644
>> --- a/gcc/config/i386/i386.md
>> +++ b/gcc/config/i386/i386.md
>> @@ -10958,6 +10958,24 @@
>>  ;; Basic conditional jump instructions.
>>  ;; We ignore the overflow flag for signed branch instructions.
>>
>> +(define_insn "*jcc_1_bnd"
>> +  [(set (pc)
>> +     (if_then_else (match_operator 1 "ix86_comparison_operator"
>> +                                   [(reg FLAGS_REG) (const_int 0)])
>> +                   (label_ref (match_operand 0))
>> +                   (pc)))]
>> +  "TARGET_MPX && ix86_bnd_prefixed_insn_p (insn)"
>> +  "bnd %+j%C1\t%l0"
>> +  [(set_attr "type" "ibr")
>> +   (set_attr "modrm" "0")
>> +   (set (attr "length")
>> +        (if_then_else (and (ge (minus (match_dup 0) (pc))
>> +                               (const_int -126))
>> +                           (lt (minus (match_dup 0) (pc))
>> +                               (const_int 128)))
>> +          (const_int 3)
>> +          (const_int 7)))])
>> +
>
> Sorry, looking at this again, shouldn't the offset be -125 rather
> than -126?  The pc in:
>
>    (ge (minus (match_dup 0) (pc))
>        (const_int -126))
>
> is the start of the instruction, so we need to add the length
> of the jump itself to the real minimum range of -128.

You are right.  Similarly upper bound should be changed from 128 to 129.

Thanks,
Ilya

>
> Thanks,
> Richard