On Sun, Mar 27, 2022 at 7:44 PM Shubham Narlawar <gsocshub...@gmail.com> wrote: > > On Wed, Feb 23, 2022 at 12:52 PM Richard Biener > <richard.guent...@gmail.com> wrote: > > > > On Tue, Feb 22, 2022 at 2:10 PM Shubham Narlawar <gsocshub...@gmail.com> > > wrote: > > > > > > On Tue, Feb 22, 2022 at 3:55 PM Richard Biener > > > <richard.guent...@gmail.com> wrote: > > > > > > > > On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar > > > > <gsocshub...@gmail.com> wrote: > > > > > > > > > > On Mon, Feb 21, 2022 at 1:02 PM Richard Biener > > > > > <richard.guent...@gmail.com> wrote: > > > > > > > > > > > > On Sun, Feb 20, 2022 at 11:44 PM Andrew Pinski via Gcc > > > > > > <gcc@gcc.gnu.org> wrote: > > > > > > > > > > > > > > On Sun, Feb 20, 2022 at 10:45 AM Shubham Narlawar > > > > > > > <gsocshub...@gmail.com> wrote: > > > > > > > > > > > > > > > > On Sat, Feb 19, 2022 at 1:15 AM Andrew Pinski > > > > > > > > <pins...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > On Fri, Feb 18, 2022 at 11:04 AM Shubham Narlawar via Gcc > > > > > > > > > <gcc@gcc.gnu.org> wrote: > > > > > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > > > I want to know whether it is correct to add left shift > > > > > > > > > > instruction to > > > > > > > > > > a constant expression statement like "_3 + 4"? > > > > > > > > > > > > > > > > > > > > I am trying to add a left shift instruction in between > > > > > > > > > > below GIMPLE > > > > > > > > > > instructions - > > > > > > > > > > > > > > > > > > > > <bb 2> : > > > > > > > > > > instrn_buffer.0_1 = instrn_buffer; > > > > > > > > > > _2 = tree.cnt; > > > > > > > > > > _3 = (int) _2; > > > > > > > > > > _4 = _3 + 4; > > > > > > > > > > _5 = (unsigned int) _4; // I want to add left > > > > > > > > > > shift here > > > > > > > > > > D.2993 = __builtin_riscv_sfploadi (instrn_buffer.0_1, 0, > > > > > > > > > > _5); > > > > > > > > > > //this is "stmt" > > > > > > > > > > > > > > > > > > > > I am using this snippet in custom gcc plugin - > > > > > > > > > > > > > > > > > > > > tree lshift_tmp = make_temp_ssa_name > > > > > > > > > > (integer_type_node, > > > > > > > > > > NULL, "slli"); > > > > > > > > > > > > > > > > > > A couple of things. > > > > > > > > > I Noticed you use integer_type_node here. Why not the type of > > > > > > > > > what is > > > > > > > > > being replaced? > > > > > > > > > That is the main thing I see right now. > > > > > > > > > > > > > > > > I want to apply left shift to a constant expression with 8 > > > > > > > > which is an > > > > > > > > integer. Since I am not replacing a statement, I am creating new > > > > > > > > GIMPLE statement - > > > > > > > > > > > > > > > > tree shift_amt = build_int_cst (integer_type_node, 8); > > > > > > > > > > > > > > > > Here, I am not replacing any GIMPLE statement. Is this the > > > > > > > > correct way > > > > > > > > to do this? > > > > > > > > > > > > > > > > My goal is to left shift constant expression and update its > > > > > > > > usage as below - > > > > > > > > > > > > > > > > _19 = (unsigned int) _18; > > > > > > > > D.2996 = __builtin_riscv_sfploadi (lexer.5_16, 12, _19); > > > > > > > > > > > > > > > > into > > > > > > > > > > > > > > > > _19 = (unsigned int) _18; > > > > > > > > temp = _19 << 8 > > > > > > > > D.2996 = __builtin_riscv_sfploadi (lexer.5_16, 12, temp); > > > > > > > > > > > > > > > > I am storing the left shift result to the new ssa variable name > > > > > > > > "temp" > > > > > > > > and updating sfploadi parameters as expected. > > > > > > > > > > > > > > > > On doing the above, dom_walker_eliminate is prohibiting me to > > > > > > > > do the > > > > > > > > above gimple transformation. Is the above transformation > > > > > > > > complete and > > > > > > > > correct? > > > > > > > > > > > > > > I think you misunderstood me. I was saying for a left shift > > > > > > > gimple, > > > > > > > the result type and the first operand type must be compatible > > > > > > > (signed > > > > > > > and unsigned types are not compatible). In the above case, you > > > > > > > have: > > > > > > > integer_type_node = unsigned_int << integer_type_name . > > > > > > > > > > > > > > Does that make sense now? > > > > > > > > > > > > Btw, the error you see is still odd - please make sure to build GCC > > > > > > with > > > > > > checking enabled or run your tests with -fchecking. For further > > > > > > help > > > > > > > > > > Sure. > > > > > > > > > > > it might be useful to post the patch you are testing to show where > > > > > > exactly > > > > > > you are hooking into to add this statement. > > > > > > > > > > My goal is to transform below IR - > > > > > > > > > > _5 = (unsigned int) _4; > > > > > D.2993 = __builtin_riscv_* (instrn_buffer.0_1, 0, _5); > > > > > > > > > > to target IR - > > > > > > > > > > _5 = (unsigned int) _4; > > > > > lshiftamt_27 = _5 << 8; > > > > > D.2996 = __builtin_riscv_* (instrn_buffer.0_1, 0, lshiftamt_27); > > > > > > > > > > I have followed this approach to build a new GIMPLE left shift > > > > > instruction - > > > > > https://github.com/gcc-mirror/gcc/blob/0435b978f95971e139882549f5a1765c50682216/gcc/ubsan.cc#L1257 > > > > > > > > > > Here is the patch which I am using - > > > > > > > > > > ------------------------------------------Patch------------------------------------------------------- > > > > > unsigned int > > > > > pass_custom_lowering::execute (function *fun) > > > > > { > > > > > /* Code for iterating over all statements of function to find > > > > > function call of form - __builtin* > > > > > > > > > > where one of parameter is constant expression of type "7 + > > > > > expression" i.e. 7 + _8 > > > > > > > > > > <bb 2> : > > > > > instrn_buffer.0_1 = instrn_buffer; > > > > > _2 = tree.cnt; > > > > > _3 = (int) _2; > > > > > _4 = _3 + 4; > > > > > _5 = (unsigned int) _4; // I want to apply left shift to _5 > > > > > D.2993 = __builtin_riscv_* (instrn_buffer.0_1, 0, _5); > > > > > > > > > > */ > > > > > gcall *stmt = dyn_cast<gcall *> (g); //here g is > > > > > __builtin_riscv_* where one of parameter is "_3 + 4" > > > > > tree parm = gimple_call_arg (stmt, 2); > > > > > > > > > > unsigned int shift = 8; > > > > > tree shift_amt = build_int_cst (unsigned_type_node, shift); > > > > > tree lshift_tmp_name = make_temp_ssa_name > > > > > (unsigned_type_node, NULL, "slli"); > > > > > gimple *lshift_stmt = gimple_build_assign (lshift_tmp_name, > > > > > LSHIFT_EXPR, parm, > > > > > shift_amt); > > > > > gsi_insert_before(&gsi, lshift_stmt, GSI_NEW_STMT); > > > > > gimple_call_set_arg (stmt, 2, lshift_tmp_name); > > > > > //update_stmt (stmt); > > > > > > > > This update_stmt is required > > > > > > Got it. Thanks! > > > > > > > > > > > > //update_ssa (TODO_update_ssa); > > > > > > > > > > return 0; > > > > > } > > > > > --------------------------------------------------------------------------------------------------------- > > > > > > > > > > Is the above code correct? > > > > > > > > Yes, the code is incomplete of course, it misses how you get to 'g' > > > > but I assume it's just FOR_EACH_BB_FN and iteration over each > > > > BBs stmts. > > > > > > Yes. These are present FOR_EACH_BB_FN in my code to identify specific > > > GIMPLE_CALL statements. > > > > > > After adding all above suggestions, fixup_cfg is failing due to the > > > above transformation. I am thinking that the position of the pass > > > might be the problem. > > > > > > Currently my pass "pass_custom_lowering" is present at end of > > > "all_lowering_pass as below - > > > > > > INSERT_PASSES_AFTER (all_lowering_passes) > > > NEXT_PASS (pass_warn_unused_result); > > > NEXT_PASS (pass_diagnose_omp_blocks); > > > NEXT_PASS (pass_diagnose_tm_blocks); > > > .......... > > > NEXT_PASS (pass_build_cfg); > > > ........ > > > NEXT_PASS (pass_build_cgraph_edges); > > > NEXT_PASS (pass_custom_lowering); > > > TERMINATE_PASS_LIST (all_lowering_passes) > > > > > > Can you suggest a proper place for the pass for the above kind of > > > transformation? > > > > I don't see a particular problem with this place but I would suggest > > to move it into the pass_build_ssa_passes group, before pass_ubsan > > for example (but certainly after pass_build_ssa). Given you run > > pre SSA build maybe you simply have stray TODO not appropriate > > in your pass_data. > > > > Anyway, try moving the pass after build-ssa. > > Hello, > > [1] With the above suggestion - I was able to do the desired > transformation using a plugin. I am generating custom left shift, > right shift, load immediate and an add instruction just before a > specific function call. > > e.g. > > slli_37 = _2 << 8; > srli_38 = slli_37 >> 8; > li_39 = (unsigned int) 15; > add_40 = srli_38 + li_39; > _22 = __builtin_* (instrn_buffer.36_1, 1, add_40); > > > [2] My ultimate aim is to identify and load immediate instruction > "li_39 = (unsigned int) 15;" during cfgexpand pass and store rtx > instructions associated with it. Then in the custom rtl pass after > register allocation pass, I want to do some transformation on it. But > this instruction is optimized by ccp and hence I am losing the > placeholder. > > ----------I am generating gimple instruction "li" using---------------- > tree load = build_int_cst (unsigned_type_node, 15); > tree li_tmp_name = make_temp_ssa_name > (unsigned_type_node, NULL, "li"); > gimple *li_stmt = gimple_build_assign (li_tmp_name, > NOP_EXPR, load); > gsi_insert_after (&gsi, li_stmt, GSI_NEW_STMT); > --------------------------------------------------------------------- > > My aim is to prohibit the above but it seems incorrect to just disable > ccp. Please give any suggestions on retaining the statement. > > 1. To prohibit above, I tried to assign global variable to "li" as below - > > tree li_tmp_name = make_temp_ssa_name > (integer_type_node, NULL, "li"); > tree foo_tmp_name = build_translation_unit_decl > (create_tmp_var_name("foo")); > DECL_EXTERNAL(foo_tmp_name) = 1; > gimple *li_stmt = gimple_build_assign (li_tmp_name, > NOP_EXPR, foo_tmp_name); > gsi_insert_after (&gsi, li_stmt, GSI_NEW_STMT); > > which generates - (but it crashes) > > li_51 = (int) <<< Unknown tree: translation_unit_decl >>>; > > Can you please suggest to me the correct way to achieve the above?
The error in the above is that you use TRANSLATION_UNIT_DECL, you want a normal VAR_DECL for this. But then if you want to just have a marker here I would suggest to build an asm() you can then drop at RTL stage again, see for example gimple-harden-conditionals.cc:detach_value () Richard. > > Thanks and Regards, > Shubham > > > > > > Thanks and Regards, > > > Shubham > > > > > > > > > > > > I was hoping to do the below transformation using above code but feel > > > > > there is some missing operation that needs to be added to above code. > > > > > The goal is simple to generate a left shift to the 3rd parameter of > > > > > function which is constant expression. > > > > > > > > > > _5 = (unsigned int) _4; > > > > > D.2993 = __builtin_riscv_* (instrn_buffer.0_1, 0, _5); > > > > > > > > > > to > > > > > > > > > > _5 = (unsigned int) _4; > > > > > lshiftamt_27 = _5 << 8; > > > > > D.2996 = __builtin_riscv_* (instrn_buffer.0_1, 0, lshiftamt_27); > > > > > > > > > > Thanks and Regards, > > > > > Shubham > > > > > > > > > > > > > > > > > > > > > > Richard. > > > > > > > > > > > > > Thanks, > > > > > > > Andrew > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also you shouldn't need to do: > > > > > > > > > update_ssa (TODO_update_ssa); > > > > > > > > > > > > > > > > > > As make_temp_ssa_name is a new SSA name already and such. > > > > > > > > > > > > > > > > Understood. > > > > > > > > > > > > > > > > Thanks and Regards, > > > > > > > > Shubham > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Andrew Pinski > > > > > > > > > > > > > > > > > > > gimple *lshift = gimple_build_assign (lshift_tmp, > > > > > > > > > > LSHIFT_EXPR, parm, > > > > > > > > > > > > > > > > > > > > build_int_cst > > > > > > > > > > (integer_type_node, 8)); > > > > > > > > > > gsi_insert_before(&gsi, lshift, GSI_NEW_STMT); > > > > > > > > > > //Update function call > > > > > > > > > > gimple_call_set_arg (stmt, idx, lshift_tmp); > > > > > > > > > > update_stmt (stmt); > > > > > > > > > > update_ssa (TODO_update_ssa); > > > > > > > > > > > > > > > > > > > > from which above GIMPLE IR is modified to - > > > > > > > > > > > > > > > > > > > > <bb 2> : > > > > > > > > > > instrn_buffer.0_1 = instrn_buffer; > > > > > > > > > > _2 = tree.cnt; > > > > > > > > > > _3 = (int) _2; > > > > > > > > > > _4 = _3 + 4; > > > > > > > > > > _5 = (unsigned int) _4; > > > > > > > > > > slli_24 = _5 << 8; > > > > > > > > > > D.2993 = __builtin_riscv_sfploadi (instrn_buffer.0_1, 0, > > > > > > > > > > slli_24); > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. When I run above code, either dominator tree validation > > > > > > > > > > or tree cfg > > > > > > > > > > fixup is failing which suggests to me it is either > > > > > > > > > > incorrect to apply > > > > > > > > > > such left shift or some more work is missing? > > > > > > > > > > > > > > > > > > > > 2. I followed how a left shift gimple assignment is > > > > > > > > > > generated but > > > > > > > > > > still feels there is something wrong with the above > > > > > > > > > > generation. Can > > > > > > > > > > someone please point me out? > > > > > > > > > > > > > > > > > > > > Thanks in advance! As always, the GCC community and its > > > > > > > > > > members are > > > > > > > > > > very supportive, responsive and helpful! > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > Shubham
