Hi, compiling the attached testcase (reduced from tree.c) with -O2 leads to vrp folding: if (D.2762_2 == 6) goto <bb 3>; else goto <bb 9>; <bb 3>: D.2766_5 = (int) D.2762_2; D.2767_6 = tree_code_type[D.2766_5]; into D.2762_2 = type_1(D)->base.code; if (D.2762_2 == 6) goto <bb 3>; else goto <bb 9>; <bb 3>: D.2766_5 = 6; D.2767_6 = tree_code_type[6]; this is good transform, however tree_code_type is constant initialized array and we miss possibility to fold it into constant until expansion triggering code in expr.c I would like to retire. We can fold tree_code_type[6], so apparently just no one calls fold_gimple_stmt on it?
Created attachment 21684 [details] testcase
OK, the problem seems to be that fold_stmt seems to make no serious attempt to fold constant references. There is some code in maybe_fold_reference that seems to partly duplicate fold_const_aggregate_ref however. What is reason for that? The following patch solves the problem, I am looking what other testcases I can find.
Created attachment 21685 [details] patch for better folding
A related testcase where we fail to fold fundamentals[0] typedef union tree_node *tree; enum tree_code { OFFSET_TYPE, ENUMERAL_TYPE, BOOLEAN_TYPE, POINTER_TYPE, FIXED_POINT_TYPE, }; struct tree_base { unsigned public_flag:1; }; struct tree_decl_with_vis { unsigned comdat_flag:1; }; union tree_node { struct tree_base base; struct tree_decl_with_vis decl_with_vis; }; enum tree_index { TI_LONG_DOUBLE_PTR_TYPE, TI_INTEGER_PTR_TYPE, TI_VOID_TYPE, TI_PTR_TYPE, TI_VA_LIST_FPR_COUNTER_FIELD, TI_BOOLEAN_TYPE, TI_FILEPTR_TYPE, TI_CURRENT_TARGET_PRAGMA, TI_CURRENT_OPTIMIZE_PRAGMA, TI_MAX }; extern tree global_trees[TI_MAX]; emit_support_tinfos (void) { static tree *const fundamentals[] = { &global_trees[TI_VOID_TYPE], &global_trees[TI_BOOLEAN_TYPE], }; int ix; for (ix = 0; fundamentals[ix]; ix++) { { tree tinfo; { ((void) (!(((tinfo)->base.public_flag) && !(__extension__ ( { __typeof (tinfo) __t = (tinfo); __t;} )->decl_with_vis. comdat_flag)) ? fancy_abort ("../../gcc/cp/rtti.c", 1529, __FUNCTION__), 0 : 0)); } } } }
And here we fail to fold messages[1] created by PRE enum { ERROR_OK, ERROR_UNKNOWN, ERROR_NUM }; enum { __LC_CTYPE = 0, __LC_NUMERIC = 1, __LC_TIME = 2, __LC_COLLATE = 3, __LC_MONETARY = 4, __LC_MESSAGES = 5, __LC_ALL = 6, __LC_PAPER = 10, __LC_MEASUREMENT = 11, __LC_IDENTIFICATION = 12 }; static const char *const _messages[] = { "no error", "unknown error", "Internal error: unknown reason", }; elf_errmsg (int err) { if (err < 0 || err >= ERROR_NUM || _messages[err] == ((void *) 0)) { err = ERROR_UNKNOWN; } return _messages[err]; }
In testcase from comment #4 the problem is that value in DECL_INITIAL is not in the form acceptable for gimple_min_invariant. Have patch. In testcase from comment #5 we never try to fold it.
In #5 the expression is created by PRE via create_expression_by_pieces that uses normal fold that is not able of constant variable folding. The statement does not get folded later and survives. I guess one can fold all statements in commit_edge_insertions but it seems by symptomatic?
Subject: Re: VRP misses oppurtunity for statement folding. On Fri, 3 Sep 2010, hubicka at gcc dot gnu dot org wrote: > ------- Comment #7 from hubicka at gcc dot gnu dot org 2010-09-03 20:28 ------- > In #5 the expression is created by PRE via create_expression_by_pieces that > uses normal fold that is not able of constant variable folding. The statement > does not get folded later and survives. I guess one can fold all statements in > commit_edge_insertions but it seems by symptomatic? You need to enhance fully_constant_vn_reference_p. Richard.
Hi, thanks. In meantime I made tree-ssa-pre to fold statements it produces and it gets me to bootstrapland with sanity check in expr.c except for Ada (with the patches I sent so far) So it seems that I need to basically duplicate all logic for initializer folding from tree-ssa-ccp.c into this function, right? I guess it makes sense, but it is all quite ugly. On VN side, i wondered if we can retire more of expand this way. For example dojump knows that: a = b ror x; if (a != 0) can be folded into: if (b != 0) (ror is rotation). I guess we should do this kind of tricks in VN instead? Honza
Subject: Re: VRP misses oppurtunity for statement folding. On Sat, 4 Sep 2010, hubicka at gcc dot gnu dot org wrote: > ------- Comment #9 from hubicka at gcc dot gnu dot org 2010-09-04 13:51 ------- > Hi, > thanks. In meantime I made tree-ssa-pre to fold statements it produces and it > gets me to bootstrapland with sanity check in expr.c except for Ada (with the > patches I sent so far) Well - that's a workaround and will cause us to miss PRE because we do not fold during translation of expressions. So I wouldn't go down that route. > So it seems that I need to basically duplicate all logic for initializer > folding from tree-ssa-ccp.c into this function, right? I guess it makes sense, > but it is all quite ugly. Yes ;) > On VN side, i wondered if we can retire more of expand this way. For example > dojump knows that: > a = b ror x; > if (a != 0) > can be folded into: > if (b != 0) > (ror is rotation). I guess we should do this kind of tricks in VN instead? Well ... it's not that easy (that's not CSE but tree-combining, so the specific thing would fit to forwprop). Richard.
Created attachment 21700 [details] proposed fix for sccvn Well, this is patch I am currently testing. At least small part is shared in between tree-ssa-ccp (probably should go into gimple-fold) and VN. I am not sure how much of other initializer folding I need to borrow - i.e. folding of var_decl into its decl_initial, handling of component_refs and such. Also the tree-ssa-ccp code seems quite incomplette, it won't fold array with incomplette initializer for example because it won't find the matching item. Probably worth to fix. Honza
Confirmed.
comment #3 works now: Folding statement: D.2751_6 = tree_code_type[D.2750_5]; Folded into: D.2751_6 = 2; I can't see how comment #4 should work - in VRP1 we end up with <bb 2>: goto <bb 4>; <bb 3>: D.2737_5 = BIT_FIELD_REF <*tinfo_4(D), 8, 0>; D.2738_6 = D.2737_5 & 1; fancy_abort ("../../gcc/cp/rtti.c", 1529, &__FUNCTION__); ix_11 = ix_1 + 1; <bb 4>: # ix_1 = PHI <0(2), ix_11(3)> D.2742_3 = fundamentals[ix_1]; if (D.2742_3 != 0B) goto <bb 3>; else goto <bb 5>; <bb 5>: return; so I suppose an earlier patch already did the folding. I can confirm comment #5. Removing VRP reference from the summary.
So the testcase in comment #5 I think the problem is we can't figure out _messages[err] == ((void *) 0 is false.
(In reply to Andrew Pinski from comment #14) > So the testcase in comment #5 I think the problem is we can't figure out > _messages[err] == ((void *) 0 is false. I think we do fold it, I see <bb 4> [local count: 440234144]: _2 = _messages[err_6(D)]; if (_2 == 0B) goto <bb 3>; [30.00%] else goto <bb 5>; [70.00%] <bb 5> [local count: 1073741824]: # err_5 = PHI <err_6(D)(4), 1(3)> # prephitmp_11 = PHI <_2(4), "unknown error"(3)> _4 = (long int) prephitmp_11; _8 = (int) _4; return _8; and [changed] ANTIC_IN[5] := { err_5 (0005), {array_ref<err_5,0,1>,mem_ref<0B>,addr_expr<&_messages>}@.MEM_7(D) (0003), {nop_expr,_3} (0004), {nop_expr,_4} (0008) } S[5] := { } Created SSA_NAME representative pretmp_9 for expression:{nop_expr,"unknown error"} (0009) ANTIC_OUT[4] := { {nop_expr,"unknown error"} (0009), {nop_expr,pretmp_9} (0010) } so it's folded during antic compute already. Works with GCC 10 as well.