mbox series

[0/1] Adding mempbrk() function to glibc

Message ID 20220613025739.23612-1-brandonfoongyankai@gmail.com
Headers
Series Adding mempbrk() function to glibc |

Message

develop--- via Libc-alpha June 13, 2022, 2:57 a.m. UTC 
  From: Brandon Foong <brandonfoongyankai@gmail.com>

Hi,

This follows up on a previous thread regarding the addition of mempbrk (https://sourceware.org/pipermail/libc-alpha/2022-January/135013.html). I have added a simple implementation of mempbrk, as well as its corresponding tests. 

Brandon Foong (1):
  string: Add basic implementation of mempbrk

 include/string.h      |   1 +
 string/Makefile       |   2 +
 string/Versions       |   1 +
 string/mempbrk.c      |  56 ++++++++++
 string/string.h       |  15 +++
 string/test-mempbrk.c | 242 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 317 insertions(+)
 create mode 100644 string/mempbrk.c
 create mode 100644 string/test-mempbrk.c

Comments

Florian Weimer June 13, 2022, 1:29 p.m. UTC | #1 
* brandonfoongyankai:

> From: Brandon Foong <brandonfoongyankai@gmail.com>
>
> Hi,
>
> This follows up on a previous thread regarding the addition of mempbrk (https://sourceware.org/pipermail/libc-alpha/2022-January/135013.html). I have added a simple implementation of mempbrk, as well as its corresponding tests. 

Andrew Pinski's advice is that this should be fixed in the compiler or
libstdc++:

| Really the issue is:
|   _8 = __builtin_memchr ("eE", _7, 2);
|   if (_8 != 0B)
| 
| 
| Should just be expanded to _7 == 'e' || _7 == 'E' .
| 
| That is:
| int f(char a)
| {
|    return  __builtin_memchr ("e", a, 1) != 0;
| }
| int f1(char a)
| {
|    return a == 'e';
| }
| int g(char a)
| {
|    return  __builtin_memchr ("eE", a, 2) != 0;
| }
| int g1(char a)
| {
|    return a == 'e' || a == 'E';
| }
| 
| f and f1 should produce the same assembly
| and g and g1 should produce the same (similar) assembly.

<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103798>

The challenge with mempbrk (and similar functions in general) is that
the setup cost is so high.  The interface is just isn't very good.  It
works for the programmer, but the compiler-generated code and the
library implementing it should likely use a different interface.

Thanks,
Florian
Jeff Law June 15, 2022, 10:23 p.m. UTC | #2 
On 6/13/2022 7:29 AM, Florian Weimer via Libc-alpha wrote:
> * brandonfoongyankai:
>
>> From: Brandon Foong <brandonfoongyankai@gmail.com>
>>
>> Hi,
>>
>> This follows up on a previous thread regarding the addition of mempbrk (https://sourceware.org/pipermail/libc-alpha/2022-January/135013.html). I have added a simple implementation of mempbrk, as well as its corresponding tests.
> Andrew Pinski's advice is that this should be fixed in the compiler or
> libstdc++:
>
> | Really the issue is:
> |   _8 = __builtin_memchr ("eE", _7, 2);
> |   if (_8 != 0B)
> |
> |
> | Should just be expanded to _7 == 'e' || _7 == 'E' .
> |
> | That is:
> | int f(char a)
> | {
> |    return  __builtin_memchr ("e", a, 1) != 0;
> | }
> | int f1(char a)
> | {
> |    return a == 'e';
> | }
> | int g(char a)
> | {
> |    return  __builtin_memchr ("eE", a, 2) != 0;
> | }
> | int g1(char a)
> | {
> |    return a == 'e' || a == 'E';
> | }
> |
> | f and f1 should produce the same assembly
> | and g and g1 should produce the same (similar) assembly.
>
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103798>
Yea, I'd tend to agree with Andrew on this.  Paying the call overhead 
for something this trivial is crazy.

jeff