Re: Validation of adding left shift stmt at GIMPLE - [Custom GCC plugin]]
On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar wrote: > > On Mon, Feb 21, 2022 at 1:02 PM Richard Biener > wrote: > > > > On Sun, Feb 20, 2022 at 11:44 PM Andrew Pinski via Gcc > > wrote: > > > > > > On Sun, Feb 20, 2022 at 10:45 AM Shubham Narlawar > > > wrote: > > > > > > > > On Sat, Feb 19, 2022 at 1:15 AM Andrew Pinski wrote: > > > > > > > > > > On Fri, Feb 18, 2022 at 11:04 AM Shubham Narlawar via Gcc > > > > > 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 - > > > > > > > > > > > >: > > > > > > 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 > >: > 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 (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_upd
Re: Benchmark recommendations needed
Dhrystone is (and probably always was) a bogus benchmark. It's a well-known truism that MIPS stands for Meaningless Indication of Processor Speed, and dhrystone scores are equally meaningless. Dhrystone fell out of common usage over 20 years ago. It's not GCC that is being peculiar, it's just Dhrystone is pointless. R. On 22/02/2022 05:22, Andras Tantos wrote: That's true, I did notice GCC being rather ... peculiar about drhystone. Is there a way to make it less clever about the benchmark? Or is there some alteration to the benchmark I can make to not trigger the special behavior in GCC? Andras On Mon, 2022-02-21 at 03:19 +, Gary Oblock via Gcc wrote: Trying to use the dhrystone isn't going to be very useful. It has many downsides not the least is that gcc's optimizer can run rings about it. Gary From: Gcc on behalf of gcc-requ...@gcc.gnu.org Sent: Tuesday, February 15, 2022 6:25 AM To: gcc@gcc.gnu.org Subject: Re: [EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] Send Gcc mailing list submissions to gcc@gcc.gnu.org To subscribe or unsubscribe via the World Wide Web, visit https://gcc.gnu.org/mailman/listinfo/gcc or, via email, send a message with subject or body 'help' to gcc-requ...@gcc.gnu.org You can reach the person managing the list at gcc-ow...@gcc.gnu.org When replying, please edit your Subject line so it is more specific than "Re: Contents of Gcc digest..."
Re: Validation of adding left shift stmt at GIMPLE - [Custom GCC plugin]]
On Tue, Feb 22, 2022 at 3:55 PM Richard Biener wrote: > > On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar > wrote: > > > > On Mon, Feb 21, 2022 at 1:02 PM Richard Biener > > wrote: > > > > > > On Sun, Feb 20, 2022 at 11:44 PM Andrew Pinski via Gcc > > > wrote: > > > > > > > > On Sun, Feb 20, 2022 at 10:45 AM Shubham Narlawar > > > > wrote: > > > > > > > > > > On Sat, Feb 19, 2022 at 1:15 AM Andrew Pinski > > > > > wrote: > > > > > > > > > > > > On Fri, Feb 18, 2022 at 11:04 AM Shubham Narlawar via Gcc > > > > > > 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 - > > > > > > > > > > > > > >: > > > > > > > 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 > > > >: > > 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 (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");
strcpy and strcat seem to lead to a stack overflow
Dear developers: I find it counterintuitive that if I repeatedly reset a variable by using strcpy with an empty string "" to that variable and then us strcat to add characters to that variable that that seems to lead to a stack overflow. I would expect strcpy to first free the variable, then malloc, then copy the string value into the variable. I think that would be a better interpretation, because it can keep running for quite some time before it overflows and doesn’t really call it. Instead, I got "Illegal instruction: 4". I ended up reimplementing the reset function, implementing it with free and malloc myself, but the way strings have been implemented in C is highly counter-intuitive. In general pointers tend to be bug-prone, but here you would expect this not to happen. I hope you can fix this. Personally, I’m looking into switching to Ada. All the best, Emile M. Hobo - Au fin! Et encore en plus. -
Re: Benchmark recommendations needed
Andras, The whole point of benchmarks is to judge a processor's performance. That being said, just crippling GCC is not reasonable because processors must be judged in the appropriate context and that includes the current state of the art compiler technology. If you have a new processor I'd benchmark it using the applications you built it for. Gary From: Andras Tantos Sent: Monday, February 21, 2022 9:22 PM To: Gary Oblock ; gcc@gcc.gnu.org Subject: Re: Benchmark recommendations needed [EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] That's true, I did notice GCC being rather ... peculiar about drhystone. Is there a way to make it less clever about the benchmark? Or is there some alteration to the benchmark I can make to not trigger the special behavior in GCC? Andras On Mon, 2022-02-21 at 03:19 +, Gary Oblock via Gcc wrote: > Trying to use the dhrystone isn't going to be very useful. It has > many downsides not the least is that gcc's optimizer can run rings > about it. > > Gary > > > From: Gcc on > behalf of gcc-requ...@gcc.gnu.org > Sent: Tuesday, February 15, 2022 6:25 AM > To: gcc@gcc.gnu.org > Subject: Re: > > [EXTERNAL EMAIL NOTICE: This email originated from an external > sender. Please be mindful of safe email handling and proprietary > information protection practices.] > > > Send Gcc mailing list submissions to > gcc@gcc.gnu.org > > To subscribe or unsubscribe via the World Wide Web, visit > https://gcc.gnu.org/mailman/listinfo/gcc > or, via email, send a message with subject or body 'help' to > gcc-requ...@gcc.gnu.org > > You can reach the person managing the list at > gcc-ow...@gcc.gnu.org > > When replying, please edit your Subject line so it is more specific > than "Re: Contents of Gcc digest..."
Re: Benchmark recommendations needed
> On Feb 22, 2022, at 4:26 PM, Gary Oblock via Gcc wrote: > > Andras, > > The whole point of benchmarks is to judge a processor's performance. > That being said, just crippling GCC is not reasonable because > processors must be judged in the appropriate context and that > includes the current state of the art compiler technology. If you have > a new processor I'd benchmark it using the applications you built it > for. Exactly. Part of what you want to see is that GCC optimizes well for the new machine, i.e., that there aren't artifacts of the machine description that get in the way of optimization. So you'd want to use applications that are good exercises not just of the code generator but also the optimizer. Dhrystone isn't really that, because it has evolved into mostly an optimizer test, not a machine or code generator test. paul
Re: strcpy and strcat seem to lead to a stack overflow
You may be thinking of string capabilities in some other language. Selected from the Linux man pages for these glibc functions: strcpy: char *strcpy(char *dest, const char *src); The strcpy() function copies the string pointed to by src, including the terminating null byte ('\0'), to the buffer pointed to by dest. The strings may not overlap, and the destination string dest must be large enough to receive the copy. Beware of buffer overruns! strcat: char *strcat(char *dest, const char *src); The strcat() function appends the src string to the dest string, over‐ writing the terminating null byte ('\0') at the end of dest, and then adds a terminating null byte. The strings may not overlap, and the dest string must have enough space for the result. If dest is not large enough, program behavior is unpredictable; buffer overruns are a favorite avenue for attacking secure programs. Neither strcpy nor strcat allocate or release buffers. The programmer is expected to have previously allocated the dest buffer of sufficient size. It seems likely from the behavior you describe, in your case, the const src string is allocated on the stack and your use of strcat with an unallocated dest is overwriting the end of the src string on each iteration. Ultimately you either run out of stack space or wipe out some other stack data which causes unpredictable behavior. Strings in C are not particularly user friendly. As to whether they are intuitive, it all depends on what language we first learn. Many languages invented in the 70s and 80s did not have strong string handling capabilities. If you learn string handling on one of those languages first, you come to not expect much and are pleasantly surprised when encountering a language that does the support work for you. Many recommend using strncpy and strncat which require explicit string lengths in order to remind the programmer to be careful about buffer sizes and to avoid the risks of unterminated strings. - patrick On 2/22/2022 3:01 PM, Emile Michel Hobo via Gcc wrote: Dear developers: I find it counterintuitive that if I repeatedly reset a variable by using strcpy with an empty string "" to that variable and then us strcat to add characters to that variable that that seems to lead to a stack overflow. I would expect strcpy to first free the variable, then malloc, then copy the string value into the variable. I think that would be a better interpretation, because it can keep running for quite some time before it overflows and doesn’t really call it. Instead, I got "Illegal instruction: 4". I ended up reimplementing the reset function, implementing it with free and malloc myself, but the way strings have been implemented in C is highly counter-intuitive. In general pointers tend to be bug-prone, but here you would expect this not to happen. I hope you can fix this. Personally, I’m looking into switching to Ada. All the best, Emile M. Hobo - Au fin! Et encore en plus. -
Re: Benchmark recommendations needed
I studied Dhrystone about 30 years ago and found it had a number of flaws back then. For example, most of the loops in the code are only executed 1-3 times, which minimizes the value of hoisting values out of inner loops. Read the Dhrystone wikipedia article for more information. Going back to what benchmarks might be useful... you might consider the Livermore Loops http://www.netlib.org/benchmark/livermorec These are 24 kernels (tight loops) originally in Fortran but ported to C 30 years ago. They are reasonably representative of floating point computational kernels. They are available without a fee. Even if you have no interest in floating point computation for your target architecture, examining the assembly output of these kernels will be helpful in finding where your port of gcc is doing well and where the machine architecture input to the various optimizer phases need some tuning. You also might review that code and write some modest test loops of your own for integer code patterns. Developing good benchmarks is a skill which requires the developer to know the intended purpose of the benchmark. I.e. is our goal to compare optimizer implementations? or different architectures (i.e. arm vs x86)? or different implementations of an architecture (i.e. intel vs amd or early x86 vs current x86) or ... well, you get the idea. Good luck, - Patrick McGehearty On 2/22/2022 3:49 PM, Paul Koning via Gcc wrote: On Feb 22, 2022, at 4:26 PM, Gary Oblock via Gcc wrote: Andras, The whole point of benchmarks is to judge a processor's performance. That being said, just crippling GCC is not reasonable because processors must be judged in the appropriate context and that includes the current state of the art compiler technology. If you have a new processor I'd benchmark it using the applications you built it for. Exactly. Part of what you want to see is that GCC optimizes well for the new machine, i.e., that there aren't artifacts of the machine description that get in the way of optimization. So you'd want to use applications that are good exercises not just of the code generator but also the optimizer. Dhrystone isn't really that, because it has evolved into mostly an optimizer test, not a machine or code generator test. paul
Re: Validation of adding left shift stmt at GIMPLE - [Custom GCC plugin]]
On Tue, Feb 22, 2022 at 2:10 PM Shubham Narlawar wrote: > > On Tue, Feb 22, 2022 at 3:55 PM Richard Biener > wrote: > > > > On Tue, Feb 22, 2022 at 8:38 AM Shubham Narlawar > > wrote: > > > > > > On Mon, Feb 21, 2022 at 1:02 PM Richard Biener > > > wrote: > > > > > > > > On Sun, Feb 20, 2022 at 11:44 PM Andrew Pinski via Gcc > > > > wrote: > > > > > > > > > > On Sun, Feb 20, 2022 at 10:45 AM Shubham Narlawar > > > > > wrote: > > > > > > > > > > > > On Sat, Feb 19, 2022 at 1:15 AM Andrew Pinski > > > > > > wrote: > > > > > > > > > > > > > > On Fri, Feb 18, 2022 at 11:04 AM Shubham Narlawar via Gcc > > > > > > > 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 - > > > > > > > > > > > > > > > >: > > > > > > > > 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 > > > > > >: > > > 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_bu