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

[Marek Olšák]

On Wed, Mar 15, 2017 at 1:56 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> On 03/15/2017 01:51 PM, Marek Olšák wrote:
>>
>> On Wed, Mar 15, 2017 at 1:21 AM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> 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 <samuel.pitoiset at gmail.com>
>>> ---
>>>
>>> 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 <stdio.h>
>>>  #include <llvm-c/Transforms/IPO.h>
>>>  #include <llvm-c/Transforms/Scalar.h>
>>> +#include <llvm-c/Support.h>
>>>
>>>  /* 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