Message ID | 20141205145738.GB53395@msticlxl57.ims.intel.com |
---|---|
State | New |
Headers | show |
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
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)))]) + (define_insn "*jcc_1" [(set (pc) (if_then_else (match_operator 1 "ix86_comparison_operator" @@ -10965,10 +10983,10 @@ (label_ref (match_operand 0)) (pc)))] "" - "%!%+j%C1\t%l0" + "%+j%C1\t%l0" [(set_attr "type" "ibr") (set_attr "modrm" "0") - (set (attr "length_nobnd") + (set (attr "length") (if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -126)) (lt (minus (match_dup 0) (pc)) @@ -10976,6 +10994,24 @@ (const_int 2) (const_int 6)))]) +(define_insn "*jcc_2_bnd" + [(set (pc) + (if_then_else (match_operator 1 "ix86_comparison_operator" + [(reg FLAGS_REG) (const_int 0)]) + (pc) + (label_ref (match_operand 0))))] + "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)))]) + (define_insn "*jcc_2" [(set (pc) (if_then_else (match_operator 1 "ix86_comparison_operator" @@ -10983,10 +11019,10 @@ (pc) (label_ref (match_operand 0))))] "" - "%!%+j%c1\t%l0" + "%+j%c1\t%l0" [(set_attr "type" "ibr") (set_attr "modrm" "0") - (set (attr "length_nobnd") + (set (attr "length") (if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -126)) (lt (minus (match_dup 0) (pc)) @@ -11420,13 +11456,28 @@ ;; Unconditional and other jump instructions +(define_insn "jump_bnd" + [(set (pc) + (label_ref (match_operand 0)))] + "TARGET_MPX && ix86_bnd_prefixed_insn_p (insn)" + "bnd jmp\t%l0" + [(set_attr "type" "ibr") + (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 6))) + (set_attr "modrm" "0")]) + (define_insn "jump" [(set (pc) (label_ref (match_operand 0)))] "" - "%!jmp\t%l0" + "jmp\t%l0" [(set_attr "type" "ibr") - (set (attr "length_nobnd") + (set (attr "length") (if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -126)) (lt (minus (match_dup 0) (pc))