On Fri, Aug 28, 2020 at 6:29 AM Gary Oblock <g...@amperecomputing.com> wrote: > > > If x_2 is a default def then the IL isn't correct in the first place. I > > doubt > > it is that way, btw. - we have verifiers that would blow up if it would. > > Richard, > > I'm just sharing this so you can tell me whether or not I'm going > crazy. ;-) > > This little function is finding that arr_2 = PHI <arr_12(2), arr_15(5)> > is problematic. > > void > wolf_fence ( > Info *info // Pass level gobal info (might not use it) > ) > { > struct cgraph_node *node; > > fprintf( stderr, > "Wolf Fence: Find wolf for default defs with non nop defines\n"); > > FOR_EACH_FUNCTION_WITH_GIMPLE_BODY ( node) > { > struct function *func = DECL_STRUCT_FUNCTION ( node->decl); > push_cfun ( func); > > unsigned int len = SSANAMES ( func)->length (); > for ( unsigned int i = 0; i < len; i++) > > { > > tree ssa_name = (*SSANAMES ( func))[i]; > if ( ssa_name == NULL ) continue; > if ( ssa_defined_default_def_p ( ssa_name) )
That's because this function is supposed to be only called on SSA default defs but you call it on all SSA names. Add a SSA_NAME_IS_DEFAULT_DEF (ssa_name) && before and all will be well. > { > gimple *def_stmt = > SSA_NAME_DEF_STMT ( ssa_name); > if ( !gimple_nop_p ( def_stmt) ) > > { > fprintf ( stderr, "Wolf fence caught :"); > print_gimple_stmt ( stderr, def_stmt, 0); > gcc_assert (0); > } > > } > > } > > pop_cfun (); > } > fprintf( stderr, "Wolf Fence: Didn't find wolf!\n"); > } > > This is run at the very start of the structure reorg pass > before any of my code did anything at all (except initiate > the structure info with a few flags and the like.) > > Here's C code: > > ----- aux.h --------------------------------------------------- > #include "stdlib.h" > typedef struct type type_t; > struct type { > double x; > double y; > }; > > extern type_t *min_of_x( type_t *, size_t); > ----- aux.c --------------------------------------------------- > #include "aux.h" > #include "stdlib.h" > > type_t * > min_of_x( type_t *arr, size_t len) > { > type_t *end_of = arr + len; > type_t *loc = arr; > double result = arr->x; > arr++; > for( ; arr < end_of ; arr++ ) { > double value = arr->x; > if ( value < result ) { > result = value; > loc = arr; > } > } > return loc; > } > ----- main.c -------------------------------------------------- > #include "aux.h" > #include "stdio.h" > > int > main(void) > { > size_t len = 10000; > type_t *data = (type_t *)malloc( len * sizeof(type_t)); > int i; > for( i = 0; i < len; i++ ) { > data[i].x = drand48(); > } > > type_t *min_x; > min_x = min_of_x( data, len); > > if ( min_x == 0 ) { > printf("min_x error\n"); > exit(-1); > } > > printf("min_x %e\n" , min_x->x); > } > --------------------------------------------------------------- > Here's the GIMPLE comining into the structure reoganization pass: > > Program: > > ;; Function min_of_x (min_of_x, funcdef_no=0, decl_uid=4391, cgraph_uid=2, > symbol_order=23) (executed once) > > min_of_x (struct type_t * arr, size_t len) > { > double value; > double result; > struct type_t * loc; > struct type_t * end_of; > > <bb 2> [local count: 118111600]: > _1 = len_7(D) * 16; > end_of_9 = arr_8(D) + _1; > result_11 = arr_8(D)->x; > arr_12 = arr_8(D) + 16; > goto <bb 6>; [100.00%] > > <bb 3> [local count: 955630225]: > value_14 = arr_2->x; > if (result_6 > value_14) > goto <bb 4>; [50.00%] > else > goto <bb 5>; [50.00%] > > <bb 4> [local count: 477815112]: > > <bb 5> [local count: 955630225]: > # loc_3 = PHI <loc_4(3), arr_2(4)> > # result_5 = PHI <result_6(3), value_14(4)> > arr_15 = arr_2 + 16; > > <bb 6> [local count: 1073741824]: > # arr_2 = PHI <arr_12(2), arr_15(5)> > # loc_4 = PHI <arr_8(D)(2), loc_3(5)> > # result_6 = PHI <result_11(2), result_5(5)> > if (arr_2 < end_of_9) > goto <bb 3>; [89.00%] > else > goto <bb 7>; [11.00%] > > <bb 7> [local count: 118111600]: > # loc_13 = PHI <loc_4(6)> > return loc_13; > > } > > > > ;; Function main (main, funcdef_no=1, decl_uid=4389, cgraph_uid=1, > symbol_order=5) (executed once) > > main () > { > struct type_t * min_x; > int i; > struct type_t * data; > > <bb 2> [local count: 10737416]: > data_10 = malloc (160000); > goto <bb 4>; [100.00%] > > <bb 3> [local count: 1063004409]: > _1 = _4 * 16; > _2 = data_10 + _1; > _3 = drand48 (); > _2->x = _3; > i_18 = i_6 + 1; > > <bb 4> [local count: 1073741824]: > # i_6 = PHI <0(2), i_18(3)> > _4 = (long unsigned int) i_6; > if (_4 != 10000) > goto <bb 3>; [99.00%] > else > goto <bb 5>; [1.00%] > > <bb 5> [local count: 10737416]: > min_x_12 = min_of_x (data_10, 10000); > if (min_x_12 == 0B) > goto <bb 6>; [0.04%] > else > goto <bb 7>; [99.96%] > > <bb 6> [local count: 4295]: > __builtin_puts (&"min_x error"[0]); > exit (-1); > > <bb 7> [local count: 10733121]: > _5 = min_x_12->x; > printf ("min_x %e\n", _5); > return 0; > > } > > Am I crazy? > > Thanks, > > Gary > > > > > > > > > ________________________________ > From: Richard Biener <richard.guent...@gmail.com> > Sent: Thursday, August 27, 2020 2:04 AM > To: Gary Oblock <g...@amperecomputing.com> > Cc: gcc@gcc.gnu.org <gcc@gcc.gnu.org> > Subject: Re: Questions regarding update_stmt and release_ssa_name_fn. > > [EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please > be mindful of safe email handling and proprietary information protection > practices.] > > > On Wed, Aug 26, 2020 at 11:32 PM Gary Oblock via Gcc <gcc@gcc.gnu.org> wrote: > > > > I'm having some major grief with a few related things that I'm try to > > do. The mostly revolve around trying to change the type of an SSA name > > (which I've given up in favor of creating new SSA names and replacing > > the ones I wanted to change.) However, this seems too has its own > > issues. > > > > In one problematic case in particular, I'm seeing a sequence like: > > > > foo_3 = mumble_1 op mumble_2 > > > > bar_5 = foo_3 op baz_4 > > > > when replace foo_3 with foo_4 the (having the needed new type.) > > > > I'm seeing a later verification phase think > > > > bar_5 = foo_4 op baz_4 > > > > is still associated with the foo_3. > > > > Should the transformation above be associated with update_stmt and/or > > release_ssa_name_fn? And if they are both needed is there a proper > > order required. Note, when I try using them, I'm seeing some malformed > > tree operands that die in horrible ways. > > > > By the way, I realize I can probably simply create a new GIMPLE stmt > > from scratch to replace the ones I'm modifying but this will cause > > some significant code bloat and I want to avoid that if at all > > possible. > > You need to call update_stmt () if you change SSA operands to > sth else. > > > There is an addition wrinkle to this problem with C code like this > > > > void > > whatever ( int x, .. ) > > { > > : > > x++; > > : > > } > > > > I'm seeing x_2 being thought of as default definition in the following > > GIMPLE stmt when it's clearly not since it's defined by the statement. > > > > x_2 = X_1 + 4 > > > > My approach has been to simply make the SSA name to replace x_2a > > normal SSA name and not a default def. Is this not reasonable and > > correct? > > If x_2 is a default def then the IL isn't correct in the first place. I doubt > it is that way, btw. - we have verifiers that would blow up if it would. > > Richard. > > > > > Thanks, > > > > Gary Oblock > > > > Gary > > > > > > > > > > > > CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is > > for the sole use of the intended recipient(s) and contains information that > > is confidential and proprietary to Ampere Computing or its subsidiaries. It > > is to be used solely for the purpose of furthering the parties' business > > relationship. Any unauthorized review, copying, or distribution of this > > email (or any attachments thereto) is strictly prohibited. If you are not > > the intended recipient, please contact the sender immediately and > > permanently delete the original and any copies of this email and any > > attachments thereto.