diff mbox series

gimple-fold: Transform stp*cpy_chk to strcpy

Message ID 20211110174728.3932930-1-siddhesh@gotplt.org
State Superseded
Headers
Series gimple-fold: Transform stp*cpy_chk to strcpy |

Commit Message

Siddhesh Poyarekar Nov. 10, 2021, 5:47 p.m. UTC 
  Use the ignore flag to transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY
when set.  This transformation will happen in a subsequent fold anyway
but do it right away and save the additional effort.

gcc/ChangeLog:

	* gimple-fold.c (gimple_fold_builtin_stxcpy_chk,
	gimple_fold_builtin_stxncpy_chk): Use BUILT_IN_STRNCPY if return
	value is not used.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 gcc/gimple-fold.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jeff Law Nov. 10, 2021, 8:36 p.m. UTC | #1 
On 11/10/2021 10:47 AM, Siddhesh Poyarekar wrote:
> Use the ignore flag to transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY
> when set.  This transformation will happen in a subsequent fold anyway
> but do it right away and save the additional effort.
>
> gcc/ChangeLog:
>
> 	* gimple-fold.c (gimple_fold_builtin_stxcpy_chk,
> 	gimple_fold_builtin_stxncpy_chk): Use BUILT_IN_STRNCPY if return
> 	value is not used.
OK.

Note we usually want testcases for this kind of stuff too.  It's not 
strictly necessary, but it's hard to go wrong if you're including a test 
that verifies your transformation happens at the point where you expect 
it to happen.


Jeff
Siddhesh Poyarekar Nov. 11, 2021, 5:29 a.m. UTC | #2 
On 11/11/21 02:06, Jeff Law wrote:
> 
> 
> On 11/10/2021 10:47 AM, Siddhesh Poyarekar wrote:
>> Use the ignore flag to transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY
>> when set.  This transformation will happen in a subsequent fold anyway
>> but do it right away and save the additional effort.
>>
>> gcc/ChangeLog:
>>
>>     * gimple-fold.c (gimple_fold_builtin_stxcpy_chk,
>>     gimple_fold_builtin_stxncpy_chk): Use BUILT_IN_STRNCPY if return
>>     value is not used.
> OK.
> 
> Note we usually want testcases for this kind of stuff too.  It's not 
> strictly necessary, but it's hard to go wrong if you're including a test 
> that verifies your transformation happens at the point where you expect 
> it to happen.

I started looking at how I would write a test for this since the folder 
would recursively simplify the statement in the same pass and realized 
that the transformation sequence during lowering is __stpncpy_chk -> 
__strncpy_chk -> __strncpy.  As a result the transformation I'm trying 
to do already will have happened higher up.  I'm testing an updated 
patch that avoids having to fold twice and go directly from 
__stpncpy_chk -> __strncpy.

Siddhesh
diff mbox series

Patch

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 74c9ce4bdc8..cadccfe3010 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -3176,7 +3176,7 @@  gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterator *gsi,
     }
 
   /* If __builtin_st{r,p}cpy_chk is used, assume st{r,p}cpy is available.  */
-  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK
+  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK && !ignore
 			      ? BUILT_IN_STPCPY : BUILT_IN_STRCPY);
   if (!fn)
     return false;
@@ -3220,7 +3220,7 @@  gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
     return false;
 
   /* If __builtin_st{r,p}ncpy_chk is used, assume st{r,p}ncpy is available.  */
-  fn = builtin_decl_explicit (fcode == BUILT_IN_STPNCPY_CHK
+  fn = builtin_decl_explicit (fcode == BUILT_IN_STPNCPY_CHK && !ignore
 			      ? BUILT_IN_STPNCPY : BUILT_IN_STRNCPY);
   if (!fn)
     return false;