Re: [Mesa-dev] [PATCH] radeonsi: disable sinking common instructions down to the end block

2017-03-23 Thread Marek Olšák 
I'll send a fix.

Marek

On Thu, Mar 23, 2017 at 12:38 PM, Nicolai Hähnle  wrote:
> On 22.03.2017 21:00, Andy Furniss wrote:
>>
>> Samuel Pitoiset wrote:
>>
>> This patch has a minor issue for me using radeonsi on tonga.
>>
>> ffmpeg vaapi hwdecode by cli or via mpv now produces the message
>>
>> mesa: for the -simplifycfg-sink-common option: may only occur zero or
>> one times!
>
>
> I guess that setup ends up loading both radeonsi_dri.so and
> radeonsi_drv_video.so, and so the command line is set twice despite the use
> of a once_flag. Any ideas?
>
> Cheers,
> Nicolai
>
>
>>
>>>
>>> On 03/15/2017 02:01 PM, Marek Olšák wrote:

 On Wed, Mar 15, 2017 at 1:56 PM, Samuel Pitoiset
  wrote:
>
>
>
> On 03/15/2017 01:51 PM, Marek Olšák wrote:
>>
>>
>> On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
>>  wrote:
>>>
>>>
>>> Initially this was a workaround for a bug introduced in LLVM 4.0
>>> in the SimplifyCFG pass that caused image instrinsics to disappear
>>> (because they were badly sunk). Finally, this is a win because it
>>> decreases SGPR spilling and increases the number of waves a bit.
>>>
>>> Although, shader-db results are good I think we might want to
>>> remove it in the future once the issue is fixed. For now, enable
>>> it for LLVM >= 4.0.
>>>
>>> This also fixes a rendering issue with the speedometer in Dirt Rally.
>>>
>>> More information can be found here https://reviews.llvm.org/D26348.
>>>
>>> Thanks to Dave Airlie for the patch.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
>>> Signed-off-by: Samuel Pitoiset 
>>> ---
>>>
>>> For those interested, here's the full shader-db report:
>>>
>>> https://hastebin.com/osepehehat.pas
>>>
>>>  src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> index 5c63b732b3..4fb7a126e4 100644
>>> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> @@ -40,6 +40,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>
>>>  /* Data for if/else/endif and bgnloop/endloop control flow
>>> structures.
>>>   */
>>> @@ -124,6 +125,11 @@ static void init_amdgpu_target()
>>> LLVMInitializeAMDGPUTarget();
>>> LLVMInitializeAMDGPUTargetMC();
>>> LLVMInitializeAMDGPUAsmPrinter();
>>> +
>>> +#if HAVE_LLVM >= 0x0400
>>> +   const char *argv[2] = {"mesa",
>>> "-simplifycfg-sink-common=false"};
>>> +   LLVMParseCommandLineOptions(2, argv, NULL);
>>> +#endif
>>
>>
>>
>> Would it possible to do:
>> if (HAVE_LLVM >= 0x0400) {
>> ...
>> }
>>
>> Also, Cc stable?
>
>
>
> Your call. But if the issue is fixed at some point in 4.0.1 or 5.0 we
> will
> need to backport one more patch.


 Since this only disables the problematic optimization, we can keep it
 in stable and we won't have to backport anything else. (optimizations
 are usually not accepted in stable)
>>>
>>>
>>> Fine by me. I will add the cc tag and the FIXME comment before pushing.
>>> Thanks.
>>>

 Marek

>>> ___
>>> 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
>
>
>
> --
> 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] radeonsi: disable sinking common instructions down to the end block

2017-03-23 Thread Samuel Pitoiset 



On 03/23/2017 12:38 PM, Nicolai Hähnle wrote:

On 22.03.2017 21:00, Andy Furniss wrote:

Samuel Pitoiset wrote:

This patch has a minor issue for me using radeonsi on tonga.

ffmpeg vaapi hwdecode by cli or via mpv now produces the message

mesa: for the -simplifycfg-sink-common option: may only occur zero or
one times!


I guess that setup ends up loading both radeonsi_dri.so and
radeonsi_drv_video.so, and so the command line is set twice despite the
use of a once_flag. Any ideas?


Yeah, that's probably why the message is displayed.



Cheers,
Nicolai





On 03/15/2017 02:01 PM, Marek Olšák wrote:

On Wed, Mar 15, 2017 at 1:56 PM, Samuel Pitoiset
 wrote:



On 03/15/2017 01:51 PM, Marek Olšák wrote:


On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
 wrote:


Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt
Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 

 /* Data for if/else/endif and bgnloop/endloop control flow
structures.
  */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa",
"-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif



Would it possible to do:
if (HAVE_LLVM >= 0x0400) {
...
}

Also, Cc stable?



Your call. But if the issue is fixed at some point in 4.0.1 or 5.0 we
will
need to backport one more patch.


Since this only disables the problematic optimization, we can keep it
in stable and we won't have to backport anything else. (optimizations
are usually not accepted in stable)


Fine by me. I will add the cc tag and the FIXME comment before pushing.
Thanks.



Marek


___
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] radeonsi: disable sinking common instructions down to the end block

2017-03-23 Thread Nicolai Hähnle 

On 22.03.2017 21:00, Andy Furniss wrote:

Samuel Pitoiset wrote:

This patch has a minor issue for me using radeonsi on tonga.

ffmpeg vaapi hwdecode by cli or via mpv now produces the message

mesa: for the -simplifycfg-sink-common option: may only occur zero or
one times!


I guess that setup ends up loading both radeonsi_dri.so and 
radeonsi_drv_video.so, and so the command line is set twice despite the 
use of a once_flag. Any ideas?


Cheers,
Nicolai





On 03/15/2017 02:01 PM, Marek Olšák wrote:

On Wed, Mar 15, 2017 at 1:56 PM, Samuel Pitoiset
 wrote:



On 03/15/2017 01:51 PM, Marek Olšák wrote:


On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
 wrote:


Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 

 /* Data for if/else/endif and bgnloop/endloop control flow
structures.
  */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa",
"-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif



Would it possible to do:
if (HAVE_LLVM >= 0x0400) {
...
}

Also, Cc stable?



Your call. But if the issue is fixed at some point in 4.0.1 or 5.0 we
will
need to backport one more patch.


Since this only disables the problematic optimization, we can keep it
in stable and we won't have to backport anything else. (optimizations
are usually not accepted in stable)


Fine by me. I will add the cc tag and the FIXME comment before pushing.
Thanks.



Marek


___
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



--
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] radeonsi: disable sinking common instructions down to the end block

2017-03-22 Thread Andy Furniss 

Samuel Pitoiset wrote:

This patch has a minor issue for me using radeonsi on tonga.

ffmpeg vaapi hwdecode by cli or via mpv now produces the message

mesa: for the -simplifycfg-sink-common option: may only occur zero or 
one times!




On 03/15/2017 02:01 PM, Marek Olšák wrote:

On Wed, Mar 15, 2017 at 1:56 PM, Samuel Pitoiset
 wrote:



On 03/15/2017 01:51 PM, Marek Olšák wrote:


On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
 wrote:


Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 

 /* Data for if/else/endif and bgnloop/endloop control flow
structures.
  */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa",
"-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif



Would it possible to do:
if (HAVE_LLVM >= 0x0400) {
...
}

Also, Cc stable?



Your call. But if the issue is fixed at some point in 4.0.1 or 5.0 we
will
need to backport one more patch.


Since this only disables the problematic optimization, we can keep it
in stable and we won't have to backport anything else. (optimizations
are usually not accepted in stable)


Fine by me. I will add the cc tag and the FIXME comment before pushing.
Thanks.



Marek


___
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] radeonsi: disable sinking common instructions down to the end block

2017-03-15 Thread Samuel Pitoiset 



On 03/15/2017 02:01 PM, Marek Olšák wrote:

On Wed, Mar 15, 2017 at 1:56 PM, Samuel Pitoiset
 wrote:



On 03/15/2017 01:51 PM, Marek Olšák wrote:


On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
 wrote:


Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 

 /* Data for if/else/endif and bgnloop/endloop control flow structures.
  */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif



Would it possible to do:
if (HAVE_LLVM >= 0x0400) {
...
}

Also, Cc stable?



Your call. But if the issue is fixed at some point in 4.0.1 or 5.0 we will
need to backport one more patch.


Since this only disables the problematic optimization, we can keep it
in stable and we won't have to backport anything else. (optimizations
are usually not accepted in stable)


Fine by me. I will add the cc tag and the FIXME comment before pushing.
Thanks.



Marek


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

Re: [Mesa-dev] [PATCH] radeonsi: disable sinking common instructions down to the end block

2017-03-15 Thread Marek Olšák 
On Wed, Mar 15, 2017 at 1:56 PM, Samuel Pitoiset
 wrote:
>
>
> On 03/15/2017 01:51 PM, Marek Olšák wrote:
>>
>> On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
>>  wrote:
>>>
>>> Initially this was a workaround for a bug introduced in LLVM 4.0
>>> in the SimplifyCFG pass that caused image instrinsics to disappear
>>> (because they were badly sunk). Finally, this is a win because it
>>> decreases SGPR spilling and increases the number of waves a bit.
>>>
>>> Although, shader-db results are good I think we might want to
>>> remove it in the future once the issue is fixed. For now, enable
>>> it for LLVM >= 4.0.
>>>
>>> This also fixes a rendering issue with the speedometer in Dirt Rally.
>>>
>>> More information can be found here https://reviews.llvm.org/D26348.
>>>
>>> Thanks to Dave Airlie for the patch.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
>>> Signed-off-by: Samuel Pitoiset 
>>> ---
>>>
>>> For those interested, here's the full shader-db report:
>>>
>>> https://hastebin.com/osepehehat.pas
>>>
>>>  src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> index 5c63b732b3..4fb7a126e4 100644
>>> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
>>> @@ -40,6 +40,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>
>>>  /* Data for if/else/endif and bgnloop/endloop control flow structures.
>>>   */
>>> @@ -124,6 +125,11 @@ static void init_amdgpu_target()
>>> LLVMInitializeAMDGPUTarget();
>>> LLVMInitializeAMDGPUTargetMC();
>>> LLVMInitializeAMDGPUAsmPrinter();
>>> +
>>> +#if HAVE_LLVM >= 0x0400
>>> +   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
>>> +   LLVMParseCommandLineOptions(2, argv, NULL);
>>> +#endif
>>
>>
>> Would it possible to do:
>> if (HAVE_LLVM >= 0x0400) {
>> ...
>> }
>>
>> Also, Cc stable?
>
>
> Your call. But if the issue is fixed at some point in 4.0.1 or 5.0 we will
> need to backport one more patch.

Since this only disables the problematic optimization, we can keep it
in stable and we won't have to backport anything else. (optimizations
are usually not accepted in stable)

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

Re: [Mesa-dev] [PATCH] radeonsi: disable sinking common instructions down to the end block

2017-03-15 Thread Samuel Pitoiset 



On 03/15/2017 01:51 PM, Marek Olšák wrote:

On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
 wrote:

Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c 
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 

 /* Data for if/else/endif and bgnloop/endloop control flow structures.
  */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif


Would it possible to do:
if (HAVE_LLVM >= 0x0400) {
...
}

Also, Cc stable?


Your call. But if the issue is fixed at some point in 4.0.1 or 5.0 we 
will need to backport one more patch.




With those done:
Reviewed-by: Marek Olšák 

Marek




 }

 static once_flag init_amdgpu_target_once_flag = ONCE_FLAG_INIT;
--
2.12.0

___
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] radeonsi: disable sinking common instructions down to the end block

2017-03-15 Thread Marek Olšák 
On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
 wrote:
> Initially this was a workaround for a bug introduced in LLVM 4.0
> in the SimplifyCFG pass that caused image instrinsics to disappear
> (because they were badly sunk). Finally, this is a win because it
> decreases SGPR spilling and increases the number of waves a bit.
>
> Although, shader-db results are good I think we might want to
> remove it in the future once the issue is fixed. For now, enable
> it for LLVM >= 4.0.
>
> This also fixes a rendering issue with the speedometer in Dirt Rally.
>
> More information can be found here https://reviews.llvm.org/D26348.
>
> Thanks to Dave Airlie for the patch.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
> Signed-off-by: Samuel Pitoiset 
> ---
>
> For those interested, here's the full shader-db report:
>
> https://hastebin.com/osepehehat.pas
>
>  src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c 
> b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
> index 5c63b732b3..4fb7a126e4 100644
> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
> @@ -40,6 +40,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  /* Data for if/else/endif and bgnloop/endloop control flow structures.
>   */
> @@ -124,6 +125,11 @@ static void init_amdgpu_target()
> LLVMInitializeAMDGPUTarget();
> LLVMInitializeAMDGPUTargetMC();
> LLVMInitializeAMDGPUAsmPrinter();
> +
> +#if HAVE_LLVM >= 0x0400
> +   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
> +   LLVMParseCommandLineOptions(2, argv, NULL);
> +#endif

Would it possible to do:
if (HAVE_LLVM >= 0x0400) {
...
}

Also, Cc stable?

With those done:
Reviewed-by: Marek Olšák 

Marek



>  }
>
>  static once_flag init_amdgpu_target_once_flag = ONCE_FLAG_INIT;
> --
> 2.12.0
>
> ___
> 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] radeonsi: disable sinking common instructions down to the end block

2017-03-14 Thread Samuel Pitoiset 



On 03/15/2017 01:59 AM, Matt Arsenault wrote:



On Mar 14, 2017, at 17:21, Samuel Pitoiset  wrote:

Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c 
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
#include 
#include 
#include 
+#include 

/* Data for if/else/endif and bgnloop/endloop control flow structures.
 */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif
}


Probably should have a FIXME that this is a workaround and a link to an open 
bug to really fix these issues


Okay, I will change it locally.



-Matt


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

[Mesa-dev] [PATCH] radeonsi: disable sinking common instructions down to the end block

2017-03-14 Thread Samuel Pitoiset 
Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c 
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Data for if/else/endif and bgnloop/endloop control flow structures.
  */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif
 }
 
 static once_flag init_amdgpu_target_once_flag = ONCE_FLAG_INIT;
-- 
2.12.0

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