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

[Timothy Arceri]

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.

> 
>>
>> Jan
>>
>>>   }
>>>   std::string
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev