Re: Validation of adding left shift stmt at GIMPLE - [Custom GCC plugin]]

[Shubham Narlawar <gsocshubham@gmail.com>]

On Mon, Feb 21, 2022 at 1:02 PM Richard Biener
<richard.guenther@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 <gsocshubham@gmail.com> wrote:
> > >
> > > On Sat, Feb 19, 2022 at 1:15 AM Andrew Pinski <pinskia@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);
          //update_ssa (TODO_update_ssa);

  return 0;
}
---------------------------------------------------------------------------------------------------------

Is the above code correct?

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