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

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

[Shubham Narlawar]

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");
          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

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

[Andrew Pinski]

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.

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.

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

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

[Shubham Narlawar]

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?

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

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

[Andrew Pinski]

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?

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

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

[Richard Biener]

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
it might be useful to post the patch you are testing to show where exactly
you are hooking into to add this statement.

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

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

[Shubham Narlawar]

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

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

[Richard Biener]

On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar <gsocshubham@gmail.com> wrote:
>
> 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);

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

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

[Shubham Narlawar]

On Tue, Feb 22, 2022 at 3:55 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar <gsocshubham@gmail.com> wrote:
> >
> > 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);
>
> 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?

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

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

[Richard Biener]

On Tue, Feb 22, 2022 at 2:10 PM Shubham Narlawar <gsocshubham@gmail.com> wrote:
>
> On Tue, Feb 22, 2022 at 3:55 PM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar <gsocshubham@gmail.com> wrote:
> > >
> > > 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);
> >
> > 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.

> 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

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

[Shubham Narlawar]

On Wed, Feb 23, 2022 at 12:52 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Feb 22, 2022 at 2:10 PM Shubham Narlawar <gsocshubham@gmail.com> wrote:
> >
> > On Tue, Feb 22, 2022 at 3:55 PM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > >
> > > On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar <gsocshubham@gmail.com> wrote:
> > > >
> > > > 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);
> > >
> > > 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?

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

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

[Richard Biener]

On Sun, Mar 27, 2022 at 7:44 PM Shubham Narlawar <gsocshubham@gmail.com> wrote:
>
> On Wed, Feb 23, 2022 at 12:52 PM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Tue, Feb 22, 2022 at 2:10 PM Shubham Narlawar <gsocshubham@gmail.com> wrote:
> > >
> > > On Tue, Feb 22, 2022 at 3:55 PM Richard Biener
> > > <richard.guenther@gmail.com> wrote:
> > > >
> > > > On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar <gsocshubham@gmail.com> wrote:
> > > > >
> > > > > 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);
> > > >
> > > > 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