This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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.vivekanandarajah@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 0x2aaaabd89af8 nothrow var <var_decl 0x2aaaadb384c0 t>def_stmt version 11 in-free-list>This is an invalid SSA name (in-free-list) that has been released. You shouldn't look at it at all.
This is actually happening in garbage collection. If i remove the check, I get the following:
make[2]: *** [_mulsc3.o] Error 1 0x97755f crash_signal /home/kugan/work/sources/gcc-fsf/test/gcc/toplev.c:335 0x55f7d1 gt_ggc_mx_lang_tree_node(void*) ./gt-c-c-decl.h:531 0x802e36 gt_ggc_mx<tree_node*> /home/kugan/work/sources/gcc-fsf/test/gcc/vec.h:1152 0x802e36 gt_ggc_mx_vec_tree_va_gc_(void*) /home/kugan/work/builds/gcc-fsf-test/obj-arm-none-linux-gnueabi/gcc1/gcc/gtype-desc.c:1205 0x8081ed gt_ggc_mx_gimple_df(void*) /home/kugan/work/builds/gcc-fsf-test/obj-arm-none-linux-gnueabi/gcc1/gcc/gtype-desc.c:1066 0x80825f gt_ggc_mx_function /home/kugan/work/builds/gcc-fsf-test/obj-arm-none-linux-gnueabi/gcc1/gcc/gtype-desc.c:1280 0x80825f gt_ggc_mx_function(void*) /home/kugan/work/builds/gcc-fsf-test/obj-arm-none-linux-gnueabi/gcc1/gcc/gtype-desc.c:1272 0x55f692 gt_ggc_mx_lang_tree_node(void*) ./gt-c-c-decl.h:389 0x807dd5 gt_ggc_mx_symtab_node_def(void*) /home/kugan/work/builds/gcc-fsf-test/obj-arm-none-linux-gnueabi/gcc1/gcc/gtype-desc.c:709 0x807fb0 gt_ggc_m_P15symtab_node_def4htab(void*) /home/kugan/work/builds/gcc-fsf-test/obj-arm-none-linux-gnueabi/gcc1/gcc/gtype-desc.c:2928 0x7b7915 ggc_mark_root_tab /home/kugan/work/sources/gcc-fsf/test/gcc/ggc-common.c:133 0x7b7c60 ggc_mark_roots() /home/kugan/work/sources/gcc-fsf/test/gcc/ggc-common.c:152 0x615669 ggc_collect() /home/kugan/work/sources/gcc-fsf/test/gcc/ggc-page.c:2077 Thanks a lot. Kugan
Richard.+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)I will change it.+ /* Allocate if not available. */ + if (ri == NULL) + { + ri = ggc_alloc_cleared_range_info_def (); + mark_range_info_unknown (ri); that looks superfluous to me. + SSA_NAME_RANGE_INFO (name) = ri; - /* Pointer attributes used for alias analysis. */ - struct ptr_info_def *ptr_info; + /* Value range information. */ + 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.def_stmt && !POINTER_TYPE_P (TREE_TYPE ((tree)&%1))"))) vrp; please change vrp_info_type and vrp to other names - this is not vrp specific info after all, I suggest ssa_name_info_type and info.I will change this too.The genric bits otherwise look ok to me, the VRP bits still look wrong (see my above question) and need explanation. Thanks, Richard.Thanks, KuganThanks, Kugan
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |