Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On Wed, Mar 28, 2018 at 08:31:38AM +0200, Martin Liška wrote: > Ok, so should we make the set of cum->bnds_in_bt based on > flag_check_pointer_bounds flag? > > If so, I've got patch that I've tested on my x86_64-linux-gnu machin. > > Martin > >From 7b5978e61305c5098a084c2352fcbacb4c347158 Mon Sep 17 00:00:00 2001 > From: marxin > Date: Wed, 21 Mar 2018 10:51:32 +0100 > Subject: [PATCH] Do not call chkp_type_bounds_count if MPX is not enabled (PR > target/84988). > > gcc/ChangeLog: > > 2018-03-21 Martin Liska > > PR target/84988 > * config/i386/i386.c (ix86_function_arg_advance): Do not call > chkp_type_bounds_count if MPX is not enabled. LGTM. > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -8618,7 +8618,8 @@ ix86_function_arg_advance (cumulative_args_t cum_v, > machine_mode mode, >if (cum->caller) > cfun->machine->outgoing_args_on_stack = true; > > - cum->bnds_in_bt = chkp_type_bounds_count (type); > + if (flag_check_pointer_bounds) > + cum->bnds_in_bt = chkp_type_bounds_count (type); > } > } > > -- > 2.16.2 > Jakub
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On 03/21/2018 01:44 PM, Jakub Jelinek wrote: On Wed, Mar 21, 2018 at 01:40:08PM +0100, Martin Liška wrote: 2018-03-21 Martin Liska PR target/84988 * config/i386/i386.c (ix86_function_arg_advance): Do not call chkp_type_bounds_count if MPX is not enabled. --- gcc/config/i386/i386.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 5b1e962dedb..0693f8fc451 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -8618,7 +8618,8 @@ ix86_function_arg_advance (cumulative_args_t cum_v, machine_mode mode, if (cum->caller) cfun->machine->outgoing_args_on_stack = true; - cum->bnds_in_bt = chkp_type_bounds_count (type); + if (type && POINTER_BOUNDS_TYPE_P (type)) + cum->bnds_in_bt = chkp_type_bounds_count (type); This is weird. POINTER_BOUNDS_TYPE_P (type) is TREE_CODE (type) == POINTER_BOUNDS_TYPE, and for POINTER_BOUNDS_TYPE chkp_type_bounds_count will just unconditionally return 0. Jakub Ok, so should we make the set of cum->bnds_in_bt based on flag_check_pointer_bounds flag? If so, I've got patch that I've tested on my x86_64-linux-gnu machin. Martin >From 7b5978e61305c5098a084c2352fcbacb4c347158 Mon Sep 17 00:00:00 2001 From: marxin Date: Wed, 21 Mar 2018 10:51:32 +0100 Subject: [PATCH] Do not call chkp_type_bounds_count if MPX is not enabled (PR target/84988). gcc/ChangeLog: 2018-03-21 Martin Liska PR target/84988 * config/i386/i386.c (ix86_function_arg_advance): Do not call chkp_type_bounds_count if MPX is not enabled. --- gcc/config/i386/i386.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index b4f6aec1434..2b2896f7ac6 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -8618,7 +8618,8 @@ ix86_function_arg_advance (cumulative_args_t cum_v, machine_mode mode, if (cum->caller) cfun->machine->outgoing_args_on_stack = true; - cum->bnds_in_bt = chkp_type_bounds_count (type); + if (flag_check_pointer_bounds) + cum->bnds_in_bt = chkp_type_bounds_count (type); } } -- 2.16.2
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On Wed, Mar 21, 2018 at 01:40:08PM +0100, Martin Liška wrote: > 2018-03-21 Martin Liska > > PR target/84988 > * config/i386/i386.c (ix86_function_arg_advance): Do not call > chkp_type_bounds_count if MPX is not enabled. > --- > gcc/config/i386/i386.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 5b1e962dedb..0693f8fc451 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -8618,7 +8618,8 @@ ix86_function_arg_advance (cumulative_args_t cum_v, > machine_mode mode, >if (cum->caller) > cfun->machine->outgoing_args_on_stack = true; > > - cum->bnds_in_bt = chkp_type_bounds_count (type); > + if (type && POINTER_BOUNDS_TYPE_P (type)) > + cum->bnds_in_bt = chkp_type_bounds_count (type); This is weird. POINTER_BOUNDS_TYPE_P (type) is TREE_CODE (type) == POINTER_BOUNDS_TYPE, and for POINTER_BOUNDS_TYPE chkp_type_bounds_count will just unconditionally return 0. Jakub
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On 03/21/2018 10:39 AM, Martin Liška wrote:
On 03/21/2018 10:36 AM, Richard Biener wrote:
On Tue, Mar 20, 2018 at 10:44 PM, Richard Sandiford
wrote:
Jeff Law writes:
On 03/20/2018 01:36 PM, Martin Liška wrote:
Hi.
This is a work-around to not iterate all members of array that can be huge.
As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want
to come
up with a new param for it.
Survives tests&bootstrap on x86_64-linux-gnu.
Martin
gcc/ChangeLog:
2018-03-20 Martin Liska
PR target/84988
* tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro.
(chkp_find_bound_slots_1): Limit number of iterations.
Or just CLOSE/WONTFIX :-)
I've got no objections here -- we want to minimize the effort put into
CHKP given its going to be deprecated.
The problem is that this affects normal configs, not just ones with
MPX enabled.
Indeed. It get's called via
#0 chkp_find_bound_slots_1 (type=0x769ee9d8, have_bound=0x2ed3868, offs=0)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1708
#1 0x01379a13 in chkp_find_bound_slots (type=0x769ee9d8,
res=0x2ed3868)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1754
#2 0x01377054 in chkp_type_bounds_count (type=0x769ee9d8)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1009
#3 0x016c664f in ix86_function_arg_advance (cum_v=...,
mode=E_BLKmode, type=0x769ee9d8, named=true)
at /space/rguenther/src/svn/early-lto-debug/gcc/config/i386/i386.c:8621
8616 {
8617 /* Track if there are outgoing arguments on stack. */
8618 if (cum->caller)
8619 cfun->machine->outgoing_args_on_stack = true;
8620
8621 cum->bnds_in_bt = chkp_type_bounds_count (type);
8622 }
8623 }
8624
8625 /* Define where to put the arguments to a function.
but I think we know POINTER_BOUNDS_TYPE_P etc. never return
true if -fcheck-pointer-* or -mmpx is not enabled, right? So we can
guard the above call appropriately and save some compile-time
for all of us?
Good observation, let me enhance the patch for the PR.
Martin
Richard.
Thanks,
Richard
Hi.
I've got this, that survives bootstrap®ression tests on x86_64-linux-gnu.
May I install it?
Thanks,
Martin
>From 15f95ac5117d9e2bab96c725716e5cb2832d3589 Mon Sep 17 00:00:00 2001
From: marxin
Date: Wed, 21 Mar 2018 10:51:32 +0100
Subject: [PATCH] Do not call chkp_type_bounds_count if MPX is not enabled (PR
target/84988).
gcc/ChangeLog:
2018-03-21 Martin Liska
PR target/84988
* config/i386/i386.c (ix86_function_arg_advance): Do not call
chkp_type_bounds_count if MPX is not enabled.
---
gcc/config/i386/i386.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 5b1e962dedb..0693f8fc451 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -8618,7 +8618,8 @@ ix86_function_arg_advance (cumulative_args_t cum_v, machine_mode mode,
if (cum->caller)
cfun->machine->outgoing_args_on_stack = true;
- cum->bnds_in_bt = chkp_type_bounds_count (type);
+ if (type && POINTER_BOUNDS_TYPE_P (type))
+ cum->bnds_in_bt = chkp_type_bounds_count (type);
}
}
--
2.16.2
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On 03/21/2018 10:36 AM, Richard Biener wrote:
On Tue, Mar 20, 2018 at 10:44 PM, Richard Sandiford
wrote:
Jeff Law writes:
On 03/20/2018 01:36 PM, Martin Liška wrote:
Hi.
This is a work-around to not iterate all members of array that can be huge.
As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want
to come
up with a new param for it.
Survives tests&bootstrap on x86_64-linux-gnu.
Martin
gcc/ChangeLog:
2018-03-20 Martin Liska
PR target/84988
* tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro.
(chkp_find_bound_slots_1): Limit number of iterations.
Or just CLOSE/WONTFIX :-)
I've got no objections here -- we want to minimize the effort put into
CHKP given its going to be deprecated.
The problem is that this affects normal configs, not just ones with
MPX enabled.
Indeed. It get's called via
#0 chkp_find_bound_slots_1 (type=0x769ee9d8, have_bound=0x2ed3868, offs=0)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1708
#1 0x01379a13 in chkp_find_bound_slots (type=0x769ee9d8,
res=0x2ed3868)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1754
#2 0x01377054 in chkp_type_bounds_count (type=0x769ee9d8)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1009
#3 0x016c664f in ix86_function_arg_advance (cum_v=...,
mode=E_BLKmode, type=0x769ee9d8, named=true)
at /space/rguenther/src/svn/early-lto-debug/gcc/config/i386/i386.c:8621
8616{
8617 /* Track if there are outgoing arguments on stack. */
8618 if (cum->caller)
8619cfun->machine->outgoing_args_on_stack = true;
8620
8621 cum->bnds_in_bt = chkp_type_bounds_count (type);
8622}
8623}
8624
8625/* Define where to put the arguments to a function.
but I think we know POINTER_BOUNDS_TYPE_P etc. never return
true if -fcheck-pointer-* or -mmpx is not enabled, right? So we can
guard the above call appropriately and save some compile-time
for all of us?
Good observation, let me enhance the patch for the PR.
Martin
Richard.
Thanks,
Richard
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On Tue, Mar 20, 2018 at 10:44 PM, Richard Sandiford
wrote:
> Jeff Law writes:
>> On 03/20/2018 01:36 PM, Martin Liška wrote:
>>> Hi.
>>>
>>> This is a work-around to not iterate all members of array that can be huge.
>>> As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want
>>> to come
>>> up with a new param for it.
>>>
>>> Survives tests&bootstrap on x86_64-linux-gnu.
>>>
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-03-20 Martin Liska
>>>
>>> PR target/84988
>>> * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro.
>>> (chkp_find_bound_slots_1): Limit number of iterations.
>> Or just CLOSE/WONTFIX :-)
>>
>> I've got no objections here -- we want to minimize the effort put into
>> CHKP given its going to be deprecated.
>
> The problem is that this affects normal configs, not just ones with
> MPX enabled.
Indeed. It get's called via
#0 chkp_find_bound_slots_1 (type=0x769ee9d8, have_bound=0x2ed3868, offs=0)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1708
#1 0x01379a13 in chkp_find_bound_slots (type=0x769ee9d8,
res=0x2ed3868)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1754
#2 0x01377054 in chkp_type_bounds_count (type=0x769ee9d8)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1009
#3 0x016c664f in ix86_function_arg_advance (cum_v=...,
mode=E_BLKmode, type=0x769ee9d8, named=true)
at /space/rguenther/src/svn/early-lto-debug/gcc/config/i386/i386.c:8621
8616{
8617 /* Track if there are outgoing arguments on stack. */
8618 if (cum->caller)
8619cfun->machine->outgoing_args_on_stack = true;
8620
8621 cum->bnds_in_bt = chkp_type_bounds_count (type);
8622}
8623}
8624
8625/* Define where to put the arguments to a function.
but I think we know POINTER_BOUNDS_TYPE_P etc. never return
true if -fcheck-pointer-* or -mmpx is not enabled, right? So we can
guard the above call appropriately and save some compile-time
for all of us?
Richard.
> Thanks,
> Richard
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
Jeff Law writes: > On 03/20/2018 01:36 PM, Martin Liška wrote: >> Hi. >> >> This is a work-around to not iterate all members of array that can be huge. >> As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want >> to come >> up with a new param for it. >> >> Survives tests&bootstrap on x86_64-linux-gnu. >> >> Martin >> >> gcc/ChangeLog: >> >> 2018-03-20 Martin Liska >> >> PR target/84988 >> * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro. >> (chkp_find_bound_slots_1): Limit number of iterations. > Or just CLOSE/WONTFIX :-) > > I've got no objections here -- we want to minimize the effort put into > CHKP given its going to be deprecated. The problem is that this affects normal configs, not just ones with MPX enabled. Thanks, Richard
Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
On 03/20/2018 01:36 PM, Martin Liška wrote: > Hi. > > This is a work-around to not iterate all members of array that can be huge. > As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want > to come > up with a new param for it. > > Survives tests&bootstrap on x86_64-linux-gnu. > > Martin > > gcc/ChangeLog: > > 2018-03-20 Martin Liska > > PR target/84988 > * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro. > (chkp_find_bound_slots_1): Limit number of iterations. Or just CLOSE/WONTFIX :-) I've got no objections here -- we want to minimize the effort put into CHKP given its going to be deprecated. jeff
[PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).
Hi. This is a work-around to not iterate all members of array that can be huge. As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want to come up with a new param for it. Survives tests&bootstrap on x86_64-linux-gnu. Martin gcc/ChangeLog: 2018-03-20 Martin Liska PR target/84988 * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro. (chkp_find_bound_slots_1): Limit number of iterations. --- gcc/tree-chkp.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index 40497ce94e7..d10e6c40423 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -1688,6 +1688,10 @@ chkp_find_bounds_for_elem (tree elem, tree *all_bounds, } } +/* Maximum number of elements to check in an array. */ + +#define CHKP_ARRAY_MAX_CHECK_STEPS4096 + /* Fill HAVE_BOUND output bitmap with information about bounds requred for object of type TYPE. @@ -1733,7 +1737,9 @@ chkp_find_bound_slots_1 (const_tree type, bitmap have_bound, || integer_minus_onep (maxval)) return; - for (cur = 0; cur <= TREE_INT_CST_LOW (maxval); cur++) + for (cur = 0; + cur <= MIN (CHKP_ARRAY_MAX_CHECK_STEPS, TREE_INT_CST_LOW (maxval)); + cur++) chkp_find_bound_slots_1 (etype, have_bound, offs + cur * esize); } }
