Re: [Mesa-dev] [PATCH] nv50/ir: don't consider the main compute function as taking arguments

2019-07-26 Thread Karol Herbst
I think this was there for generic support for functions actually and
that for OpenCL + TGSI the idea was to not inline everything by
default, so return values were handled there as well.

The proper way to handle is, to declare kernel inputs as real inputs,
because kernel inputs are fundamentally different from function
arguments and trying to handle them exactly the same will just result
in pain and fun issues like the VDPAU/VA one.

The correct way to handle that on the TGSI side is to never generate a
MAIN functions out of the actual source, then add a "main" function
which reads the shader IN variables and passes them as arguments to
the entry point called (which should be a named function inside the
TGSI). This way the now new main function has no parameters and no
return value, the world becomes sane and everybody is happy.

That's also how I implemented that for the OpenCL nir path and that
works out quite nicely as now you can just call different entry points
without having to deal with this "if this function is the entry point,
args get passed differently than being a called function" situation.

Anyway, the for the patch itself:
Reviewed-by: Karol Herbst 

On Fri, Jul 26, 2019 at 7:20 AM Ilia Mirkin  wrote:
>
> With OpenCL, kernels can take arguments and return values (?). However
> in practice, there is no more TGSI compute implementation, and even if
> there were, it would probably have named functions and no explicit main.
>
> This improves RA considerably for compute shaders, since temps are not
> kept around as return values.
>
> Signed-off-by: Ilia Mirkin 
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> index 9d0ab336c75..2dd13e70d0e 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -4298,7 +4298,7 @@ Converter::BindArgumentsPass::visit(Function *f)
>}
> }
>
> -   if (func == prog->main && prog->getType() != Program::TYPE_COMPUTE)
> +   if (func == prog->main /* && prog->getType() != Program::TYPE_COMPUTE */)
>return true;
> updatePrototype(::get(f->cfg.getRoot())->liveSet,
> ::buildLiveSets, ::ins);
> --
> 2.21.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

[Mesa-dev] [PATCH] nv50/ir: don't consider the main compute function as taking arguments

2019-07-25 Thread Ilia Mirkin
With OpenCL, kernels can take arguments and return values (?). However
in practice, there is no more TGSI compute implementation, and even if
there were, it would probably have named functions and no explicit main.

This improves RA considerably for compute shaders, since temps are not
kept around as return values.

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index 9d0ab336c75..2dd13e70d0e 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -4298,7 +4298,7 @@ Converter::BindArgumentsPass::visit(Function *f)
   }
}
 
-   if (func == prog->main && prog->getType() != Program::TYPE_COMPUTE)
+   if (func == prog->main /* && prog->getType() != Program::TYPE_COMPUTE */)
   return true;
updatePrototype(::get(f->cfg.getRoot())->liveSet,
::buildLiveSets, ::ins);
-- 
2.21.0

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