[Mesa-dev] [PATCH 11/11] r600/radeonsi/clover: always assume PIPE_SHADER_IR_NATIVE for clover

[Marek Olšák]

On Fri, Feb 2, 2018 at 8:07 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>
>
> On 02/02/18 17:21, Timothy Arceri wrote:
>>
>> On 02/02/18 16:38, Jan Vesely wrote:
>>>
>>> On Fri, 2018-02-02 at 15:03 +1100, Timothy Arceri wrote:
>>>>
>>>> When PIPE_SHADER_IR_LLVM existed this query made sense but now it
>>>> always returns PIPE_SHADER_IR_NATIVE. Also it is now conlicting
>>>> with PIPE_SHADER_IR_NIR for compute shaders, so just assume this
>>>> is always PIPE_SHADER_IR_NATIVE for clover.
>>>>
>>>> This change indirectly enables NIR support for compute shaders
>>>> on radeonsi.
>>>> ---
>>>>   src/gallium/drivers/r600/r600_pipe.c              | 6 +-----
>>>>   src/gallium/drivers/radeonsi/si_get.c             | 3 ---
>>>>   src/gallium/state_trackers/clover/core/device.cpp | 3 +--
>>>>   3 files changed, 2 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/src/gallium/drivers/r600/r600_pipe.c
>>>> b/src/gallium/drivers/r600/r600_pipe.c
>>>> index 6c021e568d..287fe497ca 100644
>>>> --- a/src/gallium/drivers/r600/r600_pipe.c
>>>> +++ b/src/gallium/drivers/r600/r600_pipe.c
>>>> @@ -595,11 +595,7 @@ static int r600_get_shader_param(struct
>>>> pipe_screen* pscreen,
>>>>       case PIPE_SHADER_CAP_MAX_SAMPLER_VIEWS:
>>>>           return 16;
>>>>           case PIPE_SHADER_CAP_PREFERRED_IR:
>>>> -        if (shader == PIPE_SHADER_COMPUTE) {
>>>> -            return PIPE_SHADER_IR_NATIVE;
>>>> -        } else {
>>>> -            return PIPE_SHADER_IR_TGSI;
>>>> -        }
>>>> +        return PIPE_SHADER_IR_TGSI;
>>>>       case PIPE_SHADER_CAP_SUPPORTED_IRS:
>>>>           if (rscreen->b.family >= CHIP_CEDAR)
>>>>               return (1 << PIPE_SHADER_IR_TGSI);
>>>> diff --git a/src/gallium/drivers/radeonsi/si_get.c
>>>> b/src/gallium/drivers/radeonsi/si_get.c
>>>> index 40f4cc267e..46cc190db1 100644
>>>> --- a/src/gallium/drivers/radeonsi/si_get.c
>>>> +++ b/src/gallium/drivers/radeonsi/si_get.c
>>>> @@ -391,9 +391,6 @@ static int si_get_shader_param(struct pipe_screen*
>>>> pscreen,
>>>>           break;
>>>>       case PIPE_SHADER_COMPUTE:
>>>>           switch (param) {
>>>> -        case PIPE_SHADER_CAP_PREFERRED_IR:
>>>> -            return PIPE_SHADER_IR_NATIVE;
>>>> -
>>>>           case PIPE_SHADER_CAP_SUPPORTED_IRS: {
>>>>               int ir = 1 << PIPE_SHADER_IR_NATIVE;
>>>> diff --git a/src/gallium/state_trackers/clover/core/device.cpp
>>>> b/src/gallium/state_trackers/clover/core/device.cpp
>>>> index 9dd7eed3f1..116f0c7604 100644
>>>> --- a/src/gallium/state_trackers/clover/core/device.cpp
>>>> +++ b/src/gallium/state_trackers/clover/core/device.cpp
>>>> @@ -243,8 +243,7 @@ device::vendor_name() const {
>>>>   enum pipe_shader_ir
>>>>   device::ir_format() const {
>>>> -   return (enum pipe_shader_ir) pipe->get_shader_param(
>>>> -      pipe, PIPE_SHADER_COMPUTE, PIPE_SHADER_CAP_PREFERRED_IR);
>>>> +   return PIPE_SHADER_IR_NATIVE;
>>>
>>>
>>> This looks like it forces IR_NATIVE for absolutely everybody. Why
>>> should other devices use IR_NATIVE just because radeonsi wants
>>> experimental NIR for GL?
>>
>>
>> It's not just about experimental NIR. If say freedreno or any other nir
>> driver was to use clover they would have the same problem.
>
>
> Well it would be return PIPE_SHADER_IR_NIR here at least.
>
>> Also I'm 99% certain that no drivers other than radeonsi and r600 even
>> care what this value is. All other drivers have this set to
>> PIPE_SHADER_IR_TGSI does that make anymore sense?
>
>
> I see no there is indeed a tgsi path for clover. I guess maybe this change
> has more impact than I first thought.
>
> Happy to here suggestions for solving the current conflict in uses of
> PIPE_SHADER_CAP_PREFERRED_IR.

If clover wants a CAP, it can have its own CAP.

Or this CAP can be removed and everybody can just use
PIPE_SHADER_CAP_SUPPORTED_IRS. st/mesa can prefer NIR if supported,
and clover can prefer native if supported. Problem solved.

Marek