Re: [Mesa-dev] [PATCH] amdgpu/addrlib: use initialization list in addrobject

[Jeremy Newton]

Dully noted, thanks for the heads up.

On July 25, 2017 12:50:22 PM EDT, "Marek Olšák"  wrote:
>Hi Jeremy,
>
>Note that addrlib is imported from our internal repository. Any
>changes to addrlib in Mesa may make later updates from the internal
>repository more difficult. It would be preferable to apply changes to
>the internal addrlib first and Mesa would get them on the next addrlib
>update.
>
>I personally don't bother cleaning up addrlib in Mesa except maybe
>fixing warnings.
>
>Marek
>
>On Mon, Jul 24, 2017 at 7:17 PM, Jeremy Newton 
>wrote:
>> Fair enough.
>>
>> Although from my tests with x86-64 GCC 6.3 (Fedora 25), it did
>produce a
>> slightly smaller binary with this patch.
>>
>> With that said, I only used whatever the default optimization flags
>are, and
>> I didn't do a diff on a disasm to see what actually changed.
>>
>> On Mon, Jul 24, 2017 at 1:03 PM, Nicolai Hähnle 
>wrote:
>>>
>>> On 23.07.2017 18:24, Mystro256 wrote:
>>> > ---
>>> >   src/amd/addrlib/core/addrobject.cpp | 4 ++--
>>> >   1 file changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/src/amd/addrlib/core/addrobject.cpp
>>> > b/src/amd/addrlib/core/addrobject.cpp
>>> > index dcdb1bf..ee2d9a9 100644
>>> > --- a/src/amd/addrlib/core/addrobject.cpp
>>> > +++ b/src/amd/addrlib/core/addrobject.cpp
>>> > @@ -61,9 +61,9 @@ Object::Object()
>>> >   *   Constructor for the Object class.
>>> >
>>> >
>
>>> >   */
>>> > -Object::Object(const Client* pClient)
>>> > +Object::Object(const Client* pClient):
>>> > +m_client (*pClient)
>>> >   {
>>> > -m_client = *pClient;
>>> >   }
>>>
>>> Thanks, but this is really a matter of taste and coding style. It
>should
>>> make no difference for the generated code, and I believe addrlib
>>> generally prefers not to use the initializer list, so NAK on this
>patch.
>>>
>>> Cheers,
>>> Nicolai
>>>
>>>
>>> >
>>> >   /**
>>> >
>>>
>>>
>>> --
>>> Lerne, wie die Welt wirklich ist,
>>> Aber vergiss niemals, wie sie sein sollte.
>>
>>
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>___
>mesa-dev mailing list
>mesa-dev@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] amdgpu/addrlib: use initialization list in addrobject

[Marek Olšák]

Hi Jeremy,

Note that addrlib is imported from our internal repository. Any
changes to addrlib in Mesa may make later updates from the internal
repository more difficult. It would be preferable to apply changes to
the internal addrlib first and Mesa would get them on the next addrlib
update.

I personally don't bother cleaning up addrlib in Mesa except maybe
fixing warnings.

Marek

On Mon, Jul 24, 2017 at 7:17 PM, Jeremy Newton  wrote:
> Fair enough.
>
> Although from my tests with x86-64 GCC 6.3 (Fedora 25), it did produce a
> slightly smaller binary with this patch.
>
> With that said, I only used whatever the default optimization flags are, and
> I didn't do a diff on a disasm to see what actually changed.
>
> On Mon, Jul 24, 2017 at 1:03 PM, Nicolai Hähnle  wrote:
>>
>> On 23.07.2017 18:24, Mystro256 wrote:
>> > ---
>> >   src/amd/addrlib/core/addrobject.cpp | 4 ++--
>> >   1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/amd/addrlib/core/addrobject.cpp
>> > b/src/amd/addrlib/core/addrobject.cpp
>> > index dcdb1bf..ee2d9a9 100644
>> > --- a/src/amd/addrlib/core/addrobject.cpp
>> > +++ b/src/amd/addrlib/core/addrobject.cpp
>> > @@ -61,9 +61,9 @@ Object::Object()
>> >   *   Constructor for the Object class.
>> >
>> > 
>> >   */
>> > -Object::Object(const Client* pClient)
>> > +Object::Object(const Client* pClient):
>> > +m_client (*pClient)
>> >   {
>> > -m_client = *pClient;
>> >   }
>>
>> Thanks, but this is really a matter of taste and coding style. It should
>> make no difference for the generated code, and I believe addrlib
>> generally prefers not to use the initializer list, so NAK on this patch.
>>
>> Cheers,
>> Nicolai
>>
>>
>> >
>> >   /**
>> >
>>
>>
>> --
>> Lerne, wie die Welt wirklich ist,
>> Aber vergiss niemals, wie sie sein sollte.
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] amdgpu/addrlib: use initialization list in addrobject

[Jeremy Newton]

Fair enough.

Although from my tests with x86-64 GCC 6.3 (Fedora 25), it did produce a
slightly smaller binary with this patch.

With that said, I only used whatever the default optimization flags are,
and I didn't do a diff on a disasm to see what actually changed.

On Mon, Jul 24, 2017 at 1:03 PM, Nicolai Hähnle  wrote:

> On 23.07.2017 18:24, Mystro256 wrote:
> > ---
> >   src/amd/addrlib/core/addrobject.cpp | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/amd/addrlib/core/addrobject.cpp b/src/amd/addrlib/core/
> addrobject.cpp
> > index dcdb1bf..ee2d9a9 100644
> > --- a/src/amd/addrlib/core/addrobject.cpp
> > +++ b/src/amd/addrlib/core/addrobject.cpp
> > @@ -61,9 +61,9 @@ Object::Object()
> >   *   Constructor for the Object class.
> >   
> 
> >   */
> > -Object::Object(const Client* pClient)
> > +Object::Object(const Client* pClient):
> > +m_client (*pClient)
> >   {
> > -m_client = *pClient;
> >   }
>
> Thanks, but this is really a matter of taste and coding style. It should
> make no difference for the generated code, and I believe addrlib
> generally prefers not to use the initializer list, so NAK on this patch.
>
> Cheers,
> Nicolai
>
>
> >
> >   /**
> >
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] amdgpu/addrlib: use initialization list in addrobject

[Nicolai Hähnle]

On 23.07.2017 18:24, Mystro256 wrote:

---
  src/amd/addrlib/core/addrobject.cpp | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/amd/addrlib/core/addrobject.cpp 
b/src/amd/addrlib/core/addrobject.cpp
index dcdb1bf..ee2d9a9 100644
--- a/src/amd/addrlib/core/addrobject.cpp
+++ b/src/amd/addrlib/core/addrobject.cpp
@@ -61,9 +61,9 @@ Object::Object()
  *   Constructor for the Object class.
  

  */
-Object::Object(const Client* pClient)
+Object::Object(const Client* pClient):
+m_client (*pClient)
  {
-m_client = *pClient;
  }


Thanks, but this is really a matter of taste and coding style. It should 
make no difference for the generated code, and I believe addrlib 
generally prefers not to use the initializer list, so NAK on this patch.


Cheers,
Nicolai


  
  /**





--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] amdgpu/addrlib: use initialization list in addrobject

[Mystro256]

Just a small patch to save a small amount of cycles, as it would be better
to use the initialization list here.

Bare with me if I sent this wrong, I don't typically use git-send-email.

Mystro256 (1):
  amdgpu/addrlib: use initialization list in addrobject

 src/amd/addrlib/core/addrobject.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.9.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] amdgpu/addrlib: use initialization list in addrobject

[Mystro256]

---
 src/amd/addrlib/core/addrobject.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/amd/addrlib/core/addrobject.cpp 
b/src/amd/addrlib/core/addrobject.cpp
index dcdb1bf..ee2d9a9 100644
--- a/src/amd/addrlib/core/addrobject.cpp
+++ b/src/amd/addrlib/core/addrobject.cpp
@@ -61,9 +61,9 @@ Object::Object()
 *   Constructor for the Object class.
 

 */
-Object::Object(const Client* pClient)
+Object::Object(const Client* pClient):
+m_client (*pClient)
 {
-m_client = *pClient;
 }
 
 /**
-- 
2.9.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev