diff mbox

[CHKP,i386] Fix retval generated for instrumented calls returning values on multiple registers

Message ID 20150402150759.GA6244@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich April 2, 2015, 3:07 p.m. UTC 
Hi,

This patch fixes nested PARALLEL in retval for isntrumented calls. Current possible call:

(call_insn:TI 6 30 17 2 (set (parallel [
                (expr_list:REG_DEP_TRUE (parallel:TI [
                            (expr_list:REG_DEP_TRUE (reg:DF 21 xmm0)
                                (const_int 0 [0]))
                            (expr_list:REG_DEP_TRUE (reg:DF 22 xmm1)
                                (const_int 8 [0x8]))
                        ])
                    (const_int 0 [0]))
                (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
                    (const_int 0 [0]))
                (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
                    (const_int 0 [0]))
            ])
        (call/j (mem:QI (symbol_ref:DI ("test1") [flags 0x41]  <function_decl 0x7f6f50e02ca8 test1.chkp>) [0 test1.chkp S1 A8])
            (const_int 0 [0]))) complex.c:11 670 {*call_value}

Such construction causes DF analysis problems.  This patch tranforms it to:

(call_insn:TI 6 30 17 2 (set (parallel:TI [
                (expr_list:REG_DEP_TRUE (reg:DF 21 xmm0)
                    (const_int 0 [0]))
                (expr_list:REG_DEP_TRUE (reg:DF 22 xmm1)
                    (const_int 8 [0x8]))
                (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
                    (const_int 0 [0]))
                (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
                    (const_int 0 [0]))
            ])
        (call/j (mem:QI (symbol_ref:DI ("test1") [flags 0x41]  <function_decl 0x7fb609bf7ca8 test1.chkp>) [0 test1.chkp S1 A8])
            (const_int 0 [0]))) complex.c:11 670 {*call_value}


Only MPX target is affected.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Does it look OK?

Thanks,
Ilya
--
2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>

	* config/i386/i386.c (ix86_expand_call): Avoid nested
	PARALLEL in returned call value.

Comments

Uros Bizjak April 2, 2015, 5:55 p.m. UTC | #1 
On Thu, Apr 2, 2015 at 5:07 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> This patch fixes nested PARALLEL in retval for isntrumented calls. Current possible call:
>
> (call_insn:TI 6 30 17 2 (set (parallel [
>                 (expr_list:REG_DEP_TRUE (parallel:TI [
>                             (expr_list:REG_DEP_TRUE (reg:DF 21 xmm0)
>                                 (const_int 0 [0]))
>                             (expr_list:REG_DEP_TRUE (reg:DF 22 xmm1)
>                                 (const_int 8 [0x8]))
>                         ])
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
>                     (const_int 0 [0]))
>             ])
>         (call/j (mem:QI (symbol_ref:DI ("test1") [flags 0x41]  <function_decl 0x7f6f50e02ca8 test1.chkp>) [0 test1.chkp S1 A8])
>             (const_int 0 [0]))) complex.c:11 670 {*call_value}
>
> Such construction causes DF analysis problems.  This patch tranforms it to:
>
> (call_insn:TI 6 30 17 2 (set (parallel:TI [
>                 (expr_list:REG_DEP_TRUE (reg:DF 21 xmm0)
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:DF 22 xmm1)
>                     (const_int 8 [0x8]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
>                     (const_int 0 [0]))
>             ])
>         (call/j (mem:QI (symbol_ref:DI ("test1") [flags 0x41]  <function_decl 0x7fb609bf7ca8 test1.chkp>) [0 test1.chkp S1 A8])
>             (const_int 0 [0]))) complex.c:11 670 {*call_value}
>
>
> Only MPX target is affected.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Does it look OK?
>
> Thanks,
> Ilya
> --
> 2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * config/i386/i386.c (ix86_expand_call): Avoid nested
>         PARALLEL in returned call value.

OK.

Thanks,
Uros.
Jan Hubicka April 2, 2015, 8:17 p.m. UTC | #2 
> Hi,
> 
> This patch fixes nested PARALLEL in retval for isntrumented calls. Current possible call:
> 
> (call_insn:TI 6 30 17 2 (set (parallel [
>                 (expr_list:REG_DEP_TRUE (parallel:TI [
>                             (expr_list:REG_DEP_TRUE (reg:DF 21 xmm0)
>                                 (const_int 0 [0]))
>                             (expr_list:REG_DEP_TRUE (reg:DF 22 xmm1)
>                                 (const_int 8 [0x8]))
>                         ])
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
>                     (const_int 0 [0]))
>             ])
>         (call/j (mem:QI (symbol_ref:DI ("test1") [flags 0x41]  <function_decl 0x7f6f50e02ca8 test1.chkp>) [0 test1.chkp S1 A8])
>             (const_int 0 [0]))) complex.c:11 670 {*call_value}
> 
> Such construction causes DF analysis problems.  This patch tranforms it to:
> 
> (call_insn:TI 6 30 17 2 (set (parallel:TI [
>                 (expr_list:REG_DEP_TRUE (reg:DF 21 xmm0)
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:DF 22 xmm1)
>                     (const_int 8 [0x8]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
>                     (const_int 0 [0]))
>                 (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
>                     (const_int 0 [0]))
>             ])
>         (call/j (mem:QI (symbol_ref:DI ("test1") [flags 0x41]  <function_decl 0x7fb609bf7ca8 test1.chkp>) [0 test1.chkp S1 A8])
>             (const_int 0 [0]))) complex.c:11 670 {*call_value}

Yep, nested parallels should not happen.

> 2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* config/i386/i386.c (ix86_expand_call): Avoid nested
> 	PARALLEL in returned call value.
> 
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 744642c..1d821cd 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -25624,8 +25624,19 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
>  	{
>  	  rtx b0 = gen_rtx_REG (BND64mode, FIRST_BND_REG);
>  	  rtx b1 = gen_rtx_REG (BND64mode, FIRST_BND_REG + 1);
> -	  retval = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (3, retval, b0, b1));
> -	  chkp_put_regs_to_expr_list (retval);
> +	  if (GET_CODE (retval) == PARALLEL)
> +	    {
> +	      b0 = gen_rtx_EXPR_LIST (VOIDmode, b0, const0_rtx);
> +	      b1 = gen_rtx_EXPR_LIST (VOIDmode, b1, const0_rtx);
> +	      rtx par = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, b0, b1));
> +	      retval = chkp_join_splitted_slot (retval, par);

I do not understand this code. Isn't it just droping the original return value
constructing prallelcontaining only the BND_REGs?

Honza
Uros Bizjak April 2, 2015, 8:37 p.m. UTC | #3 
On Thu, Apr 2, 2015 at 10:17 PM, Jan Hubicka <hubicka@ucw.cz> wrote:

>>         rtx b0 = gen_rtx_REG (BND64mode, FIRST_BND_REG);
>>         rtx b1 = gen_rtx_REG (BND64mode, FIRST_BND_REG + 1);
>> -       retval = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (3, retval, b0, b1));
>> -       chkp_put_regs_to_expr_list (retval);
>> +       if (GET_CODE (retval) == PARALLEL)
>> +         {
>> +           b0 = gen_rtx_EXPR_LIST (VOIDmode, b0, const0_rtx);
>> +           b1 = gen_rtx_EXPR_LIST (VOIDmode, b1, const0_rtx);
>> +           rtx par = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, b0, b1));
>> +           retval = chkp_join_splitted_slot (retval, par);
>
> I do not understand this code. Isn't it just droping the original return value
> constructing prallelcontaining only the BND_REGs?

Please see chkp_join_splitted_slot that merges two parallels.

Uros.
Jan Hubicka April 2, 2015, 9:14 p.m. UTC | #4 
> On Thu, Apr 2, 2015 at 10:17 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> 
> >>         rtx b0 = gen_rtx_REG (BND64mode, FIRST_BND_REG);
> >>         rtx b1 = gen_rtx_REG (BND64mode, FIRST_BND_REG + 1);
> >> -       retval = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (3, retval, b0, b1));
> >> -       chkp_put_regs_to_expr_list (retval);
> >> +       if (GET_CODE (retval) == PARALLEL)
> >> +         {
> >> +           b0 = gen_rtx_EXPR_LIST (VOIDmode, b0, const0_rtx);
> >> +           b1 = gen_rtx_EXPR_LIST (VOIDmode, b1, const0_rtx);
> >> +           rtx par = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, b0, b1));
> >> +           retval = chkp_join_splitted_slot (retval, par);
> >
> > I do not understand this code. Isn't it just droping the original return value
> > constructing prallelcontaining only the BND_REGs?
> 
> Please see chkp_join_splitted_slot that merges two parallels.

I see, OK then :)

Honza
> 
> Uros.
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 744642c..1d821cd 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -25624,8 +25624,19 @@  ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
 	{
 	  rtx b0 = gen_rtx_REG (BND64mode, FIRST_BND_REG);
 	  rtx b1 = gen_rtx_REG (BND64mode, FIRST_BND_REG + 1);
-	  retval = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (3, retval, b0, b1));
-	  chkp_put_regs_to_expr_list (retval);
+	  if (GET_CODE (retval) == PARALLEL)
+	    {
+	      b0 = gen_rtx_EXPR_LIST (VOIDmode, b0, const0_rtx);
+	      b1 = gen_rtx_EXPR_LIST (VOIDmode, b1, const0_rtx);
+	      rtx par = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, b0, b1));
+	      retval = chkp_join_splitted_slot (retval, par);
+	    }
+	  else
+	    {
+	      retval = gen_rtx_PARALLEL (VOIDmode,
+					 gen_rtvec (3, retval, b0, b1));
+	      chkp_put_regs_to_expr_list (retval);
+	    }
 	}
 
       call = gen_rtx_SET (VOIDmode, retval, call);