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

>           //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.

> 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

Reply via email to