[PATCH 0/2] strncmp builtin expansion improvement

[Aaron Sawdey]

On Mon, 2016-11-07 at 15:26 +0100, Richard Biener wrote:
> Your patchset doesn't contain a testcase so I really wonder which
> case
> we know the string length but it is not constant.
> 
> Yes, there's COND_EXPR handling in c_strlen but that should be mostly
> dead code -- the real code should be using get_maxval_strlen or
> get_range_strlen but c_strlen does not use those.
> 
> Ideally the str optabs would get profile data and alignment similar
> to
> the mem ones.
> 
> Care to share a testcase?

I think I haven't explained this well. The case I am interested in is
where the string arguments are indeed of unknown length, but the length
argument to strncmp is a constant. This is the case that I'm attempting
to address with this patch series.

This is from the strncmp-1.c test case, but modified for a constant
length argument to strncmp.

#include <string.h>
#include <stddef.h>
#include <stdlib.h>

void
test (const unsigned char *s1, const unsigned char *s2, int expected)
{
  register int value = strncmp ((char *) s1, (char *) s2, 5);

  if (expected < 0 && value >= 0)
    abort ();
  else if (expected == 0 && value != 0)
    abort ();
  else if (expected > 0 && value <= 0)
    abort ();
}

I added this small bit to builtins.c so we can see what happens:

Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c      (revision 241911)
+++ gcc/builtins.c      (working copy)
@@ -67,6 +67,7 @@
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
 #include "gimple-fold.h"
+#include "print-tree.h"
 
 
 struct target_builtins default_target_builtins;
@@ -3932,6 +3933,9 @@
     len1 = c_strlen (arg1, 1);
     len2 = c_strlen (arg2, 1);
 
+Â Â Â Â printf("len1 = %p len2 = %p\n",(void*)len1,(void*)len2);
+Â Â Â Â debug_tree(arg3);
+
     if (len1)
       len1 = size_binop_loc (loc, PLUS_EXPR, ssize_int (1), len1);
     if (len2)

The output then is as follows:

build/gcc/xgcc -B build/gcc -S -O1 strncmp-test.c
len1 = (nil) len2 = (nil)
 <integer_cst 0x7f7ba8a92588 type <integer_type 0x7f7ba8a06150 size_t>
constant 5>

Looking in the .s file you can see that strncmp was not expanded.
However the current code in i386.md for cmpstrnsi does not handle the
case where the 0 byte in both strings may occur before the length given
to strncmp.

test:
.LFB22:
        .cfi_startproc
        pushq   %rbx
        .cfi_def_cfa_offset 16
        .cfi_offset 3, -16
        movl    %edx, %ebx
        movl    $5, %edx
        call    strncmp
        movl    %ebx, %edx

I think it's pretty clear from the code in expand_builtin_strncmp that
if len1 and len2 are both NULL, you end up with len=len2 and then it
returns NULL_RTX.

Thanks,
   Aaron

--Â 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain