Patchwork explicit_bzero constraint on the destination buffer

login
register
mail settings
Submitter Adhemerval Zanella Netto
Date June 28, 2018, 5:15 p.m.
Message ID <5ec6e4bf-614d-647b-f68c-5970516fd00e@linaro.org>
Download mbox | patch
Permalink /patch/28108/
State New
Headers show

Comments

Adhemerval Zanella Netto - June 28, 2018, 5:15 p.m.
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 

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
David CARLIER - June 28, 2018, 6:37 p.m.
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
Adhemerval Zanella Netto - June 28, 2018, 7 p.m.
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

Patch

--- 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