[patch] explicit_bzero constraint on the destination buffer

[patch] explicit_bzero constraint on the destination buffer

[David CARLIER]

[-- Attachment #1: Type: text/plain, Size: 133 bytes --]

Hi,

This is my first message but here a little patch proposal for
explicit_bzero function.

Hope it s good.

Thanks.

Kind regards.

[-- Attachment #2: 0001-Constraint-on-the-destination-buffer.patch --]
[-- Type: application/octet-stream, Size: 657 bytes --]

From d70ed2be70551e962a5ae3f76197012bda9bcd27 Mon Sep 17 00:00:00 2001
From: David Carlier <dcarlier@afilias.info>
Date: Thu, 28 Jun 2018 15:39:59 +0000
Subject: [PATCH] Constraint on the destination buffer

---
 string/explicit_bzero.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/string/explicit_bzero.c b/string/explicit_bzero.c
index 7ba3f0f62c..c26ac38311 100644
--- a/string/explicit_bzero.c
+++ b/string/explicit_bzero.c
@@ -34,5 +34,5 @@ explicit_bzero (void *s, size_t len)
 {
   memset (s, '\0', len);
   /* Compiler barrier.  */
-  asm volatile ("" ::: "memory");
+  asm volatile ("" :: "r"(s)  : "memory");
 }
-- 
2.14.1

Re: [patch] explicit_bzero constraint on the destination buffer

[Adhemerval Zanella]

On 28/06/2018 13:05, David CARLIER wrote:
> Hi,
> 
> This is my first message but here a little patch proposal for
> explicit_bzero function.
> 
> Hope it s good.
> 
> Thanks.
> 
> Kind regards.
> 

Does it improve the generate code and/or compile support in any meaningful
way? What are you trying to accomplish by passing the memory pointer as an 
input operand? 

At least for x86_64 and aarch64 I am not seeing any code generation 
difference. For powerpc64le it seems it forces the frame-pointer to be
saved/restored on GCC 6.2.1:

$ diff -u original.S patched.S 
--- original.S  2018-06-28 14:09:55.576497556 -0300
+++ patched.S   2018-06-28 14:11:26.914362203 -0300
@@ -12,18 +12,21 @@
        addi 2,2,.TOC.-.LCF0@l
        .localentry     explicit_bzero,.-explicit_bzero
        mflr 0
+       std 31,-8(1)
        mr 5,4
        li 4,0
+       mr 31,3
        std 0,16(1)
-       stdu 1,-32(1)
+       stdu 1,-48(1)
        bl memset
        nop
-       addi 1,1,32
+       addi 1,1,48
        ld 0,16(1)
+       ld 31,-8(1)
        mtlr 0
        blr
        .long 0
-       .byte 0,0,0,1,128,0,0,0
+       .byte 0,0,0,1,128,1,0,0
        .size   explicit_bzero,.-explicit_bzero
        .ident  "GCC: (GNU) 6.2.1 20161129 [gcc-6-branch revision 242956]"
        .section        .note.GNU-stack,"",@progbits

I do not see it on GCC 8.1.1, but I sceptical this change brings any advantage.

[1] https://sourceware.org/ml/libc-alpha/2016-12/msg00537.html

Re: [patch] explicit_bzero constraint on the destination buffer

[David CARLIER]

Ah right indeed it has effect only with older compilers on gcc 7.3
theree is still the additional instruction, default clang 4.0 has but
not clang 6.0 for example. And I ran with those older versions sorry
for the noise.
On Thu, 28 Jun 2018 at 18:16, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 28/06/2018 13:05, David CARLIER wrote:
> > Hi,
> >
> > This is my first message but here a little patch proposal for
> > explicit_bzero function.
> >
> > Hope it s good.
> >
> > Thanks.
> >
> > Kind regards.
> >
>
> Does it improve the generate code and/or compile support in any meaningful
> way? What are you trying to accomplish by passing the memory pointer as an
> input operand?
>
> At least for x86_64 and aarch64 I am not seeing any code generation
> difference. For powerpc64le it seems it forces the frame-pointer to be
> saved/restored on GCC 6.2.1:
>
> $ diff -u original.S patched.S
> --- original.S  2018-06-28 14:09:55.576497556 -0300
> +++ patched.S   2018-06-28 14:11:26.914362203 -0300
> @@ -12,18 +12,21 @@
>         addi 2,2,.TOC.-.LCF0@l
>         .localentry     explicit_bzero,.-explicit_bzero
>         mflr 0
> +       std 31,-8(1)
>         mr 5,4
>         li 4,0
> +       mr 31,3
>         std 0,16(1)
> -       stdu 1,-32(1)
> +       stdu 1,-48(1)
>         bl memset
>         nop
> -       addi 1,1,32
> +       addi 1,1,48
>         ld 0,16(1)
> +       ld 31,-8(1)
>         mtlr 0
>         blr
>         .long 0
> -       .byte 0,0,0,1,128,0,0,0
> +       .byte 0,0,0,1,128,1,0,0
>         .size   explicit_bzero,.-explicit_bzero
>         .ident  "GCC: (GNU) 6.2.1 20161129 [gcc-6-branch revision 242956]"
>         .section        .note.GNU-stack,"",@progbits
>
> I do not see it on GCC 8.1.1, but I sceptical this change brings any advantage.
>
> [1] https://sourceware.org/ml/libc-alpha/2016-12/msg00537.html

Re: [patch] explicit_bzero constraint on the destination buffer

[Adhemerval Zanella]

On 28/06/2018 15:37, David CARLIER wrote:
> Ah right indeed it has effect only with older compilers on gcc 7.3
> theree is still the additional instruction, default clang 4.0 has but
> not clang 6.0 for example. And I ran with those older versions sorry
> for the noise.

Which version exactly are you seeing any code generation differences and
which kind of differences?

> On Thu, 28 Jun 2018 at 18:16, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 28/06/2018 13:05, David CARLIER wrote:
>>> Hi,
>>>
>>> This is my first message but here a little patch proposal for
>>> explicit_bzero function.
>>>
>>> Hope it s good.
>>>
>>> Thanks.
>>>
>>> Kind regards.
>>>
>>
>> Does it improve the generate code and/or compile support in any meaningful
>> way? What are you trying to accomplish by passing the memory pointer as an
>> input operand?
>>
>> At least for x86_64 and aarch64 I am not seeing any code generation
>> difference. For powerpc64le it seems it forces the frame-pointer to be
>> saved/restored on GCC 6.2.1:
>>
>> $ diff -u original.S patched.S
>> --- original.S  2018-06-28 14:09:55.576497556 -0300
>> +++ patched.S   2018-06-28 14:11:26.914362203 -0300
>> @@ -12,18 +12,21 @@
>>         addi 2,2,.TOC.-.LCF0@l
>>         .localentry     explicit_bzero,.-explicit_bzero
>>         mflr 0
>> +       std 31,-8(1)
>>         mr 5,4
>>         li 4,0
>> +       mr 31,3
>>         std 0,16(1)
>> -       stdu 1,-32(1)
>> +       stdu 1,-48(1)
>>         bl memset
>>         nop
>> -       addi 1,1,32
>> +       addi 1,1,48
>>         ld 0,16(1)
>> +       ld 31,-8(1)
>>         mtlr 0
>>         blr
>>         .long 0
>> -       .byte 0,0,0,1,128,0,0,0
>> +       .byte 0,0,0,1,128,1,0,0
>>         .size   explicit_bzero,.-explicit_bzero
>>         .ident  "GCC: (GNU) 6.2.1 20161129 [gcc-6-branch revision 242956]"
>>         .section        .note.GNU-stack,"",@progbits
>>
>> I do not see it on GCC 8.1.1, but I sceptical this change brings any advantage.
>>
>> [1] https://sourceware.org/ml/libc-alpha/2016-12/msg00537.html