Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Sorry for missing this problem when committing Kugan's patch. I have just committed the attached patch, which I hope fixes all the spaces/indentation issues introduced. Thanks a lot! -- Eric Botcazou
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Mon, Sep 23, 2013 at 10:34 PM, Eric Botcazou ebotca...@adacore.com wrote: I have committed it for you (rev 202831), with a few modifications (ChangeLog formatting, typos). Here is what I have committed: 2013-09-23 Kugan Vivekanandarajah kug...@linaro.org * gimple-pretty-print.c (dump_ssaname_info): New function. (dump_gimple_phi): Call it. (pp_gimple_stmt_1): Likewise. * tree-core.h (tree_ssa_name): New union ssa_name_info_type field. (range_info_def): Declare. * tree-pretty-print.c (pp_double_int): New function. (dump_generic_node): Call it. * tree-pretty-print.h (pp_double_int): Declare. * tree-ssa-alias.c (dump_alias_info): Check pointer type. * tree-ssanames.h (range_info_def): New structure. (value_range_type): Move definition here. (set_range_info, value_range_type, duplicate_ssa_name_range_info): Declare. * tree-ssanames.c (make_ssa_name_fn): Check pointer type at initialization. (set_range_info): New function. (get_range_info): Likewise. (duplicate_ssa_name_range_info): Likewise. (duplicate_ssa_name_fn): Check pointer type and call duplicate_ssa_name_range_info. * tree-ssa-copy.c (fini_copy_prop): Likewise. * tree-vrp.c (value_range_type): Remove definition, now in tree-ssanames.h. (vrp_finalize): Call set_range_info to update value range of SSA_NAMEs. * tree.h (SSA_NAME_PTR_INFO): Macro changed to access via union. (SSA_NAME_RANGE_INFO): New macro. Nice patch, but the formatting is totally wrong wrt spaces, please reformat using 2-space indentation and 8-space TABs, as already used in the files. The patch has also introduced 2 regressions in Ada: === acats tests === FAIL: c37211b FAIL: c37211c === acats Summary === # of expected passes2318 # of unexpected failures2 Program received signal SIGSEGV, Segmentation fault. vrp_finalize () at /home/eric/svn/gcc/gcc/tree-vrp.c:9458 9458 if (POINTER_TYPE_P (TREE_TYPE (name)) (gdb) bt I'm testing a trivial patch to fix that. Richard. #0 vrp_finalize () at /home/eric/svn/gcc/gcc/tree-vrp.c:9458 #1 execute_vrp () at /home/eric/svn/gcc/gcc/tree-vrp.c:9583 #2 (anonymous namespace)::pass_vrp::execute (this=optimized out) at /home/eric/svn/gcc/gcc/tree-vrp.c:9673 #3 0x00c52c9a in execute_one_pass (pass=pass@entry=0x22e2210) at /home/eric/svn/gcc/gcc/passes.c:2201 #4 0x00c52e76 in execute_pass_list (pass=0x22e2210) at /home/eric/svn/gcc/gcc/passes.c:2253 #5 0x00c52e88 in execute_pass_list (pass=0x22e04d0) at /home/eric/svn/gcc/gcc/passes.c:2254 #6 0x009b9c49 in expand_function (node=0x76d12e40) at /home/eric/svn/gcc/gcc/cgraphunit.c:1750 #7 0x009bbc17 in expand_all_functions () at /home/eric/svn/gcc/gcc/cgraphunit.c:1855 #8 compile () at /home/eric/svn/gcc/gcc/cgraphunit.c:2192 #9 0x009bc1fa in finalize_compilation_unit () at /home/eric/svn/gcc/gcc/cgraphunit.c:2269 #10 0x006681b5 in gnat_write_global_declarations () at /home/eric/svn/gcc/gcc/ada/gcc-interface/utils.c:5630 #11 0x00d4577d in compile_file () at /home/eric/svn/gcc/gcc/toplev.c:560 #12 0x00d4750a in do_compile () at /home/eric/svn/gcc/gcc/toplev.c:1891 #13 toplev_main (argc=14, argv=0x7fffdca8) at /home/eric/svn/gcc/gcc/toplev.c:1967 #14 0x76f2a23d in __libc_start_main () from /lib64/libc.so.6 #15 0x00635381 in _start () at ../sysdeps/x86_64/elf/start.S:113 (gdb) p name $1 = (tree) 0x0 -- Eric Botcazou
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 24/09/13 19:23, Richard Biener wrote: On Mon, Sep 23, 2013 at 10:34 PM, Eric Botcazou ebotca...@adacore.com wrote: I have committed it for you (rev 202831), with a few modifications (ChangeLog formatting, typos). Here is what I have committed: 2013-09-23 Kugan Vivekanandarajah kug...@linaro.org * gimple-pretty-print.c (dump_ssaname_info): New function. (dump_gimple_phi): Call it. (pp_gimple_stmt_1): Likewise. * tree-core.h (tree_ssa_name): New union ssa_name_info_type field. (range_info_def): Declare. * tree-pretty-print.c (pp_double_int): New function. (dump_generic_node): Call it. * tree-pretty-print.h (pp_double_int): Declare. * tree-ssa-alias.c (dump_alias_info): Check pointer type. * tree-ssanames.h (range_info_def): New structure. (value_range_type): Move definition here. (set_range_info, value_range_type, duplicate_ssa_name_range_info): Declare. * tree-ssanames.c (make_ssa_name_fn): Check pointer type at initialization. (set_range_info): New function. (get_range_info): Likewise. (duplicate_ssa_name_range_info): Likewise. (duplicate_ssa_name_fn): Check pointer type and call duplicate_ssa_name_range_info. * tree-ssa-copy.c (fini_copy_prop): Likewise. * tree-vrp.c (value_range_type): Remove definition, now in tree-ssanames.h. (vrp_finalize): Call set_range_info to update value range of SSA_NAMEs. * tree.h (SSA_NAME_PTR_INFO): Macro changed to access via union. (SSA_NAME_RANGE_INFO): New macro. Nice patch, but the formatting is totally wrong wrt spaces, please reformat using 2-space indentation and 8-space TABs, as already used in the files. I am looking at everything and will send a patch to fix that. The patch has also introduced 2 regressions in Ada: === acats tests === FAIL: c37211b FAIL: c37211c === acats Summary === # of expected passes2318 # of unexpected failures2 I am sorry I missed this as I didnt test ada. I wrongly assumed that all the frontends are enabled by dafault. Program received signal SIGSEGV, Segmentation fault. vrp_finalize () at /home/eric/svn/gcc/gcc/tree-vrp.c:9458 9458 if (POINTER_TYPE_P (TREE_TYPE (name)) (gdb) bt I'm testing a trivial patch to fix that. I think the return value of ssa_name () (i.e. name) can be NULL and it has to be checked for NULL. In tree-vrp.c it is not checked in some other places related to debugging. In other places (eg. in tree-ssa-pre.c) there are checks . Thanks for looking into it and I will wait for your fix. Thanks, Kugan Richard. #0 vrp_finalize () at /home/eric/svn/gcc/gcc/tree-vrp.c:9458 #1 execute_vrp () at /home/eric/svn/gcc/gcc/tree-vrp.c:9583 #2 (anonymous namespace)::pass_vrp::execute (this=optimized out) at /home/eric/svn/gcc/gcc/tree-vrp.c:9673 #3 0x00c52c9a in execute_one_pass (pass=pass@entry=0x22e2210) at /home/eric/svn/gcc/gcc/passes.c:2201 #4 0x00c52e76 in execute_pass_list (pass=0x22e2210) at /home/eric/svn/gcc/gcc/passes.c:2253 #5 0x00c52e88 in execute_pass_list (pass=0x22e04d0) at /home/eric/svn/gcc/gcc/passes.c:2254 #6 0x009b9c49 in expand_function (node=0x76d12e40) at /home/eric/svn/gcc/gcc/cgraphunit.c:1750 #7 0x009bbc17 in expand_all_functions () at /home/eric/svn/gcc/gcc/cgraphunit.c:1855 #8 compile () at /home/eric/svn/gcc/gcc/cgraphunit.c:2192 #9 0x009bc1fa in finalize_compilation_unit () at /home/eric/svn/gcc/gcc/cgraphunit.c:2269 #10 0x006681b5 in gnat_write_global_declarations () at /home/eric/svn/gcc/gcc/ada/gcc-interface/utils.c:5630 #11 0x00d4577d in compile_file () at /home/eric/svn/gcc/gcc/toplev.c:560 #12 0x00d4750a in do_compile () at /home/eric/svn/gcc/gcc/toplev.c:1891 #13 toplev_main (argc=14, argv=0x7fffdca8) at /home/eric/svn/gcc/gcc/toplev.c:1967 #14 0x76f2a23d in __libc_start_main () from /lib64/libc.so.6 #15 0x00635381 in _start () at ../sysdeps/x86_64/elf/start.S:113 (gdb) p name $1 = (tree) 0x0 -- Eric Botcazou
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Tue, 24 Sep 2013, Richard Biener wrote:
On Mon, Sep 23, 2013 at 10:34 PM, Eric Botcazou ebotca...@adacore.com wrote:
I have committed it for you (rev 202831), with a few modifications
(ChangeLog formatting, typos).
Here is what I have committed:
2013-09-23 Kugan Vivekanandarajah kug...@linaro.org
* gimple-pretty-print.c (dump_ssaname_info): New function.
(dump_gimple_phi): Call it.
(pp_gimple_stmt_1): Likewise.
* tree-core.h (tree_ssa_name): New union ssa_name_info_type field.
(range_info_def): Declare.
* tree-pretty-print.c (pp_double_int): New function.
(dump_generic_node): Call it.
* tree-pretty-print.h (pp_double_int): Declare.
* tree-ssa-alias.c (dump_alias_info): Check pointer type.
* tree-ssanames.h (range_info_def): New structure.
(value_range_type): Move definition here.
(set_range_info, value_range_type, duplicate_ssa_name_range_info):
Declare.
* tree-ssanames.c (make_ssa_name_fn): Check pointer type at
initialization.
(set_range_info): New function.
(get_range_info): Likewise.
(duplicate_ssa_name_range_info): Likewise.
(duplicate_ssa_name_fn): Check pointer type and call
duplicate_ssa_name_range_info.
* tree-ssa-copy.c (fini_copy_prop): Likewise.
* tree-vrp.c (value_range_type): Remove definition, now in
tree-ssanames.h.
(vrp_finalize): Call set_range_info to update value range of
SSA_NAMEs.
* tree.h (SSA_NAME_PTR_INFO): Macro changed to access via union.
(SSA_NAME_RANGE_INFO): New macro.
Nice patch, but the formatting is totally wrong wrt spaces, please reformat
using 2-space indentation and 8-space TABs, as already used in the files.
The patch has also introduced 2 regressions in Ada:
=== acats tests ===
FAIL: c37211b
FAIL: c37211c
=== acats Summary ===
# of expected passes2318
# of unexpected failures2
Program received signal SIGSEGV, Segmentation fault.
vrp_finalize () at /home/eric/svn/gcc/gcc/tree-vrp.c:9458
9458 if (POINTER_TYPE_P (TREE_TYPE (name))
(gdb) bt
I'm testing a trivial patch to fix that.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
Richard.
2013-09-24 Richard Biener rguent...@suse.de
* tree-vrp.c (vrp_finalize): Check for SSA name presence.
Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c (revision 202860)
+++ gcc/tree-vrp.c (working copy)
@@ -9455,7 +9455,8 @@ vrp_finalize (void)
{
tree name = ssa_name (i);
- if (POINTER_TYPE_P (TREE_TYPE (name))
+ if (!name
+ || POINTER_TYPE_P (TREE_TYPE (name))
|| (vr_value[i]-type == VR_VARYING)
|| (vr_value[i]-type == VR_UNDEFINED))
continue;
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 23 September 2013 22:34, Eric Botcazou ebotca...@adacore.com wrote:
Nice patch, but the formatting is totally wrong wrt spaces, please reformat
using 2-space indentation and 8-space TABs, as already used in the files.
Sorry for missing this problem when committing Kugan's patch.
I have just committed the attached patch, which I hope fixes all the
spaces/indentation issues introduced.
Christophe.
2013-09-24 Christophe Lyon christophe.l...@linaro.org
* gimple-pretty-print.c: Various whitespace tweaks.
* tree-core.h: Likewise.
* tree-pretty-print.c: Likewise.
* tree-ssa-alias.c: Likewise.
* tree-ssa-copy.c: Likewise.
* tree-ssanames.c: Likewise.
* tree-ssanames.h: Likewise.
* tree-vrp.c: Likewise.
Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c (revision 202868)
+++ gcc/tree-vrp.c (revision 202869)
@@ -9451,48 +9451,48 @@ vrp_finalize (void)
/* Set value range to non pointer SSA_NAMEs. */
for (i = 0; i num_vr_values; i++)
- if (vr_value[i])
-{
- tree name = ssa_name (i);
+if (vr_value[i])
+ {
+ tree name = ssa_name (i);
if (!name
|| POINTER_TYPE_P (TREE_TYPE (name))
- || (vr_value[i]-type == VR_VARYING)
- || (vr_value[i]-type == VR_UNDEFINED))
-continue;
-
- if ((TREE_CODE (vr_value[i]-min) == INTEGER_CST)
- (TREE_CODE (vr_value[i]-max) == INTEGER_CST))
-{
- if (vr_value[i]-type == VR_RANGE)
-set_range_info (name,
-tree_to_double_int (vr_value[i]-min),
-tree_to_double_int (vr_value[i]-max));
- else if (vr_value[i]-type == VR_ANTI_RANGE)
-{
- /* VR_ANTI_RANGE ~[min, max] is encoded compactly as
- [max + 1, min - 1] without additional attributes.
- When min value max value, we know that it is
- VR_ANTI_RANGE; it is VR_RANGE otherwise. */
-
- /* ~[0,0] anti-range is represented as
- range. */
- if (TYPE_UNSIGNED (TREE_TYPE (name))
- integer_zerop (vr_value[i]-min)
- integer_zerop (vr_value[i]-max))
-set_range_info (name,
-double_int_one,
-double_int::max_value
-(TYPE_PRECISION (TREE_TYPE (name)), true));
- else
-set_range_info (name,
-tree_to_double_int (vr_value[i]-max)
-+ double_int_one,
-tree_to_double_int (vr_value[i]-min)
-- double_int_one);
-}
-}
-}
+ || (vr_value[i]-type == VR_VARYING)
+ || (vr_value[i]-type == VR_UNDEFINED))
+ continue;
+
+ if ((TREE_CODE (vr_value[i]-min) == INTEGER_CST)
+(TREE_CODE (vr_value[i]-max) == INTEGER_CST))
+ {
+ if (vr_value[i]-type == VR_RANGE)
+ set_range_info (name,
+ tree_to_double_int (vr_value[i]-min),
+ tree_to_double_int (vr_value[i]-max));
+ else if (vr_value[i]-type == VR_ANTI_RANGE)
+ {
+ /* VR_ANTI_RANGE ~[min, max] is encoded compactly as
+ [max + 1, min - 1] without additional attributes.
+ When min value max value, we know that it is
+ VR_ANTI_RANGE; it is VR_RANGE otherwise. */
+
+ /* ~[0,0] anti-range is represented as
+ range. */
+ if (TYPE_UNSIGNED (TREE_TYPE (name))
+integer_zerop (vr_value[i]-min)
+integer_zerop (vr_value[i]-max))
+ set_range_info (name,
+ double_int_one,
+ double_int::max_value
+ (TYPE_PRECISION (TREE_TYPE (name)), true));
+ else
+ set_range_info (name,
+ tree_to_double_int (vr_value[i]-max)
+ + double_int_one,
+ tree_to_double_int (vr_value[i]-min)
+ - double_int_one);
+ }
+ }
+ }
/* Free allocated memory. */
for (i = 0; i num_vr_values; i++)
Index: gcc/tree-pretty-print.c
===
--- gcc/tree-pretty-print.c (revision 202868)
+++ gcc/tree-pretty-print.c (revision 202869)
@@ -1063,8 +1063,8 @@ dump_generic_node (pretty_printer *buffe
pp_string (buffer, B); /* pseudo-unit */
}
else
-pp_double_int (buffer, tree_to_double_int (node),
- TYPE_UNSIGNED (TREE_TYPE (node)));
+
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Thu, Sep 19, 2013 at 7:07 AM, Kugan
kugan.vivekanandara...@linaro.org wrote:
Thanks Richard for the review.
On 18/09/13 18:55, Richard Biener wrote:
On Wed, 18 Sep 2013, Kugan wrote:
Thanks Richard for the review.
On 16/09/13 23:43, Richard Biener wrote:
On Mon, 16 Sep 2013, Kugan wrote:
[Snip]
+2013-09-17 Kugan Vivekanandarajah kug...@linaro.org
+
+ * gimple-pretty-print.c (dump_ssaname_info) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
ChangeLog should be formated
* gimple-pretty-print.c (dump_ssaname_info): New function.
(dump_gimple_phi): Call it.
(pp_gimple_stmt_1: Likewise.
* tree-pretty-print.c (dump_intger_cst_node): New function.
...
+pp_printf (buffer, # RANGE );
+pp_printf (buffer, %s[, range_type == VR_RANGE ? : ~);
+dump_intger_cst_node (buffer,
+ double_int_to_tree (TREE_TYPE (node),
min));
I was asking for a pp_double_int, not a dump_integer_cst_node function
as now you are creating a tree node in GC memory just to dump its
contents ... pp_double_int needs to be passed information on the
signedness of the value. It would roughly look like
Sorry, I understood it wrong.
pp_double_int (pretty_printer *pp, double_int d, bool uns)
{
if (d.fits_shwi ())
pp_wide_integer (pp, d.low);
else if (d.fits_uhwi ())
pp_unsigned_wide_integer (pp, d.low);
else
{
unsigned HOST_WIDE_INT low = d.low;
HOST_WIDE_INT high = d.high;
if (!uns d.is_negative ())
{
pp_minus (pp);
high = ~high + !low;
low = -low;
}
/* Would %x%0*x or %x%*0x get zero-padding on all
systems? */
sprintf (pp_buffer (pp)-digit_buffer,
HOST_WIDE_INT_PRINT_DOUBLE_HEX,
(unsigned HOST_WIDE_INT) high, low);
pp_string (pp, pp_buffer (pp)-digit_buffer);
}
}
and the INTEGER_CST case would use it like
if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE)
...
else
pp_double_int (buffer, tree_to_double_int (node),
TYPE_UNSIGNED (TREE_TYPE (node)));
+enum value_range_type
+get_range_info (tree name, double_int *min, double_int *max)
+{
ah, I see you have already made an appropriate change here.
+ /* Check for an empty range with minimum zero (of type
+ unsigned) that will wraparround. */
+ if (!(TYPE_UNSIGNED (TREE_TYPE (name))
+ integer_zerop (vr_value[i]-min)))
+set_range_info (name,
+tree_to_double_int (vr_value[i]-max)
++ double_int_one,
+tree_to_double_int (vr_value[i]-min)
+- double_int_one);
Yeah, I think ~[0,0] is the only anti-range that can be represented as
range that we keep. So maybe
if (TYPE_UNSIGNED (TREE_TYPE (name))
integer_zerop (vr_value[i]-min)
integer_zerop (vr_value[i]-max))
set_range_info (name,
double_int_one,
double_int::max_value
(TYPE_PRECISION (TREE_TYPE (name)),
true));
else
set_range_info (name,
+tree_to_double_int (vr_value[i]-max)
++ double_int_one,
+tree_to_double_int (vr_value[i]-min)
+- double_int_one);
to preserve ~[0,0] which looks like an important case when for example
looking at a divisor in a division.
Ok with those changes.
I have changed all of the above in the attached patch and ChangeLog. If this
is OK, could someone please commit it for me. I don’t have access to commit
it.
Bootstrapped and regtested on x86_64-unknown-linux-gnu and arm-none
linux-gnueabi.
Ok.
Thanks and sorry that it took so long,
Richard.
Thanks,
Kugan
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Kugan,
I have changed all of the above in the attached patch and ChangeLog. If this
is OK, could someone please commit it for me. I don’t have access to commit
it.
Bootstrapped and regtested on x86_64-unknown-linux-gnu and arm-none
linux-gnueabi.
Ok.
Thanks and sorry that it took so long,
Richard.
Thanks,
Kugan
I have committed it for you (rev 202831), with a few modifications
(ChangeLog formatting, typos).
Here is what I have committed:
2013-09-23 Kugan Vivekanandarajah kug...@linaro.org
* gimple-pretty-print.c (dump_ssaname_info): New function.
(dump_gimple_phi): Call it.
(pp_gimple_stmt_1): Likewise.
* tree-core.h (tree_ssa_name): New union ssa_name_info_type field.
(range_info_def): Declare.
* tree-pretty-print.c (pp_double_int): New function.
(dump_generic_node): Call it.
* tree-pretty-print.h (pp_double_int): Declare.
* tree-ssa-alias.c (dump_alias_info): Check pointer type.
* tree-ssanames.h (range_info_def): New structure.
(value_range_type): Move definition here.
(set_range_info, value_range_type, duplicate_ssa_name_range_info):
Declare.
* tree-ssanames.c (make_ssa_name_fn): Check pointer type at
initialization.
(set_range_info): New function.
(get_range_info): Likewise.
(duplicate_ssa_name_range_info): Likewise.
(duplicate_ssa_name_fn): Check pointer type and call
duplicate_ssa_name_range_info.
* tree-ssa-copy.c (fini_copy_prop): Likewise.
* tree-vrp.c (value_range_type): Remove definition, now in
tree-ssanames.h.
(vrp_finalize): Call set_range_info to update value range of
SSA_NAMEs.
* tree.h (SSA_NAME_PTR_INFO): Macro changed to access via union.
(SSA_NAME_RANGE_INFO): New macro.
Index: gcc/gimple-pretty-print.c
===
--- gcc/gimple-pretty-print.c (revision 202830)
+++ gcc/gimple-pretty-print.c (working copy)
@@ -1600,23 +1600,20 @@ dump_gimple_asm (pretty_printer *buffer,
}
}
-
-/* Dump a PHI node PHI. BUFFER, SPC and FLAGS are as in pp_gimple_stmt_1.
- The caller is responsible for calling pp_flush on BUFFER to finalize
- pretty printer. */
+/* Dump ptr_info and range_info for NODE on pretty_printer BUFFER with
+ SPC spaces of indent. */
static void
-dump_gimple_phi (pretty_printer *buffer, gimple phi, int spc, int flags)
+dump_ssaname_info (pretty_printer *buffer, tree node, int spc)
{
- size_t i;
- tree lhs = gimple_phi_result (phi);
+ if (TREE_CODE (node) != SSA_NAME)
+return;
- if (flags TDF_ALIAS
- POINTER_TYPE_P (TREE_TYPE (lhs))
- SSA_NAME_PTR_INFO (lhs))
+ if (POINTER_TYPE_P (TREE_TYPE (node))
+ SSA_NAME_PTR_INFO (node))
{
unsigned int align, misalign;
- struct ptr_info_def *pi = SSA_NAME_PTR_INFO (lhs);
+ struct ptr_info_def *pi = SSA_NAME_PTR_INFO (node);
pp_string (buffer, PT = );
pp_points_to_solution (buffer, pi-pt);
newline_and_indent (buffer, spc);
@@ -1628,6 +1625,41 @@ dump_gimple_phi (pretty_printer *buffer,
pp_string (buffer, # );
}
+ if (!POINTER_TYPE_P (TREE_TYPE (node))
+ SSA_NAME_RANGE_INFO (node))
+{
+ double_int min, max;
+ value_range_type range_type = get_range_info (node, min, max);
+
+ if (range_type == VR_VARYING)
+pp_printf (buffer, # RANGE VR_VARYING);
+ else if (range_type == VR_RANGE || range_type == VR_ANTI_RANGE)
+{
+ pp_printf (buffer, # RANGE );
+ pp_printf (buffer, %s[, range_type == VR_RANGE ? : ~);
+ pp_double_int (buffer, min, TYPE_UNSIGNED (TREE_TYPE (node)));
+ pp_printf (buffer, , );
+ pp_double_int (buffer, max, TYPE_UNSIGNED (TREE_TYPE (node)));
+ pp_printf (buffer, ]);
+ newline_and_indent (buffer, spc);
+}
+}
+}
+
+
+/* Dump a PHI node PHI. BUFFER, SPC and FLAGS are as in pp_gimple_stmt_1.
+ The caller is responsible for calling pp_flush on BUFFER to finalize
+ pretty printer. */
+
+static void
+dump_gimple_phi (pretty_printer *buffer, gimple phi, int spc, int flags)
+{
+ size_t i;
+ tree lhs = gimple_phi_result (phi);
+
+ if (flags TDF_ALIAS)
+dump_ssaname_info (buffer, lhs, spc);
+
if (flags TDF_RAW)
dump_gimple_fmt (buffer, spc, flags, %G %T, , phi,
gimple_phi_result (phi));
@@ -1908,27 +1940,9 @@ pp_gimple_stmt_1 (pretty_printer *buffer
gimple_has_mem_ops (gs))
dump_gimple_mem_ops (buffer, gs, spc, flags);
- if ((flags TDF_ALIAS)
- gimple_has_lhs (gs))
-{
- tree lhs = gimple_get_lhs (gs);
- if (TREE_CODE (lhs) == SSA_NAME
- POINTER_TYPE_P (TREE_TYPE (lhs))
- SSA_NAME_PTR_INFO (lhs))
- {
- unsigned int align, misalign;
- struct ptr_info_def *pi = SSA_NAME_PTR_INFO (lhs);
- pp_string (buffer, # PT = );
- pp_points_to_solution (buffer, pi-pt);
- newline_and_indent (buffer,
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
I have committed it for you (rev 202831), with a few modifications (ChangeLog formatting, typos). Here is what I have committed: 2013-09-23 Kugan Vivekanandarajah kug...@linaro.org * gimple-pretty-print.c (dump_ssaname_info): New function. (dump_gimple_phi): Call it. (pp_gimple_stmt_1): Likewise. * tree-core.h (tree_ssa_name): New union ssa_name_info_type field. (range_info_def): Declare. * tree-pretty-print.c (pp_double_int): New function. (dump_generic_node): Call it. * tree-pretty-print.h (pp_double_int): Declare. * tree-ssa-alias.c (dump_alias_info): Check pointer type. * tree-ssanames.h (range_info_def): New structure. (value_range_type): Move definition here. (set_range_info, value_range_type, duplicate_ssa_name_range_info): Declare. * tree-ssanames.c (make_ssa_name_fn): Check pointer type at initialization. (set_range_info): New function. (get_range_info): Likewise. (duplicate_ssa_name_range_info): Likewise. (duplicate_ssa_name_fn): Check pointer type and call duplicate_ssa_name_range_info. * tree-ssa-copy.c (fini_copy_prop): Likewise. * tree-vrp.c (value_range_type): Remove definition, now in tree-ssanames.h. (vrp_finalize): Call set_range_info to update value range of SSA_NAMEs. * tree.h (SSA_NAME_PTR_INFO): Macro changed to access via union. (SSA_NAME_RANGE_INFO): New macro. Nice patch, but the formatting is totally wrong wrt spaces, please reformat using 2-space indentation and 8-space TABs, as already used in the files. The patch has also introduced 2 regressions in Ada: === acats tests === FAIL: c37211b FAIL: c37211c === acats Summary === # of expected passes2318 # of unexpected failures2 Program received signal SIGSEGV, Segmentation fault. vrp_finalize () at /home/eric/svn/gcc/gcc/tree-vrp.c:9458 9458 if (POINTER_TYPE_P (TREE_TYPE (name)) (gdb) bt #0 vrp_finalize () at /home/eric/svn/gcc/gcc/tree-vrp.c:9458 #1 execute_vrp () at /home/eric/svn/gcc/gcc/tree-vrp.c:9583 #2 (anonymous namespace)::pass_vrp::execute (this=optimized out) at /home/eric/svn/gcc/gcc/tree-vrp.c:9673 #3 0x00c52c9a in execute_one_pass (pass=pass@entry=0x22e2210) at /home/eric/svn/gcc/gcc/passes.c:2201 #4 0x00c52e76 in execute_pass_list (pass=0x22e2210) at /home/eric/svn/gcc/gcc/passes.c:2253 #5 0x00c52e88 in execute_pass_list (pass=0x22e04d0) at /home/eric/svn/gcc/gcc/passes.c:2254 #6 0x009b9c49 in expand_function (node=0x76d12e40) at /home/eric/svn/gcc/gcc/cgraphunit.c:1750 #7 0x009bbc17 in expand_all_functions () at /home/eric/svn/gcc/gcc/cgraphunit.c:1855 #8 compile () at /home/eric/svn/gcc/gcc/cgraphunit.c:2192 #9 0x009bc1fa in finalize_compilation_unit () at /home/eric/svn/gcc/gcc/cgraphunit.c:2269 #10 0x006681b5 in gnat_write_global_declarations () at /home/eric/svn/gcc/gcc/ada/gcc-interface/utils.c:5630 #11 0x00d4577d in compile_file () at /home/eric/svn/gcc/gcc/toplev.c:560 #12 0x00d4750a in do_compile () at /home/eric/svn/gcc/gcc/toplev.c:1891 #13 toplev_main (argc=14, argv=0x7fffdca8) at /home/eric/svn/gcc/gcc/toplev.c:1967 #14 0x76f2a23d in __libc_start_main () from /lib64/libc.so.6 #15 0x00635381 in _start () at ../sysdeps/x86_64/elf/start.S:113 (gdb) p name $1 = (tree) 0x0 -- Eric Botcazou
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 16/09/13 15:13, Richard Biener wrote: +void +get_range_info (tree name, double_int min, double_int max, +enum value_range_type range_type) I'm not sure we want to use references. Well - first time. Personally, I don't think we should ever allow non-const references. Use of references means you can't tell from the source code that an argument to a function can be modified. It leads to confusion and makes things harder to debug. It can also lead to more object code bloat because more things end up being addressable. R.
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, 18 Sep 2013, Richard Earnshaw wrote: On 16/09/13 15:13, Richard Biener wrote: +void +get_range_info (tree name, double_int min, double_int max, +enum value_range_type range_type) I'm not sure we want to use references. Well - first time. Personally, I don't think we should ever allow non-const references. Use of references means you can't tell from the source code that an argument to a function can be modified. It leads to confusion and makes things harder to debug. That's a good argument, so please make get_range_info take pointers to min/max/range_type. Richard.
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, Sep 18, 2013 at 10:57:57AM +0200, Richard Biener wrote: On Wed, 18 Sep 2013, Richard Earnshaw wrote: On 16/09/13 15:13, Richard Biener wrote: +void +get_range_info (tree name, double_int min, double_int max, +enum value_range_type range_type) I'm not sure we want to use references. Well - first time. Personally, I don't think we should ever allow non-const references. Use of references means you can't tell from the source code that an argument to a function can be modified. It leads to confusion and makes things harder to debug. That's a good argument, so please make get_range_info take pointers to min/max/range_type. Or return range_type and just take min/max pointers? Jakub
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, Sep 18, 2013 at 10:57:57AM +0200, Richard Biener wrote: On Wed, 18 Sep 2013, Richard Earnshaw wrote: On 16/09/13 15:13, Richard Biener wrote: +void +get_range_info (tree name, double_int min, double_int max, +enum value_range_type range_type) I'm not sure we want to use references. Well - first time. Personally, I don't think we should ever allow non-const references. Use of references means you can't tell from the source code that an argument to a function can be modified. It leads to confusion and makes things harder to debug. I use non-const references in ipa-devirt code and speculative indirect call removal. I am not thrilled by them, just it seems common in C++ source bases. If we don't want them, i will remove these (they are used on two or three places, so it is easy) Honza That's a good argument, so please make get_range_info take pointers to min/max/range_type. Or return range_type and just take min/max pointers? Jakub
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, 18 Sep 2013, Kugan wrote:
Thanks Richard for the review.
On 16/09/13 23:43, Richard Biener wrote:
On Mon, 16 Sep 2013, Kugan wrote:
Hi,
Updated the patch to the latest changes in trunk that splits tree.h. I
also
noticed an error in printing double_int and fixed it.
Is this OK?
print_gimple_stmt (dump_file, stmt, 0,
-TDF_SLIM | (dump_flags TDF_LINENO));
+TDF_SLIM | TDF_RANGE | (dump_flags
TDF_LINENO));
this should be (dump_flags (TDF_LINENO|TDF_RANGE)) do not always
dump range info. I'd have simply re-used TDF_ALIAS (and interpret
it as SSA annotation info), adding -range in dump file modifiers
is ok with me.
+static void
+print_double_int (pretty_printer *buffer, double_int cst)
+{
+ tree node = double_int_to_tree (integer_type_node, cst);
+ if (TREE_INT_CST_HIGH (node) == 0)
+pp_printf (buffer, HOST_WIDE_INT_PRINT_UNSIGNED, TREE_INT_CST_LOW
(node));
+ else if (TREE_INT_CST_HIGH (node) == -1
+TREE_INT_CST_LOW (node) != 0)
+pp_printf (buffer, - HOST_WIDE_INT_PRINT_UNSIGNED,
+ -TREE_INT_CST_LOW (node));
+ else
+sprintf (pp_buffer (buffer)-digit_buffer,
+ HOST_WIDE_INT_PRINT_DOUBLE_HEX,
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_HIGH (node),
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_LOW (node));
using sprintf here looks like a layering violation to me. You
probably want to factor out code from the INTEGER_CST handling
of tree-pretty-print.c:dump_generic_node into a pp_double_int
function in pretty-print.[ch] instead.
@@ -1628,6 +1647,27 @@ dump_gimple_phi (pretty_printer *buffer, gimple
phi, int spc, int flags)
pp_string (buffer, # );
}
+ if ((flags TDF_RANGE)
+ !POINTER_TYPE_P (TREE_TYPE (lhs))
+ SSA_NAME_RANGE_INFO (lhs))
+{
+ double_int min, max;
+ value_range_type range_type;
I realize the scheme is pre-existing but can you try factoring
out the dumping of SSA_NAME_PTR_INFO / SSA_NAME_RANGE_INFO into
a separate routine that can be shared by dump_gimple_phi and
pp_gimple_stmt_1?
+get_range_info (tree name, double_int min, double_int max,
+enum value_range_type range_type)
+{
+ gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
+ gcc_assert (TREE_CODE (name) == SSA_NAME);
+ range_info_def *ri = SSA_NAME_RANGE_INFO (name);
the TREE_CODE (name) == SSA_NAME assert is redundant with the
tree-checking performed by SSA_NAME_RANGE_INFO. Likewise in
the other functions.
+void
+get_range_info (tree name, double_int min, double_int max,
+enum value_range_type range_type)
I'm not sure we want to use references. Well - first time.
+ /* If min max, it is VR_ANTI_RANGE. */
+ if (ri-min.scmp (ri-max) == 1)
+{
I think that's wrong and needs to be conditional on TYPE_UNSIGNED
of the SSA name.
+ else if (vr_value[i]-type == VR_ANTI_RANGE)
+{
+ /* VR_ANTI_RANGE ~[min, max] is encoded compactly as
+ [max + 1, min - 1] without additional attributes.
+ When min value max value, we know that it is
+ VR_ANTI_RANGE; it is VR_RANGE othewise. */
+ set_range_info (name,
+ tree_to_double_int (vr_value[i]-max)
+ + double_int_one,
+ tree_to_double_int (vr_value[i]-min)
+ - double_int_one);
there is a complication for when max + 1 or min - 1 overflow - those
should be non-canonical ranges I think, but double-check this
(check set_and_canonicalize_value_range).
I have now added a check for min == 0 for unsigned type. AFAIK, For double_int
type, this is the only case we should check.
I have also made the other changes you have asked me to do. Please find the
modified patch and ChangeLog.
Bootstrapped and regtested for x86_64-unknown-linux-gnu. Is this OK.
Thanks,
Kugan
+2013-09-17 Kugan Vivekanandarajah kug...@linaro.org
+
+ * gimple-pretty-print.c (dump_ssaname_info) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
ChangeLog should be formated
* gimple-pretty-print.c (dump_ssaname_info): New function.
(dump_gimple_phi): Call it.
(pp_gimple_stmt_1: Likewise.
* tree-pretty-print.c (dump_intger_cst_node): New function.
...
+pp_printf (buffer, # RANGE );
+pp_printf (buffer, %s[, range_type == VR_RANGE ? : ~);
+dump_intger_cst_node (buffer,
+ double_int_to_tree (TREE_TYPE (node),
min));
I was asking for a pp_double_int, not a dump_integer_cst_node function
as now you are creating a tree node in
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, 18 Sep 2013, Jan Hubicka wrote: On Wed, Sep 18, 2013 at 10:57:57AM +0200, Richard Biener wrote: On Wed, 18 Sep 2013, Richard Earnshaw wrote: On 16/09/13 15:13, Richard Biener wrote: +void +get_range_info (tree name, double_int min, double_int max, +enum value_range_type range_type) I'm not sure we want to use references. Well - first time. Personally, I don't think we should ever allow non-const references. Use of references means you can't tell from the source code that an argument to a function can be modified. It leads to confusion and makes things harder to debug. I use non-const references in ipa-devirt code and speculative indirect call removal. I am not thrilled by them, just it seems common in C++ source bases. If we don't want them, i will remove these (they are used on two or three places, so it is easy) Yeah, I think we should amend the coding conventions appropriately. Richard.
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Thanks Richard for the review.
On 18/09/13 18:55, Richard Biener wrote:
On Wed, 18 Sep 2013, Kugan wrote:
Thanks Richard for the review.
On 16/09/13 23:43, Richard Biener wrote:
On Mon, 16 Sep 2013, Kugan wrote:
[Snip]
+2013-09-17 Kugan Vivekanandarajah kug...@linaro.org
+
+ * gimple-pretty-print.c (dump_ssaname_info) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
ChangeLog should be formated
* gimple-pretty-print.c (dump_ssaname_info): New function.
(dump_gimple_phi): Call it.
(pp_gimple_stmt_1: Likewise.
* tree-pretty-print.c (dump_intger_cst_node): New function.
...
+pp_printf (buffer, # RANGE );
+pp_printf (buffer, %s[, range_type == VR_RANGE ? : ~);
+dump_intger_cst_node (buffer,
+ double_int_to_tree (TREE_TYPE (node),
min));
I was asking for a pp_double_int, not a dump_integer_cst_node function
as now you are creating a tree node in GC memory just to dump its
contents ... pp_double_int needs to be passed information on the
signedness of the value. It would roughly look like
Sorry, I understood it wrong.
pp_double_int (pretty_printer *pp, double_int d, bool uns)
{
if (d.fits_shwi ())
pp_wide_integer (pp, d.low);
else if (d.fits_uhwi ())
pp_unsigned_wide_integer (pp, d.low);
else
{
unsigned HOST_WIDE_INT low = d.low;
HOST_WIDE_INT high = d.high;
if (!uns d.is_negative ())
{
pp_minus (pp);
high = ~high + !low;
low = -low;
}
/* Would %x%0*x or %x%*0x get zero-padding on all
systems? */
sprintf (pp_buffer (pp)-digit_buffer,
HOST_WIDE_INT_PRINT_DOUBLE_HEX,
(unsigned HOST_WIDE_INT) high, low);
pp_string (pp, pp_buffer (pp)-digit_buffer);
}
}
and the INTEGER_CST case would use it like
if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE)
...
else
pp_double_int (buffer, tree_to_double_int (node),
TYPE_UNSIGNED (TREE_TYPE (node)));
+enum value_range_type
+get_range_info (tree name, double_int *min, double_int *max)
+{
ah, I see you have already made an appropriate change here.
+ /* Check for an empty range with minimum zero (of type
+ unsigned) that will wraparround. */
+ if (!(TYPE_UNSIGNED (TREE_TYPE (name))
+ integer_zerop (vr_value[i]-min)))
+set_range_info (name,
+tree_to_double_int (vr_value[i]-max)
++ double_int_one,
+tree_to_double_int (vr_value[i]-min)
+- double_int_one);
Yeah, I think ~[0,0] is the only anti-range that can be represented as
range that we keep. So maybe
if (TYPE_UNSIGNED (TREE_TYPE (name))
integer_zerop (vr_value[i]-min)
integer_zerop (vr_value[i]-max))
set_range_info (name,
double_int_one,
double_int::max_value
(TYPE_PRECISION (TREE_TYPE (name)), true));
else
set_range_info (name,
+tree_to_double_int (vr_value[i]-max)
++ double_int_one,
+tree_to_double_int (vr_value[i]-min)
+- double_int_one);
to preserve ~[0,0] which looks like an important case when for example
looking at a divisor in a division.
Ok with those changes.
I have changed all of the above in the attached patch and ChangeLog. If
this is OK, could someone please commit it for me. I don’t have access
to commit it.
Bootstrapped and regtested on x86_64-unknown-linux-gnu and arm-none
linux-gnueabi.
Thanks,
Kugan
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index ad70c24..6331636 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,24 @@
+2013-09-19 Kugan Vivekanandarajah kug...@linaro.org
+
+ * gimple-pretty-print.c (dump_ssaname_info) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Call dump_ssaname_info.
+ * (pp_gimple_stmt_1) : Likewise.
+ * tree-pretty-print.c (pp_double_int) : New function.
+ * (dump_generic_node) : Call pp_double_int.
+ * tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+ * tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+ initialize.
+ * (set_range_info) : New function.
+ * (get_range_info) : Likewise.
+ * (duplicate_ssa_name_range_info) : Likewise.
+ * (duplicate_ssa_name_fn) : Check pointer type and call
+ duplicate_ssa_name_range_info.
+ * tree-ssa-copy.c (fini_copy_prop) : Likewise.
+ * tree-vrp.c (vrp_finalize): Call set_range_info to update
+ value
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Thanks Richard for the review.
On 16/09/13 23:43, Richard Biener wrote:
On Mon, 16 Sep 2013, Kugan wrote:
Hi,
Updated the patch to the latest changes in trunk that splits tree.h. I also
noticed an error in printing double_int and fixed it.
Is this OK?
print_gimple_stmt (dump_file, stmt, 0,
-TDF_SLIM | (dump_flags TDF_LINENO));
+TDF_SLIM | TDF_RANGE | (dump_flags
TDF_LINENO));
this should be (dump_flags (TDF_LINENO|TDF_RANGE)) do not always
dump range info. I'd have simply re-used TDF_ALIAS (and interpret
it as SSA annotation info), adding -range in dump file modifiers
is ok with me.
+static void
+print_double_int (pretty_printer *buffer, double_int cst)
+{
+ tree node = double_int_to_tree (integer_type_node, cst);
+ if (TREE_INT_CST_HIGH (node) == 0)
+pp_printf (buffer, HOST_WIDE_INT_PRINT_UNSIGNED, TREE_INT_CST_LOW
(node));
+ else if (TREE_INT_CST_HIGH (node) == -1
+TREE_INT_CST_LOW (node) != 0)
+pp_printf (buffer, - HOST_WIDE_INT_PRINT_UNSIGNED,
+ -TREE_INT_CST_LOW (node));
+ else
+sprintf (pp_buffer (buffer)-digit_buffer,
+ HOST_WIDE_INT_PRINT_DOUBLE_HEX,
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_HIGH (node),
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_LOW (node));
using sprintf here looks like a layering violation to me. You
probably want to factor out code from the INTEGER_CST handling
of tree-pretty-print.c:dump_generic_node into a pp_double_int
function in pretty-print.[ch] instead.
@@ -1628,6 +1647,27 @@ dump_gimple_phi (pretty_printer *buffer, gimple
phi, int spc, int flags)
pp_string (buffer, # );
}
+ if ((flags TDF_RANGE)
+ !POINTER_TYPE_P (TREE_TYPE (lhs))
+ SSA_NAME_RANGE_INFO (lhs))
+{
+ double_int min, max;
+ value_range_type range_type;
I realize the scheme is pre-existing but can you try factoring
out the dumping of SSA_NAME_PTR_INFO / SSA_NAME_RANGE_INFO into
a separate routine that can be shared by dump_gimple_phi and
pp_gimple_stmt_1?
+get_range_info (tree name, double_int min, double_int max,
+enum value_range_type range_type)
+{
+ gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
+ gcc_assert (TREE_CODE (name) == SSA_NAME);
+ range_info_def *ri = SSA_NAME_RANGE_INFO (name);
the TREE_CODE (name) == SSA_NAME assert is redundant with the
tree-checking performed by SSA_NAME_RANGE_INFO. Likewise in
the other functions.
+void
+get_range_info (tree name, double_int min, double_int max,
+enum value_range_type range_type)
I'm not sure we want to use references. Well - first time.
+ /* If min max, it is VR_ANTI_RANGE. */
+ if (ri-min.scmp (ri-max) == 1)
+{
I think that's wrong and needs to be conditional on TYPE_UNSIGNED
of the SSA name.
+ else if (vr_value[i]-type == VR_ANTI_RANGE)
+{
+ /* VR_ANTI_RANGE ~[min, max] is encoded compactly as
+ [max + 1, min - 1] without additional attributes.
+ When min value max value, we know that it is
+ VR_ANTI_RANGE; it is VR_RANGE othewise. */
+ set_range_info (name,
+ tree_to_double_int (vr_value[i]-max)
+ + double_int_one,
+ tree_to_double_int (vr_value[i]-min)
+ - double_int_one);
there is a complication for when max + 1 or min - 1 overflow - those
should be non-canonical ranges I think, but double-check this
(check set_and_canonicalize_value_range).
I have now added a check for min == 0 for unsigned type. AFAIK, For
double_int type, this is the only case we should check.
I have also made the other changes you have asked me to do. Please find
the modified patch and ChangeLog.
Bootstrapped and regtested for x86_64-unknown-linux-gnu. Is this OK.
Thanks,
Kugan
+2013-09-17 Kugan Vivekanandarajah kug...@linaro.org
+
+ * gimple-pretty-print.c (dump_ssaname_info) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
+ * tree-pretty-print.c (dump_intger_cst_node) : New function.
+ * (dump_generic_node) : Call dump_intger_cst_node for INTEGER_CST.
+ * tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+ * tree-ssa-copy.c (fini_copy_prop) : Check pointer type and copy
+ range info.
+ * tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+ initialize.
+ * (set_range_info) : New function.
+ * (get_range_info) : Likewise.
+ * (duplicate_ssa_name_range_info) : Likewise.
+ * (duplicate_ssa_name_fn) : Check pointer type and call correct
+ duplicate function.
+ * tree-vrp.c (vrp_finalize): Call set_range_info to upddate
+ value range of SSA_NAMEs.
+ * tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+ * tree.h
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Mon, 16 Sep 2013, Kugan wrote:
Hi,
Updated the patch to the latest changes in trunk that splits tree.h. I also
noticed an error in printing double_int and fixed it.
Is this OK?
print_gimple_stmt (dump_file, stmt, 0,
-TDF_SLIM | (dump_flags TDF_LINENO));
+TDF_SLIM | TDF_RANGE | (dump_flags
TDF_LINENO));
this should be (dump_flags (TDF_LINENO|TDF_RANGE)) do not always
dump range info. I'd have simply re-used TDF_ALIAS (and interpret
it as SSA annotation info), adding -range in dump file modifiers
is ok with me.
+static void
+print_double_int (pretty_printer *buffer, double_int cst)
+{
+ tree node = double_int_to_tree (integer_type_node, cst);
+ if (TREE_INT_CST_HIGH (node) == 0)
+pp_printf (buffer, HOST_WIDE_INT_PRINT_UNSIGNED, TREE_INT_CST_LOW
(node));
+ else if (TREE_INT_CST_HIGH (node) == -1
+TREE_INT_CST_LOW (node) != 0)
+pp_printf (buffer, - HOST_WIDE_INT_PRINT_UNSIGNED,
+ -TREE_INT_CST_LOW (node));
+ else
+sprintf (pp_buffer (buffer)-digit_buffer,
+ HOST_WIDE_INT_PRINT_DOUBLE_HEX,
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_HIGH (node),
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_LOW (node));
using sprintf here looks like a layering violation to me. You
probably want to factor out code from the INTEGER_CST handling
of tree-pretty-print.c:dump_generic_node into a pp_double_int
function in pretty-print.[ch] instead.
@@ -1628,6 +1647,27 @@ dump_gimple_phi (pretty_printer *buffer, gimple
phi, int spc, int flags)
pp_string (buffer, # );
}
+ if ((flags TDF_RANGE)
+ !POINTER_TYPE_P (TREE_TYPE (lhs))
+ SSA_NAME_RANGE_INFO (lhs))
+{
+ double_int min, max;
+ value_range_type range_type;
I realize the scheme is pre-existing but can you try factoring
out the dumping of SSA_NAME_PTR_INFO / SSA_NAME_RANGE_INFO into
a separate routine that can be shared by dump_gimple_phi and
pp_gimple_stmt_1?
+get_range_info (tree name, double_int min, double_int max,
+enum value_range_type range_type)
+{
+ gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
+ gcc_assert (TREE_CODE (name) == SSA_NAME);
+ range_info_def *ri = SSA_NAME_RANGE_INFO (name);
the TREE_CODE (name) == SSA_NAME assert is redundant with the
tree-checking performed by SSA_NAME_RANGE_INFO. Likewise in
the other functions.
+void
+get_range_info (tree name, double_int min, double_int max,
+enum value_range_type range_type)
I'm not sure we want to use references. Well - first time.
+ /* If min max, it is VR_ANTI_RANGE. */
+ if (ri-min.scmp (ri-max) == 1)
+{
I think that's wrong and needs to be conditional on TYPE_UNSIGNED
of the SSA name.
+ else if (vr_value[i]-type == VR_ANTI_RANGE)
+{
+ /* VR_ANTI_RANGE ~[min, max] is encoded compactly as
+ [max + 1, min - 1] without additional attributes.
+ When min value max value, we know that it is
+ VR_ANTI_RANGE; it is VR_RANGE othewise. */
+ set_range_info (name,
+ tree_to_double_int (vr_value[i]-max)
+ + double_int_one,
+ tree_to_double_int (vr_value[i]-min)
+ - double_int_one);
there is a complication for when max + 1 or min - 1 overflow - those
should be non-canonical ranges I think, but double-check this
(check set_and_canonicalize_value_range).
+/* Type of value ranges. See value_range_d In tree-vrp.c for a
+ description of these types. */
+enum value_range_type { VR_UNDEFINED, VR_RANGE, VR_ANTI_RANGE, VR_VARYING
};
+
+/* Sets the value range to SSA. */
+void set_range_info (tree ssa, double_int min, double_int max);
+/* Gets the value range from SSA. */
+void get_range_info (tree name, double_int min, double_int max,
+ enum value_range_type range_type);
put these into tree-ssanames.h please, likewise struct GTY (())
range_info_def - this is where the ptr-info stuff went very recently.
Thanks,
Richard.
Thanks,
Kugan
+2013-09-12 Kugan Vivekanandarajah kug...@linaro.org
+
+ * cfgexpand.c (maybe_dump_rtl_for_gimple_stmt) : Add range to dump.
+ * gimple-pretty-print.c (print_double_int) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
+ * tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+ * tree-ssa-copy.c (fini_copy_prop) : Check pointer type and copy
+ range info.
+ * tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+ initialize.
+ * (set_range_info) : New function.
+ * (get_range_info) : Likewise.
+ * (duplicate_ssa_name_range_info) : Likewise.
+ * (duplicate_ssa_name_fn) : Check pointer type and call correct
+ duplicate function.
+ * tree-vrp.c (vrp_finalize): Call
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Hi,
Updated the patch to the latest changes in trunk that splits tree.h. I
also noticed an error in printing double_int and fixed it.
Is this OK?
Thanks,
Kugan
+2013-09-12 Kugan Vivekanandarajah kug...@linaro.org
+
+ * cfgexpand.c (maybe_dump_rtl_for_gimple_stmt) : Add range to dump.
+ * gimple-pretty-print.c (print_double_int) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
+ * tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+ * tree-ssa-copy.c (fini_copy_prop) : Check pointer type and copy
+ range info.
+ * tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+ initialize.
+ * (set_range_info) : New function.
+ * (get_range_info) : Likewise.
+ * (duplicate_ssa_name_range_info) : Likewise.
+ * (duplicate_ssa_name_fn) : Check pointer type and call correct
+ duplicate function.
+ * tree-vrp.c (vrp_finalize): Call set_range_info to upddate
+ value range of SSA_NAMEs.
+ * tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+ * tree.h (SSA_NAME_RANGE_INFO) : New macro
+
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 88e48c2..302188e 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1820,7 +1820,7 @@ maybe_dump_rtl_for_gimple_stmt (gimple stmt, rtx since)
{
fprintf (dump_file, \n;; );
print_gimple_stmt (dump_file, stmt, 0,
- TDF_SLIM | (dump_flags TDF_LINENO));
+ TDF_SLIM | TDF_RANGE | (dump_flags TDF_LINENO));
fprintf (dump_file, \n);
print_rtl (dump_file, since ? NEXT_INSN (since) : since);
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index ddc770a..8896d89 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -83,9 +83,10 @@ enum tree_dump_index
#define TDF_CSELIB (1 23) /* Dump cselib details. */
#define TDF_SCEV (1 24) /* Dump SCEV details. */
#define TDF_COMMENT (1 25) /* Dump lines with prefix ;; */
-#define MSG_OPTIMIZED_LOCATIONS (1 26) /* -fopt-info optimized sources */
-#define MSG_MISSED_OPTIMIZATION (1 27) /* missed opportunities */
-#define MSG_NOTE (1 28) /* general optimization info */
+#define TDF_RANGE (1 26) /* Dump range information. */
+#define MSG_OPTIMIZED_LOCATIONS (1 27) /* -fopt-info optimized sources */
+#define MSG_MISSED_OPTIMIZATION (1 28) /* missed opportunities */
+#define MSG_NOTE (1 29) /* general optimization info */
#define MSG_ALL (MSG_OPTIMIZED_LOCATIONS | MSG_MISSED_OPTIMIZATION \
| MSG_NOTE)
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 01a1ab5..6531010 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -1600,6 +1600,25 @@ dump_gimple_asm (pretty_printer *buffer, gimple gs, int spc, int flags)
}
}
+/* Dumps double_int CST to BUFFER. */
+
+static void
+print_double_int (pretty_printer *buffer, double_int cst)
+{
+ tree node = double_int_to_tree (integer_type_node, cst);
+ if (TREE_INT_CST_HIGH (node) == 0)
+pp_printf (buffer, HOST_WIDE_INT_PRINT_UNSIGNED, TREE_INT_CST_LOW (node));
+ else if (TREE_INT_CST_HIGH (node) == -1
+TREE_INT_CST_LOW (node) != 0)
+pp_printf (buffer, - HOST_WIDE_INT_PRINT_UNSIGNED,
+ -TREE_INT_CST_LOW (node));
+ else
+sprintf (pp_buffer (buffer)-digit_buffer,
+ HOST_WIDE_INT_PRINT_DOUBLE_HEX,
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_HIGH (node),
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_LOW (node));
+}
+
/* Dump a PHI node PHI. BUFFER, SPC and FLAGS are as in pp_gimple_stmt_1.
The caller is responsible for calling pp_flush on BUFFER to finalize
@@ -1628,6 +1647,27 @@ dump_gimple_phi (pretty_printer *buffer, gimple phi, int spc, int flags)
pp_string (buffer, # );
}
+ if ((flags TDF_RANGE)
+ !POINTER_TYPE_P (TREE_TYPE (lhs))
+ SSA_NAME_RANGE_INFO (lhs))
+{
+ double_int min, max;
+ value_range_type range_type;
+ get_range_info (lhs, min, max, range_type);
+ if (range_type == VR_VARYING)
+pp_printf (buffer, # RANGE VR_VARYING);
+ else if (range_type == VR_RANGE || range_type == VR_ANTI_RANGE)
+ {
+pp_printf (buffer, # RANGE );
+pp_printf (buffer, %s[, range_type == VR_RANGE ? : ~);
+print_double_int (buffer, min);
+pp_printf (buffer, , );
+print_double_int (buffer, max);
+pp_printf (buffer, ]);
+newline_and_indent (buffer, spc);
+ }
+}
+
if (flags TDF_RAW)
dump_gimple_fmt (buffer, spc, flags, %G %T, , phi,
gimple_phi_result (phi));
@@ -1930,6 +1970,32 @@ pp_gimple_stmt_1 (pretty_printer *buffer, gimple gs, int spc, int flags)
}
}
+ if ((flags TDF_RANGE)
+ gimple_has_lhs (gs))
+{
+ tree lhs = gimple_get_lhs (gs);
+ if ((TREE_CODE (lhs) == SSA_NAME)
+
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Here is the modified patch that addresses the comments form Richard and
Jakub.
This also includes:
1. Added TDF_RANGE to dump range_info
2. Moved enum value_range_type to tree.h (Is this the right place?)
Bootstrapped and regtested for x86_64-unknown-linux-gnu and arm-none
linux-gnueabi.
Is this Ok,
Thanks,
Kugan
+2013-09-12 Kugan Vivekanandarajah kug...@linaro.org
+
+ * cfgexpand.c (maybe_dump_rtl_for_gimple_stmt) : Add range to dump.
+ * gimple-pretty-print.c (print_double_int) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
+ * tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+ * tree-ssa-copy.c (fini_copy_prop) : Check pointer type and copy
+ range info.
+ * tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+ initialize.
+ * (set_range_info) : New function.
+ * (get_range_info) : Likewise.
+ * (duplicate_ssa_name_range_info) : Likewise.
+ * (duplicate_ssa_name_fn) : Check pointer type and call correct
+ duplicate function.
+ * tree-vrp.c (vrp_finalize): Call set_range_info to upddate
+ value range of SSA_NAMEs.
+ * tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+ * tree.h (SSA_NAME_RANGE_INFO) : New macro
+
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index a7d9170..f3fdd49 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1820,7 +1820,7 @@ maybe_dump_rtl_for_gimple_stmt (gimple stmt, rtx since)
{
fprintf (dump_file, \n;; );
print_gimple_stmt (dump_file, stmt, 0,
- TDF_SLIM | (dump_flags TDF_LINENO));
+ TDF_SLIM | TDF_RANGE | (dump_flags TDF_LINENO));
fprintf (dump_file, \n);
print_rtl (dump_file, since ? NEXT_INSN (since) : since);
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index 77f5de6..354dd92 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -82,9 +82,10 @@ enum tree_dump_index
#define TDF_CSELIB (1 23) /* Dump cselib details. */
#define TDF_SCEV (1 24) /* Dump SCEV details. */
#define TDF_COMMENT (1 25) /* Dump lines with prefix ;; */
-#define MSG_OPTIMIZED_LOCATIONS (1 26) /* -fopt-info optimized sources */
-#define MSG_MISSED_OPTIMIZATION (1 27) /* missed opportunities */
-#define MSG_NOTE (1 28) /* general optimization info */
+#define TDF_RANGE (1 26) /* Dump range information. */
+#define MSG_OPTIMIZED_LOCATIONS (1 27) /* -fopt-info optimized sources */
+#define MSG_MISSED_OPTIMIZATION (1 28) /* missed opportunities */
+#define MSG_NOTE (1 29) /* general optimization info */
#define MSG_ALL (MSG_OPTIMIZED_LOCATIONS | MSG_MISSED_OPTIMIZATION \
| MSG_NOTE)
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 1d40680..af1a13d 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -1581,6 +1581,24 @@ dump_gimple_asm (pretty_printer *buffer, gimple gs, int spc, int flags)
}
}
+/* Dumps double_int CST to BUFFER. */
+
+static void
+print_double_int (pretty_printer *buffer, double_int cst)
+{
+ tree node = double_int_to_tree (integer_type_node, cst);
+ if (TREE_INT_CST_HIGH (node) == 0)
+pp_printf (buffer, HOST_WIDE_INT_PRINT_UNSIGNED, TREE_INT_CST_LOW (node));
+ else if (TREE_INT_CST_HIGH (node) == -1
+TREE_INT_CST_LOW (node) != 0)
+pp_printf (buffer, - HOST_WIDE_INT_PRINT_UNSIGNED,
+ -TREE_INT_CST_LOW (node));
+ else
+pp_printf (buffer, 0x% HOST_LONG_FORMAT x% HOST_LONG_FORMAT x,
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_HIGH (node),
+ (unsigned HOST_WIDE_INT) TREE_INT_CST_LOW (node));
+}
+
/* Dump a PHI node PHI. BUFFER, SPC and FLAGS are as in pp_gimple_stmt_1.
The caller is responsible for calling pp_flush on BUFFER to finalize
@@ -1609,6 +1627,27 @@ dump_gimple_phi (pretty_printer *buffer, gimple phi, int spc, int flags)
pp_string (buffer, # );
}
+ if ((flags TDF_RANGE)
+ !POINTER_TYPE_P (TREE_TYPE (lhs))
+ SSA_NAME_RANGE_INFO (lhs))
+{
+ double_int min, max;
+ value_range_type range_type;
+ get_range_info (lhs, min, max, range_type);
+ if (range_type == VR_VARYING)
+pp_printf (buffer, # RANGE VR_VARYING);
+ else if (range_type == VR_RANGE || range_type == VR_ANTI_RANGE)
+ {
+pp_printf (buffer, # RANGE );
+pp_printf (buffer, %s[, range_type == VR_RANGE ? : ~);
+print_double_int (buffer, min);
+pp_printf (buffer, , );
+print_double_int (buffer, max);
+pp_printf (buffer, ]);
+newline_and_indent (buffer, spc);
+ }
+}
+
if (flags TDF_RAW)
dump_gimple_fmt (buffer, spc, flags, %G %T, , phi,
gimple_phi_result (phi));
@@ -1911,6 +1950,32 @@ pp_gimple_stmt_1 (pretty_printer *buffer, gimple gs, int spc, int flags)
}
}
+ if
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, 11 Sep 2013, Kugan wrote: Thanks Jakub for the review. On 10/09/13 23:10, Jakub Jelinek wrote: On Tue, Sep 10, 2013 at 03:17:50PM +0200, Richard Biener wrote: unsigned short s; s.1_3 = (short unsigned int) l.0_2; l.0_2: VARYING s.1_3: [0, +INF] Note that [0, +INF] is the same as VARYING and [-INF, +INF] and VARYING for l.0_2 is the same as [-INF, +INF]. Yeah, I don't see much value in differentiating between VR_VARYING and VR_RANGE [TYPE_MIN_VALUE, TYPE_MAX_VALUE] (perhaps a question is what to do for types with precisions different from TYPE_MODE's bitsize, if we should store for VARYING/UNDEFINED a range of all possible values in the mode). Unsigned type will be always = 0, even if it is VARYING or UNDEFINED. What is the valid bit good for? Is it meant just for integrals with 2*HOST_BITS_PER_WIDE_INT precision, which we can't represent in double_int? I'd say we just don't want to keep track on the value ranges for those. Ok, I will remove the valid. And, do we need to distinguish between VR_RANGE and VR_ANTI_RANGE? I mean, can't we always store the range in VR_RANGE format? Instead of -[3,7] we'd store [8,2] and define that if the min double_int is bigger than max double_int, then it is [min,+infinity] merged with [-infinity,max] range (i.e. -[max+1,min-1])? Ok, I will change this too. Make sure to add a predicate that can tell whether its an anti-range then. Richard.
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, Sep 11, 2013 at 11:02:30AM +0200, Richard Biener wrote:
Make sure to add a predicate that can tell whether its an anti-range
then.
Or perhaps extraction routine, that given an SSA_NAME will give you
a triplet,
{ enum value_range_type vr_type; double_int min, max; }
For non-integral SSA_NAMEs, or SSA_NAMEs with NULL RANGE_INFO
(should include integral types with 2 * HOST_BITS_PER_WIDE_INT
precision) will give you VR_VARYING, for the case where
min = max VR_RANGE and otherwise decode the max min range
into VR_ANTI_RANGE with adjusted min/max?
Then the min/max encoding of anti range would be just a more compact
way of encoding it to lower memory usage.
Jakub
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, 11 Sep 2013, Jakub Jelinek wrote:
On Wed, Sep 11, 2013 at 11:02:30AM +0200, Richard Biener wrote:
Make sure to add a predicate that can tell whether its an anti-range
then.
Or perhaps extraction routine, that given an SSA_NAME will give you
a triplet,
{ enum value_range_type vr_type; double_int min, max; }
For non-integral SSA_NAMEs, or SSA_NAMEs with NULL RANGE_INFO
(should include integral types with 2 * HOST_BITS_PER_WIDE_INT
precision) will give you VR_VARYING, for the case where
min = max VR_RANGE and otherwise decode the max min range
into VR_ANTI_RANGE with adjusted min/max?
Then the min/max encoding of anti range would be just a more compact
way of encoding it to lower memory usage.
Yeah, that also works.
Richard.
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Tue, 10 Sep 2013, Kugan wrote:
On 09/09/13 19:01, Richard Biener wrote:
On Mon, Sep 9, 2013 at 1:09 AM, Kugan kugan.vivekanandara...@linaro.org
wrote:
On 06/09/13 16:16, Richard Biener wrote:
On 9/3/13 2:15 PM, Kugan wrote:
Thanks Richard for reviewing.
On 02/09/13 22:15, Richard Biener wrote:
On Wed, Jul 3, 2013 at 2:25 PM, Kugan
kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for
GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else
return
+ false. */
+static bool
[snip]
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max),
vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but
noticed some
problems and that?s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is
expected to
have a value that will overflow (or varying) as this is what is
assigned to
a smaller variable. extract_range_from_assignment initially
calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be
assigned
from the rhs expression and not the value that we will have with
proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above.
And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
This is true. But just by looking at the value range of s.1.3 we will
only see [0 +INF], as we are transferring directly from the lattice to
lhs its value range.
[0, +INF] here tells us vrp_val_is_max and it is not
is_positive_overflow_infinity (or varying). Thats why we need to get
the
value range of RHS expression which will tell us the actual range. We
can then use this range and see of we can fit it to lhs type without
truncation.
Well, my point is you want to look at the l.0_2 value-range for this.
Storing the l.0_2 value-range for s.1_3 is wrong.
Yes, tree SSA_NAME should have it's correct value range. But, assigning
rhs
expression's value range is not totally wrong , it is just that it can be
conservative value range (please correct me if I am wrong here) in few
cases, as it can have wider range.
If it's a sign-changing conversion it can be surely wrong.
It is not sign-changing conversion. Rather, when we have rhs expression value
which is VR_VARYING it is set to [0, +INF]
i.e, in extract_range_from_assignment, if the value range is VR_VARYING,
follwing is done
if (vr-type == VR_VARYING)
extract_range_basic (vr, stmt);
In extract_range_basic (when the value range is varying), when the following
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Tue, Sep 10, 2013 at 03:17:50PM +0200, Richard Biener wrote:
unsigned short s;
s.1_3 = (short unsigned int) l.0_2;
l.0_2: VARYING
s.1_3: [0, +INF]
Note that [0, +INF] is the same as VARYING and [-INF, +INF] and VARYING for
l.0_2 is the same as [-INF, +INF].
Yeah, I don't see much value in differentiating between VR_VARYING and
VR_RANGE [TYPE_MIN_VALUE, TYPE_MAX_VALUE] (perhaps a question is what to do
for types with precisions different from TYPE_MODE's bitsize, if we should
store for VARYING/UNDEFINED a range of all possible values in the mode).
Unsigned type will be always = 0, even if it is VARYING or UNDEFINED.
What is the valid bit good for? Is it meant just for integrals with
2*HOST_BITS_PER_WIDE_INT precision, which we can't represent in double_int?
I'd say we just don't want to keep track on the value ranges for those.
And, do we need to distinguish between VR_RANGE and VR_ANTI_RANGE?
I mean, can't we always store the range in VR_RANGE format? Instead of
-[3,7] we'd store [8,2] and define that if the min double_int is bigger than
max double_int, then it is [min,+infinity] merged with [-infinity,max] range
(i.e. -[max+1,min-1])?
Micha just suggested
union vrp_info_type {
/* Pointer attributes used for alias analysis. */
struct GTY ((tag (0))) ptr_info_def *ptr_info;
/* Value range attributes used for zero/sign extension elimination.
*/
struct GTY ((tag (1))) range_info_def *range_info;
} GTY ((desc (%1.typed.type ? !POINTER_TYPE_P (TREE_TYPE
Why not TREE_TYPE(%1) here and why the (tree) cast?
Jakub
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 10/09/13 22:47, Richard Biener wrote:
On Tue, 10 Sep 2013, Kugan wrote:
On 09/09/13 19:01, Richard Biener wrote:
On Mon, Sep 9, 2013 at 1:09 AM, Kugan kugan.vivekanandara...@linaro.org
wrote:
On 06/09/13 16:16, Richard Biener wrote:
On 9/3/13 2:15 PM, Kugan wrote:
Thanks Richard for reviewing.
On 02/09/13 22:15, Richard Biener wrote:
On Wed, Jul 3, 2013 at 2:25 PM, Kugan
kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for
GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else
return
+ false. */
+static bool
[snip]
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max),
vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but
noticed some
problems and that?s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is
expected to
have a value that will overflow (or varying) as this is what is
assigned to
a smaller variable. extract_range_from_assignment initially
calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be
assigned
from the rhs expression and not the value that we will have with
proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above.
And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
This is true. But just by looking at the value range of s.1.3 we will
only see [0 +INF], as we are transferring directly from the lattice to
lhs its value range.
[0, +INF] here tells us vrp_val_is_max and it is not
is_positive_overflow_infinity (or varying). Thats why we need to get
the
value range of RHS expression which will tell us the actual range. We
can then use this range and see of we can fit it to lhs type without
truncation.
Well, my point is you want to look at the l.0_2 value-range for this.
Storing the l.0_2 value-range for s.1_3 is wrong.
Yes, tree SSA_NAME should have it's correct value range. But, assigning
rhs
expression's value range is not totally wrong , it is just that it can be
conservative value range (please correct me if I am wrong here) in few
cases, as it can have wider range.
If it's a sign-changing conversion it can be surely wrong.
It is not sign-changing conversion. Rather, when we have rhs expression value
which is VR_VARYING it is set to [0, +INF]
i.e, in extract_range_from_assignment, if the value range is VR_VARYING,
follwing is done
if (vr-type == VR_VARYING)
extract_range_basic (vr, stmt);
In extract_range_basic (when the value range is varying), when the following
code executes, it changes VR_VARYING to [0, +INF],
if (INTEGRAL_TYPE_P (type)
gimple_stmt_nonnegative_warnv_p (stmt, sop))
set_value_range_to_nonnegative (vr, type,
sop || stmt_overflow_infinity (stmt));
This will happen only when we have VR_VARYING for the rhs expression. This is
wrong from zero/sign extension elimination point of view as we cant rely on
this converted value range.
Currently I am leaving this as varying so that we can decide whether to
eliminate the zero/sign extension. This is not completely wrong.
unsigned short s;
s.1_3 = (short unsigned int) l.0_2;
l.0_2: VARYING
s.1_3: [0, +INF]
Note that [0, +INF] is the same as VARYING and [-INF, +INF] and VARYING for
l.0_2 is the same as [-INF, +INF].
I get it now. I will all the above as varying.
Similarly (extracted form a
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Mon, Sep 9, 2013 at 1:09 AM, Kugan kugan.vivekanandara...@linaro.org wrote:
On 06/09/13 16:16, Richard Biener wrote:
On 9/3/13 2:15 PM, Kugan wrote:
Thanks Richard for reviewing.
On 02/09/13 22:15, Richard Biener wrote:
On Wed, Jul 3, 2013 at 2:25 PM, Kugan
kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
[snip]
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max),
vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but
noticed some
problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is
expected to
have a value that will overflow (or varying) as this is what is
assigned to
a smaller variable. extract_range_from_assignment initially
calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be
assigned
from the rhs expression and not the value that we will have with proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above. And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
This is true. But just by looking at the value range of s.1.3 we will
only see [0 +INF], as we are transferring directly from the lattice to
lhs its value range.
[0, +INF] here tells us vrp_val_is_max and it is not
is_positive_overflow_infinity (or varying). Thats why we need to get the
value range of RHS expression which will tell us the actual range. We
can then use this range and see of we can fit it to lhs type without
truncation.
Well, my point is you want to look at the l.0_2 value-range for this.
Storing the l.0_2 value-range for s.1_3 is wrong.
Yes, tree SSA_NAME should have it's correct value range. But, assigning rhs
expression's value range is not totally wrong , it is just that it can be
conservative value range (please correct me if I am wrong here) in few
cases, as it can have wider range.
If it's a sign-changing conversion it can be surely wrong.
I can use the rhs value range in the above case. We can also eliminate
redundant zero/sign extensions for gimple binary and ternary stmts. In this
case we will have to calculate the value range. We will have to reuse these
logic in tree-vrp.
I fail to see the issue given no concrete example.
Other option is to add another attribute in range_info_t to indicate if
set_value_range_to_nonnegative is used in value range extraction.
Why should the result of this not be accurately represented in the lattice?
What is your preferred solution please.
I don't know because I do not understand the problem at hand.
I understand that the above code of mine needs to be changed but not
convinced about the best way to do that.
I can possibly re-factor extract_range_from_assignment to give me this
information with an additional argument. Could you kindly let me know
your
preference.
/* SSA name annotations. */
+ union vrp_info_type {
+/* Pointer attributes used for alias analysis. */
+struct GTY ((tag (TREE_SSA_PTR_INFO))) ptr_info_def *ptr_info;
+/* Value range attributes used for zero/sign extension
elimination.
*/
/* Value range information. */
+struct GTY ((tag (TREE_SSA_RANGE_INFO))) range_info_def
*range_info;
+ } GTY ((desc (%1.def_stmt
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 09/09/13 19:01, Richard Biener wrote:
On Mon, Sep 9, 2013 at 1:09 AM, Kugan kugan.vivekanandara...@linaro.org wrote:
On 06/09/13 16:16, Richard Biener wrote:
On 9/3/13 2:15 PM, Kugan wrote:
Thanks Richard for reviewing.
On 02/09/13 22:15, Richard Biener wrote:
On Wed, Jul 3, 2013 at 2:25 PM, Kugan
kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
[snip]
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max),
vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but
noticed some
problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is
expected to
have a value that will overflow (or varying) as this is what is
assigned to
a smaller variable. extract_range_from_assignment initially
calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be
assigned
from the rhs expression and not the value that we will have with proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above. And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
This is true. But just by looking at the value range of s.1.3 we will
only see [0 +INF], as we are transferring directly from the lattice to
lhs its value range.
[0, +INF] here tells us vrp_val_is_max and it is not
is_positive_overflow_infinity (or varying). Thats why we need to get the
value range of RHS expression which will tell us the actual range. We
can then use this range and see of we can fit it to lhs type without
truncation.
Well, my point is you want to look at the l.0_2 value-range for this.
Storing the l.0_2 value-range for s.1_3 is wrong.
Yes, tree SSA_NAME should have it's correct value range. But, assigning rhs
expression's value range is not totally wrong , it is just that it can be
conservative value range (please correct me if I am wrong here) in few
cases, as it can have wider range.
If it's a sign-changing conversion it can be surely wrong.
It is not sign-changing conversion. Rather, when we have rhs expression
value which is VR_VARYING it is set to [0, +INF]
i.e, in extract_range_from_assignment, if the value range is VR_VARYING,
follwing is done
if (vr-type == VR_VARYING)
extract_range_basic (vr, stmt);
In extract_range_basic (when the value range is varying), when the
following code executes, it changes VR_VARYING to [0, +INF],
if (INTEGRAL_TYPE_P (type)
gimple_stmt_nonnegative_warnv_p (stmt, sop))
set_value_range_to_nonnegative (vr, type,
sop || stmt_overflow_infinity (stmt));
This will happen only when we have VR_VARYING for the rhs expression.
This is wrong from zero/sign extension elimination point of view as we
cant rely on this converted value range.
Currently I am leaving this as varying so that we can decide whether to
eliminate the zero/sign extension. This is not completely wrong.
unsigned short s;
s.1_3 = (short unsigned int) l.0_2;
l.0_2: VARYING
s.1_3: [0, +INF]
Similarly (extracted form a testcase)
unsigned char _4;
unsigned char _2;
unsigned char _5;
_5 = _4 + _2;
value range extracted for expression (_4 + _2)
extract_range_from_binary_expr is VARYING and
_5 has value range [0 +INF] or [0, 255] after
set_value_range_to_nonnegative is done.
I
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 06/09/13 16:16, Richard Biener wrote:
On 9/3/13 2:15 PM, Kugan wrote:
Thanks Richard for reviewing.
On 02/09/13 22:15, Richard Biener wrote:
On Wed, Jul 3, 2013 at 2:25 PM, Kugan
kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
[snip]
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max),
vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but
noticed some
problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is
expected to
have a value that will overflow (or varying) as this is what is
assigned to
a smaller variable. extract_range_from_assignment initially
calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be assigned
from the rhs expression and not the value that we will have with proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above. And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
This is true. But just by looking at the value range of s.1.3 we will
only see [0 +INF], as we are transferring directly from the lattice to
lhs its value range.
[0, +INF] here tells us vrp_val_is_max and it is not
is_positive_overflow_infinity (or varying). Thats why we need to get the
value range of RHS expression which will tell us the actual range. We
can then use this range and see of we can fit it to lhs type without
truncation.
Well, my point is you want to look at the l.0_2 value-range for this.
Storing the l.0_2 value-range for s.1_3 is wrong.
Yes, tree SSA_NAME should have it's correct value range. But, assigning
rhs expression's value range is not totally wrong , it is just that it
can be conservative value range (please correct me if I am wrong here)
in few cases, as it can have wider range.
I can use the rhs value range in the above case. We can also eliminate
redundant zero/sign extensions for gimple binary and ternary stmts. In
this case we will have to calculate the value range. We will have to
reuse these logic in tree-vrp.
Other option is to add another attribute in range_info_t to indicate if
set_value_range_to_nonnegative is used in value range extraction.
What is your preferred solution please.
I understand that the above code of mine needs to be changed but not
convinced about the best way to do that.
I can possibly re-factor extract_range_from_assignment to give me this
information with an additional argument. Could you kindly let me know
your
preference.
/* SSA name annotations. */
+ union vrp_info_type {
+/* Pointer attributes used for alias analysis. */
+struct GTY ((tag (TREE_SSA_PTR_INFO))) ptr_info_def *ptr_info;
+/* Value range attributes used for zero/sign extension
elimination.
*/
/* Value range information. */
+struct GTY ((tag (TREE_SSA_RANGE_INFO))) range_info_def
*range_info;
+ } GTY ((desc (%1.def_stmt !POINTER_TYPE_P (TREE_TYPE
((tree)%1) vrp;
why do you need to test %1.def_stmt here?
I have seen some tree_ssa_name with def_stmt NULL. Thats why I added
this.
Is that something that should never happen.
It should never happen - they should have a GIMPLE_NOP.
I am seeing def_stmt of NULL for TREE_NOTHROW node.
debug_tree dumps the following in this case:
ssa_name 0x2bd89af8 nothrow var var_decl 0x2db384c0 tdef_stmt
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 9/3/13 2:15 PM, Kugan wrote:
Thanks Richard for reviewing.
On 02/09/13 22:15, Richard Biener wrote:
On Wed, Jul 3, 2013 at 2:25 PM, Kugan
kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
+extract_exp_value_range (gimple stmt, value_range_t *vr)
+{
+ gcc_assert (is_gimple_assign (stmt));
+ tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
...
@@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
*gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+
+ /* Set value range information for ssa. */
+ if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ !SSA_NAME_RANGE_INFO (lhs))
+{
+ value_range_t vr = VR_INITIALIZER;
...
+ if (extract_exp_value_range (stmt, vr))
+tree_ssa_set_value_range (lhs,
+ tree_to_double_int (vr.min),
+ tree_to_double_int (vr.max),
+ vr.type == VR_RANGE);
+}
This looks overly complicated to me. In vrp_finalize you can simply do
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max),
vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but
noticed some
problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is
expected to
have a value that will overflow (or varying) as this is what is
assigned to
a smaller variable. extract_range_from_assignment initially
calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be assigned
from the rhs expression and not the value that we will have with proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above. And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
This is true. But just by looking at the value range of s.1.3 we will
only see [0 +INF], as we are transferring directly from the lattice to
lhs its value range.
[0, +INF] here tells us vrp_val_is_max and it is not
is_positive_overflow_infinity (or varying). Thats why we need to get the
value range of RHS expression which will tell us the actual range. We
can then use this range and see of we can fit it to lhs type without
truncation.
Well, my point is you want to look at the l.0_2 value-range for this.
Storing the l.0_2 value-range for s.1_3 is wrong.
I understand that the above code of mine needs to be changed but not
convinced about the best way to do that.
I can possibly re-factor extract_range_from_assignment to give me this
information with an additional argument. Could you kindly let me know
your
preference.
/* SSA name annotations. */
+ union vrp_info_type {
+/* Pointer attributes used for alias analysis. */
+struct GTY ((tag (TREE_SSA_PTR_INFO))) ptr_info_def *ptr_info;
+/* Value range attributes used for zero/sign extension
elimination.
*/
/* Value range
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Thanks Richard for reviewing.
On 02/09/13 22:15, Richard Biener wrote:
On Wed, Jul 3, 2013 at 2:25 PM, Kugan kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
+extract_exp_value_range (gimple stmt, value_range_t *vr)
+{
+ gcc_assert (is_gimple_assign (stmt));
+ tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
...
@@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
*gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+
+ /* Set value range information for ssa. */
+ if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ !SSA_NAME_RANGE_INFO (lhs))
+{
+ value_range_t vr = VR_INITIALIZER;
...
+ if (extract_exp_value_range (stmt, vr))
+tree_ssa_set_value_range (lhs,
+ tree_to_double_int (vr.min),
+ tree_to_double_int (vr.max),
+ vr.type == VR_RANGE);
+}
This looks overly complicated to me. In vrp_finalize you can simply do
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max), vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but noticed some
problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is expected to
have a value that will overflow (or varying) as this is what is assigned to
a smaller variable. extract_range_from_assignment initially calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be assigned
from the rhs expression and not the value that we will have with proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above. And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
This is true. But just by looking at the value range of s.1.3 we will
only see [0 +INF], as we are transferring directly from the lattice to
lhs its value range.
[0, +INF] here tells us vrp_val_is_max and it is not
is_positive_overflow_infinity (or varying). Thats why we need to get the
value range of RHS expression which will tell us the actual range. We
can then use this range and see of we can fit it to lhs type without
truncation.
I understand that the above code of mine needs to be changed but not
convinced about the best way to do that.
I can possibly re-factor extract_range_from_assignment to give me this
information with an additional argument. Could you kindly let me know your
preference.
/* SSA name annotations. */
+ union vrp_info_type {
+/* Pointer attributes used for alias analysis. */
+struct GTY ((tag (TREE_SSA_PTR_INFO))) ptr_info_def *ptr_info;
+/* Value range attributes used for zero/sign extension elimination.
*/
/* Value range information. */
+struct GTY ((tag (TREE_SSA_RANGE_INFO))) range_info_def
*range_info;
+ } GTY ((desc (%1.def_stmt !POINTER_TYPE_P (TREE_TYPE
((tree)%1) vrp;
why do you need to test %1.def_stmt here?
I have seen some tree_ssa_name with def_stmt NULL. Thats why I
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
I'd like to ping this patch 1 of 2 that removes redundant zero/sign
extension using value range information.
Bootstrapped and no new regression for x86_64-unknown-linux-gnu and
arm-none-linux-gnueabi.
Thanks you for your time.
Kugan
n 14/08/13 16:49, Kugan wrote:
Hi Richard,
Here is an attempt to address your earlier review comments. Bootstrapped
and there is no new regression for X86_64 and arm. Thank you very much
for your time.
Thanks,
Kugan
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,25 @@
+2013-08-14 Kugan Vivekanandarajah kug...@linaro.org
+
+* tree-flow.h (mark_range_info_unknown): New function definition.
+* tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+* tree-ssa-copy.c (fini_copy_prop) : Check pointer type and copy
+range info.
+* tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+initialize.
+* (mark_range_info_unknown) : New function.
+* (duplicate_ssa_name_range_info) : Likewise.
+* (duplicate_ssa_name_fn) : Check pointer type and call correct
+duplicate function.
+* tree-vrp.c (extract_exp_value_range): New function.
+* (simplify_stmt_using_ranges): Call extract_exp_value_range and
+tree_ssa_set_value_range.
+* tree-ssaname.c (ssa_range_info): New function.
+* tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+* tree.h (SSA_NAME_RANGE_INFO) : New macro
+* gimple-pretty-print.c (print_double_int) : New function.
+* gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+* (pp_gimple_stmt_1) : Likewise.
+
2013-08-09 Jan Hubicka j...@suse.cz
* cgraph.c (cgraph_create_edge_1): Clear speculative flag.
On 03/07/13 21:55, Kugan wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
+extract_exp_value_range (gimple stmt, value_range_t *vr)
+{
+ gcc_assert (is_gimple_assign (stmt));
+ tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
...
@@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
*gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+
+ /* Set value range information for ssa. */
+ if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ !SSA_NAME_RANGE_INFO (lhs))
+{
+ value_range_t vr = VR_INITIALIZER;
...
+ if (extract_exp_value_range (stmt, vr))
+tree_ssa_set_value_range (lhs,
+ tree_to_double_int (vr.min),
+ tree_to_double_int (vr.max),
+ vr.type == VR_RANGE);
+}
This looks overly complicated to me. In vrp_finalize you can simply do
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max),
vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but noticed
some problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is expected
to have a value that will overflow (or varying) as this is what is
assigned to a smaller variable. extract_range_from_assignment initially
calculates the value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be assigned
from the rhs expression and not the value that we will have with proper
assignment.
I understand that
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Wed, Jul 3, 2013 at 2:25 PM, Kugan kugan.vivekanandara...@linaro.org wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
+extract_exp_value_range (gimple stmt, value_range_t *vr)
+{
+ gcc_assert (is_gimple_assign (stmt));
+ tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
...
@@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
*gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+
+ /* Set value range information for ssa. */
+ if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ !SSA_NAME_RANGE_INFO (lhs))
+{
+ value_range_t vr = VR_INITIALIZER;
...
+ if (extract_exp_value_range (stmt, vr))
+tree_ssa_set_value_range (lhs,
+ tree_to_double_int (vr.min),
+ tree_to_double_int (vr.max),
+ vr.type == VR_RANGE);
+}
This looks overly complicated to me. In vrp_finalize you can simply do
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max), vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but noticed some
problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is expected to
have a value that will overflow (or varying) as this is what is assigned to
a smaller variable. extract_range_from_assignment initially calculates the
value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be assigned
from the rhs expression and not the value that we will have with proper
assignment.
I don't understand this. The relevant statement is
s.1_3 = (short unsigned int) l.0_2;
right? You have value-ranges for both s.1_3 and l.0_2 as above. And
you clearly cannot optimize the truncation away (and if you could,
you wond't need value-range information for that fact).
I understand that the above code of mine needs to be changed but not
convinced about the best way to do that.
I can possibly re-factor extract_range_from_assignment to give me this
information with an additional argument. Could you kindly let me know your
preference.
/* SSA name annotations. */
+ union vrp_info_type {
+/* Pointer attributes used for alias analysis. */
+struct GTY ((tag (TREE_SSA_PTR_INFO))) ptr_info_def *ptr_info;
+/* Value range attributes used for zero/sign extension elimination.
*/
/* Value range information. */
+struct GTY ((tag (TREE_SSA_RANGE_INFO))) range_info_def
*range_info;
+ } GTY ((desc (%1.def_stmt !POINTER_TYPE_P (TREE_TYPE
((tree)%1) vrp;
why do you need to test %1.def_stmt here?
I have seen some tree_ssa_name with def_stmt NULL. Thats why I added this.
Is that something that should never happen.
It should never happen - they should have a GIMPLE_NOP.
+void
+set_range_info (tree name, double_int min,
+ double_int max, bool vr_range)
you have some whitespace issues here (please properly use tabs)
+ /* Allocate if not available. */
+ if (ri == NULL)
+{
+ ri = ggc_alloc_cleared_range_info_def ();
+ mark_range_info_unknown (ri);
that
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Hi Richard,
Here is an attempt to address your earlier review comments. Bootstrapped
and there is no new regression for X86_64 and arm. Thank you very much
for your time.
Thanks,
Kugan
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,25 @@
+2013-08-14 Kugan Vivekanandarajah kug...@linaro.org
+
+ * tree-flow.h (mark_range_info_unknown): New function definition.
+ * tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+ * tree-ssa-copy.c (fini_copy_prop) : Check pointer type and copy
+ range info.
+ * tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+ initialize.
+ * (mark_range_info_unknown) : New function.
+ * (duplicate_ssa_name_range_info) : Likewise.
+ * (duplicate_ssa_name_fn) : Check pointer type and call correct
+ duplicate function.
+ * tree-vrp.c (extract_exp_value_range): New function.
+ * (simplify_stmt_using_ranges): Call extract_exp_value_range and
+ tree_ssa_set_value_range.
+ * tree-ssaname.c (ssa_range_info): New function.
+ * tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+ * tree.h (SSA_NAME_RANGE_INFO) : New macro
+ * gimple-pretty-print.c (print_double_int) : New function.
+ * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+ * (pp_gimple_stmt_1) : Likewise.
+
2013-08-09 Jan Hubicka j...@suse.cz
* cgraph.c (cgraph_create_edge_1): Clear speculative flag.
On 03/07/13 21:55, Kugan wrote:
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN
stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
+extract_exp_value_range (gimple stmt, value_range_t *vr)
+{
+ gcc_assert (is_gimple_assign (stmt));
+ tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
...
@@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
*gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+
+ /* Set value range information for ssa. */
+ if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ !SSA_NAME_RANGE_INFO (lhs))
+{
+ value_range_t vr = VR_INITIALIZER;
...
+ if (extract_exp_value_range (stmt, vr))
+tree_ssa_set_value_range (lhs,
+ tree_to_double_int (vr.min),
+ tree_to_double_int (vr.max),
+ vr.type == VR_RANGE);
+}
This looks overly complicated to me. In vrp_finalize you can simply do
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max), vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but noticed
some problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is expected
to have a value that will overflow (or varying) as this is what is
assigned to a smaller variable. extract_range_from_assignment initially
calculates the value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be assigned
from the rhs expression and not the value that we will have with proper
assignment.
I understand that the above code of mine needs to be changed but not
convinced about the best way to do that.
I can possibly re-factor extract_range_from_assignment to give me this
information with an additional argument. Could
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On 17/06/13 18:33, Richard Biener wrote:
On Mon, 17 Jun 2013, Kugan wrote:
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
+extract_exp_value_range (gimple stmt, value_range_t *vr)
+{
+ gcc_assert (is_gimple_assign (stmt));
+ tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
...
@@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
*gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+
+ /* Set value range information for ssa. */
+ if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ !SSA_NAME_RANGE_INFO (lhs))
+{
+ value_range_t vr = VR_INITIALIZER;
...
+ if (extract_exp_value_range (stmt, vr))
+tree_ssa_set_value_range (lhs,
+ tree_to_double_int (vr.min),
+ tree_to_double_int (vr.max),
+ vr.type == VR_RANGE);
+}
This looks overly complicated to me. In vrp_finalize you can simply do
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max), vr_value[i].type
== VR_RANGE);
}
Thanks Richard for taking time to review it.
I was doing something like what you are suggesting earlier but noticed
some problems and that’s the reason why I changed.
For example, for the following testcase from the test suite,
unsigned long l = (unsigned long)-2;
unsigned short s;
int main () {
long t = l + 1;
s = l;
if (s != (unsigned short) -2)
abort ();
exit (0);
}
with the following gimple stmts
main ()
{
short unsigned int s.1;
long unsigned int l.0;
;; basic block 2, loop depth 0
;;pred: ENTRY
l.0_2 = l;
s.1_3 = (short unsigned int) l.0_2;
s = s.1_3;
if (s.1_3 != 65534)
goto bb 3;
else
goto bb 4;
;;succ: 3
;;4
;; basic block 3, loop depth 0
;;pred: 2
abort ();
;;succ:
;; basic block 4, loop depth 0
;;pred: 2
exit (0);
;;succ:
}
has the following value range.
l.0_2: VARYING
s.1_3: [0, +INF]
From zero/sign extension point of view, the variable s.1_3 is expected
to have a value that will overflow (or varying) as this is what is
assigned to a smaller variable. extract_range_from_assignment initially
calculates the value range as VARYING but later changed to [0, +INF] by
extract_range_basic. What I need here is the value that will be assigned
from the rhs expression and not the value that we will have with proper
assignment.
I understand that the above code of mine needs to be changed but not
convinced about the best way to do that.
I can possibly re-factor extract_range_from_assignment to give me this
information with an additional argument. Could you kindly let me know
your preference.
/* SSA name annotations. */
+ union vrp_info_type {
+/* Pointer attributes used for alias analysis. */
+struct GTY ((tag (TREE_SSA_PTR_INFO))) ptr_info_def *ptr_info;
+/* Value range attributes used for zero/sign extension elimination.
*/
/* Value range information. */
+struct GTY ((tag (TREE_SSA_RANGE_INFO))) range_info_def
*range_info;
+ } GTY ((desc (%1.def_stmt !POINTER_TYPE_P (TREE_TYPE
((tree)%1) vrp;
why do you need to test %1.def_stmt here?
I have seen some tree_ssa_name with def_stmt NULL. Thats why I added
this. Is that something that should never happen.
Thanks,
Kugan
Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
On Mon, 17 Jun 2013, Kugan wrote:
Can you please help to review this patch? Richard reviewed the original patch
and asked it to be split into two parts. Also, he wanted a review from RTL
maintainer for the RTL changes.
Thanks,
Kugan
On 03/06/13 11:43, Kugan wrote:
Hi,
This patch adds value range information to tree SSA_NAME during Value
Range Propagation (VRP) pass in preparation to removes some of the
redundant sign/zero extensions during RTL expansion.
This is based on the original patch posted in
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00610.html and addresses
the review comments of Richard Biener.
Tested on X86_64 and ARM.
I would like review comments on this.
Thanks,
Kugan
+2013-06-03 Kugan Vivekanandarajah kug...@linaro.org
+
+* gcc/gcc/tree-flow.h: Declared structure range_info_def and function
+definition for mark_range_info_unknown.
+* gcc/tree-ssa-alias.c (dump_alias_info) : Check pointer type
+* gcc/tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+initialize.
+* (mark_range_info_unknown) : New function.
+* (duplicate_ssa_name_range_info) : Likewise.
+* (duplicate_ssa_name_fn) : Check pointer type and call correct
+duplicate function.
+* gcc/tree-vrp.c (extract_exp_value_range): New function.
+* (simplify_stmt_using_ranges): Call extract_exp_value_range and
+tree_ssa_set_value_range.
+* gcc/tree.c (tree_ssa_set_value_range): New function.
+* gcc/tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+* gcc/tree.h (SSA_NAME_RANGE_INFO) : New macro
These go to gcc/ChangeLog and thus do not need the gcc/ prefix.
+/* Value range information for SSA_NAMEs representing non-pointer
variables. */
+
+struct GTY (()) range_info_def {
+ /* Set to true if VR_RANGE and false if VR_ANTI_RANGE. */
+ bool vr_range;
+ /* Minmum for value range. */
+ double_int min;
+ /* Maximum for value range. */
+ double_int max;
+ /* Set to true if range is valid. */
+ bool valid;
+};
this way you waste a lot of padding, so please move the two bool
members next to each other.
+/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN stmt.
+ If the extracted value range is valid, return true else return
+ false. */
+static bool
+extract_exp_value_range (gimple stmt, value_range_t *vr)
+{
+ gcc_assert (is_gimple_assign (stmt));
+ tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+ enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
...
@@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
*gsi)
{
enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
tree rhs1 = gimple_assign_rhs1 (stmt);
+ tree lhs = gimple_assign_lhs (stmt);
+
+ /* Set value range information for ssa. */
+ if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
+ INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ !SSA_NAME_RANGE_INFO (lhs))
+{
+ value_range_t vr = VR_INITIALIZER;
...
+ if (extract_exp_value_range (stmt, vr))
+tree_ssa_set_value_range (lhs,
+ tree_to_double_int (vr.min),
+ tree_to_double_int (vr.max),
+ vr.type == VR_RANGE);
+}
This looks overly complicated to me. In vrp_finalize you can simply do
for (i = 0; i num_vr_values; i++)
if (vr_value[i])
{
tree name = ssa_name (i);
if (POINTER_TYPE_P (name))
continue;
if (vr_value[i].type == VR_RANGE
|| vr_value[i].type == VR_ANTI_RANGE)
tree_ssa_set_value_range (name, tree_to_double_int
(vr_value[i].min), tree_to_double_int (vr_value[i].max), vr_value[i].type
== VR_RANGE);
}
that is, transfer directly from the lattice.
+/* Set zero/sign extension redundant for ssa def stmt. */
+
+void
+tree_ssa_set_value_range (tree ssa, double_int min,
+ double_int max, bool vr_range)
+{
The comment needs updating. Also this doesn't belong in tree.c
but in tree-ssanames.c with a more suitable name like
set_range_info ().
+/* The garbage collector needs to know the interpretation of the
+ value range info in the tree_ssa_name. */
+#define TREE_SSA_PTR_INFO (0)
+#define TREE_SSA_RANGE_INFO (1)
no need for those, just use GTY ((tag (0)) and 1.
+ /* Value range information. */
/* SSA name annotations. */
+ union vrp_info_type {
+/* Pointer attributes used for alias analysis. */
+struct GTY ((tag (TREE_SSA_PTR_INFO))) ptr_info_def *ptr_info;
+/* Value range attributes used for zero/sign extension elimination.
*/
/* Value range information. */
+struct GTY ((tag (TREE_SSA_RANGE_INFO))) range_info_def
*range_info;
+ } GTY ((desc (%1.def_stmt
[ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL
Can you please help to review this patch? Richard reviewed the original
patch and asked it to be split into two parts. Also, he wanted a review
from RTL maintainer for the RTL changes.
Thanks,
Kugan
On 03/06/13 11:43, Kugan wrote:
Hi,
This patch adds value range information to tree SSA_NAME during Value
Range Propagation (VRP) pass in preparation to removes some of the
redundant sign/zero extensions during RTL expansion.
This is based on the original patch posted in
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00610.html and addresses
the review comments of Richard Biener.
Tested on X86_64 and ARM.
I would like review comments on this.
Thanks,
Kugan
+2013-06-03 Kugan Vivekanandarajah kug...@linaro.org
+
+* gcc/gcc/tree-flow.h: Declared structure range_info_def and function
+definition for mark_range_info_unknown.
+* gcc/tree-ssa-alias.c (dump_alias_info) : Check pointer type
+* gcc/tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+initialize.
+* (mark_range_info_unknown) : New function.
+* (duplicate_ssa_name_range_info) : Likewise.
+* (duplicate_ssa_name_fn) : Check pointer type and call correct
+duplicate function.
+* gcc/tree-vrp.c (extract_exp_value_range): New function.
+* (simplify_stmt_using_ranges): Call extract_exp_value_range and
+tree_ssa_set_value_range.
+* gcc/tree.c (tree_ssa_set_value_range): New function.
+* gcc/tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+* gcc/tree.h (SSA_NAME_RANGE_INFO) : New macro
+
diff --git a/gcc/tree-flow.h b/gcc/tree-flow.h
index 24fcfbf..dd4e2f5 100644
--- a/gcc/tree-flow.h
+++ b/gcc/tree-flow.h
@@ -147,6 +147,19 @@ struct GTY(()) ptr_info_def
unsigned int misalign;
};
+/* Value range information for SSA_NAMEs representing non-pointer variables. */
+
+struct GTY (()) range_info_def {
+ /* Set to true if VR_RANGE and false if VR_ANTI_RANGE. */
+ bool vr_range;
+ /* Minmum for value range. */
+ double_int min;
+ /* Maximum for value range. */
+ double_int max;
+ /* Set to true if range is valid. */
+ bool valid;
+};
+
/* It is advantageous to avoid things like life analysis for variables which
do not need PHI nodes. This enum describes whether or not a particular
@@ -532,6 +545,7 @@ extern void replace_ssa_name_symbol (tree, tree);
extern bool get_ptr_info_alignment (struct ptr_info_def *, unsigned int *,
unsigned int *);
extern void mark_ptr_info_alignment_unknown (struct ptr_info_def *);
+extern void mark_range_info_unknown (struct range_info_def *);
extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int,
unsigned int);
extern void adjust_ptr_info_misalignment (struct ptr_info_def *,
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 2ecd139..8ccecb5 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -404,6 +404,7 @@ dump_alias_info (FILE *file)
struct ptr_info_def *pi;
if (ptr == NULL_TREE
+ || !POINTER_TYPE_P (TREE_TYPE (ptr))
|| SSA_NAME_IN_FREE_LIST (ptr))
continue;
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 0a405ce..420ae00 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -151,7 +151,11 @@ make_ssa_name_fn (struct function *fn, tree var, gimple stmt)
SET_SSA_NAME_VAR_OR_IDENTIFIER (t, var);
}
SSA_NAME_DEF_STMT (t) = stmt;
- SSA_NAME_PTR_INFO (t) = NULL;
+ if (POINTER_TYPE_P (TREE_TYPE (t)))
+SSA_NAME_PTR_INFO (t) = NULL;
+ else
+SSA_NAME_RANGE_INFO (t) = NULL;
+
SSA_NAME_IN_FREE_LIST (t) = 0;
SSA_NAME_IS_DEFAULT_DEF (t) = 0;
imm = (SSA_NAME_IMM_USE_NODE (t));
@@ -266,6 +270,14 @@ mark_ptr_info_alignment_unknown (struct ptr_info_def *pi)
pi-misalign = 0;
}
+/* Set the range described by RI has invalid values. */
+
+void
+mark_range_info_unknown (struct range_info_def *ri)
+{
+ ri-valid = false;
+}
+
/* Store the the power-of-two byte alignment and the deviation from that
alignment of pointer described by PI to ALIOGN and MISALIGN
respectively. */
@@ -359,6 +371,26 @@ duplicate_ssa_name_ptr_info (tree name, struct ptr_info_def *ptr_info)
SSA_NAME_PTR_INFO (name) = new_ptr_info;
}
+/* Creates a duplicate of the range_info_def at RANGE_INFO for use by
+ the SSA name NAME. */
+void
+duplicate_ssa_name_range_info (tree name, struct range_info_def *range_info)
+{
+ struct range_info_def *new_range_info;
+
+ gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
+ gcc_assert (!SSA_NAME_RANGE_INFO (name));
+
+ if (!range_info)
+return;
+
+ new_range_info = ggc_alloc_range_info_def ();
+ *new_range_info = *range_info;
+
+ SSA_NAME_RANGE_INFO (name) = new_range_info;
+}
+
+
/* Creates a duplicate of a ssa name NAME tobe defined by statement STMT
in function FN. */
@@ -367,10 +399,20 @@ tree
duplicate_ssa_name_fn (struct function *fn, tree name, gimple stmt)
{
tree new_name = copy_ssa_name_fn (fn, name, stmt);
- struct ptr_info_def
