diff mbox

[MPX,2/X] Pointers Checker [9/25] Bound constants

Message ID 20131031091516.GE54327@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Oct. 31, 2013, 9:15 a.m. UTC 
Hi,

Here is a patch which adds support for bound constant to be used as DECL_INITIAL for constant static bounds generated by compiler.

Thanks,
Ilya
--

gcc/

2013-10-23  Ilya Enkovich  <ilya.enkovich@intel.com>

	* emit-rtl.c (immed_double_const): Support MODE_POINTER_BOUNDS.
	* explow.c (trunc_int_for_mode): Likewise.
	* varpool.c (ctor_for_folding): Do not fold constant
	bounds vars.

Comments

Jeff Law Nov. 7, 2013, 6:54 p.m. UTC | #1 
On 10/31/13 03:15, Ilya Enkovich wrote:
> Hi,
>
> Here is a patch which adds support for bound constant to be used as DECL_INITIAL for constant static bounds generated by compiler.
>
> Thanks,
> Ilya
> --
>
> gcc/
>
> 2013-10-23  Ilya Enkovich  <ilya.enkovich@intel.com>
>
> 	* emit-rtl.c (immed_double_const): Support MODE_POINTER_BOUNDS.
> 	* explow.c (trunc_int_for_mode): Likewise.
> 	* varpool.c (ctor_for_folding): Do not fold constant
> 	bounds vars.
I'm having a bit of trouble reconciling "add support for bound constant 
to be used as DECL_INITIAL" rationale text and the actual patch.

 From reading the patch it appears that you want to allow generation of 
immediate constants for objects with MODE_POINTER_BOUNDS.  OK, I can see 
how that is useful.

I can kindof see how you want to error out if someone asks for a 
constant to be truncated to MODE_POINTER_BOUNDS.  Did this trip in 
practice or is it preemptive?


> diff --git a/gcc/varpool.c b/gcc/varpool.c
> index 2eb1fc1..d9c08c1 100644
> --- a/gcc/varpool.c
> +++ b/gcc/varpool.c
> @@ -254,6 +254,12 @@ ctor_for_folding (tree decl)
>         && TREE_CODE (decl) != CONST_DECL)
>       return error_mark_node;
>
> +  /* Static constant bounds are created to be
> +     used instead of constants and therefore
> +     do not let folding it.  */
> +  if (POINTER_BOUNDS_P (decl))
> +    return error_mark_node;
Here's the part I'm struggling a bit with.    Why did you need this?

Isn't this going to prevent that DECL from being used in folding?   The 
bounds shouldn't really affect that AFAICT.

jeff
Ilya Enkovich Nov. 7, 2013, 7:59 p.m. UTC | #2 
2013/11/7 Jeff Law <law@redhat.com>:
> On 10/31/13 03:15, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> Here is a patch which adds support for bound constant to be used as
>> DECL_INITIAL for constant static bounds generated by compiler.
>>
>> Thanks,
>> Ilya
>> --
>>
>> gcc/
>>
>> 2013-10-23  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         * emit-rtl.c (immed_double_const): Support MODE_POINTER_BOUNDS.
>>         * explow.c (trunc_int_for_mode): Likewise.
>>         * varpool.c (ctor_for_folding): Do not fold constant
>>         bounds vars.
>
> I'm having a bit of trouble reconciling "add support for bound constant to
> be used as DECL_INITIAL" rationale text and the actual patch.
>
> From reading the patch it appears that you want to allow generation of
> immediate constants for objects with MODE_POINTER_BOUNDS.  OK, I can see how
> that is useful.
>
> I can kindof see how you want to error out if someone asks for a constant to
> be truncated to MODE_POINTER_BOUNDS.  Did this trip in practice or is it
> preemptive?

As far as I remember change in trunc_int_mode was required to expand
bound constants on 32bit target. Size of the constant is equal to size
of the HOST_WIDE_INT and thus constant generation goes through
gen_int_mode and trunc_int_for_mode.

>
>
>
>> diff --git a/gcc/varpool.c b/gcc/varpool.c
>> index 2eb1fc1..d9c08c1 100644
>> --- a/gcc/varpool.c
>> +++ b/gcc/varpool.c
>> @@ -254,6 +254,12 @@ ctor_for_folding (tree decl)
>>         && TREE_CODE (decl) != CONST_DECL)
>>       return error_mark_node;
>>
>> +  /* Static constant bounds are created to be
>> +     used instead of constants and therefore
>> +     do not let folding it.  */
>> +  if (POINTER_BOUNDS_P (decl))
>> +    return error_mark_node;
>
> Here's the part I'm struggling a bit with.    Why did you need this?
>
> Isn't this going to prevent that DECL from being used in folding?   The
> bounds shouldn't really affect that AFAICT.

Bounds constants were introduced only for initialization of constant
bound vars. Such vars are used to hold commonly used zero bounds (for
cases when bounds are unknown) values and null bounds (for null
pointers). Usage of such vars is optional and is controlled via
compiler flag. It is used to try to decrease overhead on bounds
creation. E.g. for MPX we need two instructions to create zero bounds
and also it require one GPR. One of these instructions does not become
nop when MPX is off which additionally increases overhead.  Having
constant var we can just load bounds using one MPX instruction.  And
if I do not prevent folding for these vars then all constant bounds
vars usages are replaced with immediate bounds constant usages and I
do not get desired effect.  Since there are no instructions working
with bounds immediates, I do not see reasons for folding.

Thanks,
Ilya

>
> jeff
Ilya Enkovich Nov. 18, 2013, 9:22 a.m. UTC | #3 
Ping

2013/11/7 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2013/11/7 Jeff Law <law@redhat.com>:
>> On 10/31/13 03:15, Ilya Enkovich wrote:
>>>
>>> Hi,
>>>
>>> Here is a patch which adds support for bound constant to be used as
>>> DECL_INITIAL for constant static bounds generated by compiler.
>>>
>>> Thanks,
>>> Ilya
>>> --
>>>
>>> gcc/
>>>
>>> 2013-10-23  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>         * emit-rtl.c (immed_double_const): Support MODE_POINTER_BOUNDS.
>>>         * explow.c (trunc_int_for_mode): Likewise.
>>>         * varpool.c (ctor_for_folding): Do not fold constant
>>>         bounds vars.
>>
>> I'm having a bit of trouble reconciling "add support for bound constant to
>> be used as DECL_INITIAL" rationale text and the actual patch.
>>
>> From reading the patch it appears that you want to allow generation of
>> immediate constants for objects with MODE_POINTER_BOUNDS.  OK, I can see how
>> that is useful.
>>
>> I can kindof see how you want to error out if someone asks for a constant to
>> be truncated to MODE_POINTER_BOUNDS.  Did this trip in practice or is it
>> preemptive?
>
> As far as I remember change in trunc_int_mode was required to expand
> bound constants on 32bit target. Size of the constant is equal to size
> of the HOST_WIDE_INT and thus constant generation goes through
> gen_int_mode and trunc_int_for_mode.
>
>>
>>
>>
>>> diff --git a/gcc/varpool.c b/gcc/varpool.c
>>> index 2eb1fc1..d9c08c1 100644
>>> --- a/gcc/varpool.c
>>> +++ b/gcc/varpool.c
>>> @@ -254,6 +254,12 @@ ctor_for_folding (tree decl)
>>>         && TREE_CODE (decl) != CONST_DECL)
>>>       return error_mark_node;
>>>
>>> +  /* Static constant bounds are created to be
>>> +     used instead of constants and therefore
>>> +     do not let folding it.  */
>>> +  if (POINTER_BOUNDS_P (decl))
>>> +    return error_mark_node;
>>
>> Here's the part I'm struggling a bit with.    Why did you need this?
>>
>> Isn't this going to prevent that DECL from being used in folding?   The
>> bounds shouldn't really affect that AFAICT.
>
> Bounds constants were introduced only for initialization of constant
> bound vars. Such vars are used to hold commonly used zero bounds (for
> cases when bounds are unknown) values and null bounds (for null
> pointers). Usage of such vars is optional and is controlled via
> compiler flag. It is used to try to decrease overhead on bounds
> creation. E.g. for MPX we need two instructions to create zero bounds
> and also it require one GPR. One of these instructions does not become
> nop when MPX is off which additionally increases overhead.  Having
> constant var we can just load bounds using one MPX instruction.  And
> if I do not prevent folding for these vars then all constant bounds
> vars usages are replaced with immediate bounds constant usages and I
> do not get desired effect.  Since there are no instructions working
> with bounds immediates, I do not see reasons for folding.
>
> Thanks,
> Ilya
>
>>
>> jeff
diff mbox

Patch

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index b0fc846..5d13b69 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -538,7 +538,8 @@  immed_double_const (HOST_WIDE_INT i0, HOST_WIDE_INT i1, enum machine_mode mode)
 		  || GET_MODE_CLASS (mode) == MODE_PARTIAL_INT
 		  /* We can get a 0 for an error mark.  */
 		  || GET_MODE_CLASS (mode) == MODE_VECTOR_INT
-		  || GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT);
+		  || GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT
+		  || GET_MODE_CLASS (mode) == MODE_POINTER_BOUNDS);
 
       if (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT)
 	return gen_int_mode (i0, mode);
diff --git a/gcc/explow.c b/gcc/explow.c
index f278e29..095434f 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -52,7 +52,8 @@  trunc_int_for_mode (HOST_WIDE_INT c, enum machine_mode mode)
   int width = GET_MODE_PRECISION (mode);
 
   /* You want to truncate to a _what_?  */
-  gcc_assert (SCALAR_INT_MODE_P (mode));
+  gcc_assert (SCALAR_INT_MODE_P (mode)
+	      || POINTER_BOUNDS_MODE_P (mode));
 
   /* Canonicalize BImode to 0 and STORE_FLAG_VALUE.  */
   if (mode == BImode)
diff --git a/gcc/varpool.c b/gcc/varpool.c
index 2eb1fc1..d9c08c1 100644
--- a/gcc/varpool.c
+++ b/gcc/varpool.c
@@ -254,6 +254,12 @@  ctor_for_folding (tree decl)
       && TREE_CODE (decl) != CONST_DECL)
     return error_mark_node;
 
+  /* Static constant bounds are created to be
+     used instead of constants and therefore
+     do not let folding it.  */
+  if (POINTER_BOUNDS_P (decl))
+    return error_mark_node;
+
   if (TREE_CODE (decl) == CONST_DECL
       || DECL_IN_CONSTANT_POOL (decl))
     return DECL_INITIAL (decl);