[Mesa-dev,7/9] radeonsi: add support for easy opcodes from ARB_gpu_shader5

Submitted by Marek Olšák on March 2, 2015, 11:54 a.m.

Details

Message ID 1425297263-25991-7-git-send-email-maraeo@gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Marek Olšák March 2, 2015, 11:54 a.m.
From: Marek Olšák <marek.olsak@amd.com>

---
 src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
index 385d3ad..034095f 100644
--- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
+++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
@@ -1293,6 +1293,8 @@  void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
 	bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and;
 	bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl;
 	bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit;
+	bld_base->op_actions[TGSI_OPCODE_BREV].emit = build_tgsi_intrinsic_nomem;
+	bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = "llvm.AMDGPU.brev";
 	bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit;
 	bld_base->op_actions[TGSI_OPCODE_CEIL].emit = build_tgsi_intrinsic_nomem;
 	bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil";
@@ -1326,6 +1328,8 @@  void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
 	bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp;
 	bld_base->op_actions[TGSI_OPCODE_IABS].emit = build_tgsi_intrinsic_nomem;
 	bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
+	bld_base->op_actions[TGSI_OPCODE_IBFE].emit = build_tgsi_intrinsic_nomem;
+	bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = "llvm.AMDGPU.bfe.i32";
 	bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
 	bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
 	bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
@@ -1350,6 +1354,8 @@  void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
 	bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod;
 	bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not;
 	bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or;
+	bld_base->op_actions[TGSI_OPCODE_POPC].emit = build_tgsi_intrinsic_nomem;
+	bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32";
 	bld_base->op_actions[TGSI_OPCODE_POW].emit = build_tgsi_intrinsic_nomem;
 	bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32";
 	bld_base->op_actions[TGSI_OPCODE_ROUND].emit = build_tgsi_intrinsic_nomem;
@@ -1389,6 +1395,8 @@  void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
 	bld_base->op_actions[TGSI_OPCODE_TRUNC].emit = build_tgsi_intrinsic_nomem;
 	bld_base->op_actions[TGSI_OPCODE_TRUNC].intr_name = "llvm.AMDGPU.trunc";
 	bld_base->op_actions[TGSI_OPCODE_UADD].emit = emit_uadd;
+	bld_base->op_actions[TGSI_OPCODE_UBFE].emit = build_tgsi_intrinsic_nomem;
+	bld_base->op_actions[TGSI_OPCODE_UBFE].intr_name = "llvm.AMDGPU.bfe.u32";
 	bld_base->op_actions[TGSI_OPCODE_UDIV].emit = emit_udiv;
 	bld_base->op_actions[TGSI_OPCODE_UMAX].emit = build_tgsi_intrinsic_nomem;
 	bld_base->op_actions[TGSI_OPCODE_UMAX].intr_name = "llvm.AMDGPU.umax";

Comments

On Mon, Mar 02, 2015 at 12:54:21PM +0100, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
> 

I'm still unsure whether it's better to use intrinsics or LLVM IR
to implement these.  I will think about this some more.

-Tom

> ---
>  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> index 385d3ad..034095f 100644
> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> @@ -1293,6 +1293,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and;
>  	bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl;
>  	bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit;
> +	bld_base->op_actions[TGSI_OPCODE_BREV].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = "llvm.AMDGPU.brev";
>  	bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit;
>  	bld_base->op_actions[TGSI_OPCODE_CEIL].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil";
> @@ -1326,6 +1328,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp;
>  	bld_base->op_actions[TGSI_OPCODE_IABS].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
> +	bld_base->op_actions[TGSI_OPCODE_IBFE].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = "llvm.AMDGPU.bfe.i32";
>  	bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
>  	bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
>  	bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
> @@ -1350,6 +1354,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod;
>  	bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not;
>  	bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or;
> +	bld_base->op_actions[TGSI_OPCODE_POPC].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32";
>  	bld_base->op_actions[TGSI_OPCODE_POW].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32";
>  	bld_base->op_actions[TGSI_OPCODE_ROUND].emit = build_tgsi_intrinsic_nomem;
> @@ -1389,6 +1395,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_TRUNC].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_TRUNC].intr_name = "llvm.AMDGPU.trunc";
>  	bld_base->op_actions[TGSI_OPCODE_UADD].emit = emit_uadd;
> +	bld_base->op_actions[TGSI_OPCODE_UBFE].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_UBFE].intr_name = "llvm.AMDGPU.bfe.u32";
>  	bld_base->op_actions[TGSI_OPCODE_UDIV].emit = emit_udiv;
>  	bld_base->op_actions[TGSI_OPCODE_UMAX].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_UMAX].intr_name = "llvm.AMDGPU.umax";
> -- 
> 2.1.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Mon, Mar 02, 2015 at 12:54:21PM +0100, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
> 

Hi Marek,

After discussing with Matt, I think we should use LLVM IR rather than
intrinsics for IBFE and UBFE and then add patterns for them either in
the TableGen Files or AMDGPUISelDAGToDAG.cpp.

Using intrinsics for BREV and POPC is fine though.

-Tom

> ---
>  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> index 385d3ad..034095f 100644
> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> @@ -1293,6 +1293,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and;
>  	bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl;
>  	bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit;
> +	bld_base->op_actions[TGSI_OPCODE_BREV].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = "llvm.AMDGPU.brev";
>  	bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit;
>  	bld_base->op_actions[TGSI_OPCODE_CEIL].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil";
> @@ -1326,6 +1328,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp;
>  	bld_base->op_actions[TGSI_OPCODE_IABS].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
> +	bld_base->op_actions[TGSI_OPCODE_IBFE].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = "llvm.AMDGPU.bfe.i32";
>  	bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
>  	bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
>  	bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
> @@ -1350,6 +1354,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod;
>  	bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not;
>  	bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or;
> +	bld_base->op_actions[TGSI_OPCODE_POPC].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32";
>  	bld_base->op_actions[TGSI_OPCODE_POW].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32";
>  	bld_base->op_actions[TGSI_OPCODE_ROUND].emit = build_tgsi_intrinsic_nomem;
> @@ -1389,6 +1395,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>  	bld_base->op_actions[TGSI_OPCODE_TRUNC].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_TRUNC].intr_name = "llvm.AMDGPU.trunc";
>  	bld_base->op_actions[TGSI_OPCODE_UADD].emit = emit_uadd;
> +	bld_base->op_actions[TGSI_OPCODE_UBFE].emit = build_tgsi_intrinsic_nomem;
> +	bld_base->op_actions[TGSI_OPCODE_UBFE].intr_name = "llvm.AMDGPU.bfe.u32";
>  	bld_base->op_actions[TGSI_OPCODE_UDIV].emit = emit_udiv;
>  	bld_base->op_actions[TGSI_OPCODE_UMAX].emit = build_tgsi_intrinsic_nomem;
>  	bld_base->op_actions[TGSI_OPCODE_UMAX].intr_name = "llvm.AMDGPU.umax";
> -- 
> 2.1.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
OK. What about patches 8 an 9?

Marek

On Thu, Mar 5, 2015 at 8:30 PM, Tom Stellard <tom@stellard.net> wrote:
> On Mon, Mar 02, 2015 at 12:54:21PM +0100, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>
> Hi Marek,
>
> After discussing with Matt, I think we should use LLVM IR rather than
> intrinsics for IBFE and UBFE and then add patterns for them either in
> the TableGen Files or AMDGPUISelDAGToDAG.cpp.
>
> Using intrinsics for BREV and POPC is fine though.
>
> -Tom
>
>> ---
>>  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>> index 385d3ad..034095f 100644
>> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>> @@ -1293,6 +1293,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>       bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and;
>>       bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl;
>>       bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit;
>> +     bld_base->op_actions[TGSI_OPCODE_BREV].emit = build_tgsi_intrinsic_nomem;
>> +     bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = "llvm.AMDGPU.brev";
>>       bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit;
>>       bld_base->op_actions[TGSI_OPCODE_CEIL].emit = build_tgsi_intrinsic_nomem;
>>       bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil";
>> @@ -1326,6 +1328,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>       bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp;
>>       bld_base->op_actions[TGSI_OPCODE_IABS].emit = build_tgsi_intrinsic_nomem;
>>       bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
>> +     bld_base->op_actions[TGSI_OPCODE_IBFE].emit = build_tgsi_intrinsic_nomem;
>> +     bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = "llvm.AMDGPU.bfe.i32";
>>       bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
>>       bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
>>       bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
>> @@ -1350,6 +1354,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>       bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod;
>>       bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not;
>>       bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or;
>> +     bld_base->op_actions[TGSI_OPCODE_POPC].emit = build_tgsi_intrinsic_nomem;
>> +     bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32";
>>       bld_base->op_actions[TGSI_OPCODE_POW].emit = build_tgsi_intrinsic_nomem;
>>       bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32";
>>       bld_base->op_actions[TGSI_OPCODE_ROUND].emit = build_tgsi_intrinsic_nomem;
>> @@ -1389,6 +1395,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>       bld_base->op_actions[TGSI_OPCODE_TRUNC].emit = build_tgsi_intrinsic_nomem;
>>       bld_base->op_actions[TGSI_OPCODE_TRUNC].intr_name = "llvm.AMDGPU.trunc";
>>       bld_base->op_actions[TGSI_OPCODE_UADD].emit = emit_uadd;
>> +     bld_base->op_actions[TGSI_OPCODE_UBFE].emit = build_tgsi_intrinsic_nomem;
>> +     bld_base->op_actions[TGSI_OPCODE_UBFE].intr_name = "llvm.AMDGPU.bfe.u32";
>>       bld_base->op_actions[TGSI_OPCODE_UDIV].emit = emit_udiv;
>>       bld_base->op_actions[TGSI_OPCODE_UMAX].emit = build_tgsi_intrinsic_nomem;
>>       bld_base->op_actions[TGSI_OPCODE_UMAX].intr_name = "llvm.AMDGPU.umax";
>> --
>> 2.1.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Tue, Mar 10, 2015 at 12:42:38PM +0100, Marek Olšák wrote:
> OK. What about patches 8 an 9?
> 

I think the intrinsics in 9 are OK, but 8 should be using LLVM IR.

-Tom

> Marek
> 
> On Thu, Mar 5, 2015 at 8:30 PM, Tom Stellard <tom@stellard.net> wrote:
> > On Mon, Mar 02, 2015 at 12:54:21PM +0100, Marek Olšák wrote:
> >> From: Marek Olšák <marek.olsak@amd.com>
> >>
> >
> > Hi Marek,
> >
> > After discussing with Matt, I think we should use LLVM IR rather than
> > intrinsics for IBFE and UBFE and then add patterns for them either in
> > the TableGen Files or AMDGPUISelDAGToDAG.cpp.
> >
> > Using intrinsics for BREV and POPC is fine though.
> >
> > -Tom
> >
> >> ---
> >>  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >> index 385d3ad..034095f 100644
> >> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> >> @@ -1293,6 +1293,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
> >>       bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and;
> >>       bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl;
> >>       bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit;
> >> +     bld_base->op_actions[TGSI_OPCODE_BREV].emit = build_tgsi_intrinsic_nomem;
> >> +     bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = "llvm.AMDGPU.brev";
> >>       bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit;
> >>       bld_base->op_actions[TGSI_OPCODE_CEIL].emit = build_tgsi_intrinsic_nomem;
> >>       bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil";
> >> @@ -1326,6 +1328,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
> >>       bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp;
> >>       bld_base->op_actions[TGSI_OPCODE_IABS].emit = build_tgsi_intrinsic_nomem;
> >>       bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
> >> +     bld_base->op_actions[TGSI_OPCODE_IBFE].emit = build_tgsi_intrinsic_nomem;
> >> +     bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = "llvm.AMDGPU.bfe.i32";
> >>       bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
> >>       bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
> >>       bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
> >> @@ -1350,6 +1354,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
> >>       bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod;
> >>       bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not;
> >>       bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or;
> >> +     bld_base->op_actions[TGSI_OPCODE_POPC].emit = build_tgsi_intrinsic_nomem;
> >> +     bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32";
> >>       bld_base->op_actions[TGSI_OPCODE_POW].emit = build_tgsi_intrinsic_nomem;
> >>       bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32";
> >>       bld_base->op_actions[TGSI_OPCODE_ROUND].emit = build_tgsi_intrinsic_nomem;
> >> @@ -1389,6 +1395,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
> >>       bld_base->op_actions[TGSI_OPCODE_TRUNC].emit = build_tgsi_intrinsic_nomem;
> >>       bld_base->op_actions[TGSI_OPCODE_TRUNC].intr_name = "llvm.AMDGPU.trunc";
> >>       bld_base->op_actions[TGSI_OPCODE_UADD].emit = emit_uadd;
> >> +     bld_base->op_actions[TGSI_OPCODE_UBFE].emit = build_tgsi_intrinsic_nomem;
> >> +     bld_base->op_actions[TGSI_OPCODE_UBFE].intr_name = "llvm.AMDGPU.bfe.u32";
> >>       bld_base->op_actions[TGSI_OPCODE_UDIV].emit = emit_udiv;
> >>       bld_base->op_actions[TGSI_OPCODE_UMAX].emit = build_tgsi_intrinsic_nomem;
> >>       bld_base->op_actions[TGSI_OPCODE_UMAX].intr_name = "llvm.AMDGPU.umax";
> >> --
> >> 2.1.0
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
It seems overkill to use LLVM IR for BFE. There are several ways to
express BFE(value, offset, bits) exactly, for example:

# if bits == 0:
#     0
# else if offset + bits < 32:
#     (value << (32 - offset - bits)) >> (32 - bits)
# else:
#     value >> offset

Which can be simplified to either:

# both = offset + bits
# if bits == 0:
#     0
# else if both < 32:
#     (value << (32 - both)) >> (32 - bits)
# else:
#     value >> offset

Or:

# if bits == 0:
#     0
# else if offset + bits < 32:
#     inv_bits = 32 - bits
#     (value << (inv_bits - offset)) >> inv_bits
# else:
#     value >> offset

All 3 variants should use ASHR if the signed version is required.
Alternatively, one can use this as well:

# if bits == 32:
#     value
# else:
#     (value >> offset) & ((1 << bits) - 1)

And an explicit sign extension should be used for the signed version.

The first three are so complex that I doubt it would be easy to
recognize them and match them precisely. They are also very unlikely
to appear open-coded in real applications, therefore, expanding them
and then matching them again seems like a huge waste of time. Apps
will likely just use LSHR+AND, which extracts a bitfield, but it's not
the same as BFE, so BFE can't be used for that. (if the second operand
of AND is a constant expression, then BFE can indeed be used)

I agree that we could use LLVM IR for simple instructions like BFM and
BFI, but I doubt it's a good idea for complex instructions like BFE,
and any small optimization that changes the expression can break the
matching.

Marek


On Tue, Mar 10, 2015 at 3:30 PM, Tom Stellard <tom@stellard.net> wrote:
> On Tue, Mar 10, 2015 at 12:42:38PM +0100, Marek Olšák wrote:
>> OK. What about patches 8 an 9?
>>
>
> I think the intrinsics in 9 are OK, but 8 should be using LLVM IR.
>
> -Tom
>
>> Marek
>>
>> On Thu, Mar 5, 2015 at 8:30 PM, Tom Stellard <tom@stellard.net> wrote:
>> > On Mon, Mar 02, 2015 at 12:54:21PM +0100, Marek Olšák wrote:
>> >> From: Marek Olšák <marek.olsak@amd.com>
>> >>
>> >
>> > Hi Marek,
>> >
>> > After discussing with Matt, I think we should use LLVM IR rather than
>> > intrinsics for IBFE and UBFE and then add patterns for them either in
>> > the TableGen Files or AMDGPUISelDAGToDAG.cpp.
>> >
>> > Using intrinsics for BREV and POPC is fine though.
>> >
>> > -Tom
>> >
>> >> ---
>> >>  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 ++++++++
>> >>  1 file changed, 8 insertions(+)
>> >>
>> >> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>> >> index 385d3ad..034095f 100644
>> >> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>> >> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>> >> @@ -1293,6 +1293,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>> >>       bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and;
>> >>       bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl;
>> >>       bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit;
>> >> +     bld_base->op_actions[TGSI_OPCODE_BREV].emit = build_tgsi_intrinsic_nomem;
>> >> +     bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = "llvm.AMDGPU.brev";
>> >>       bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit;
>> >>       bld_base->op_actions[TGSI_OPCODE_CEIL].emit = build_tgsi_intrinsic_nomem;
>> >>       bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil";
>> >> @@ -1326,6 +1328,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>> >>       bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp;
>> >>       bld_base->op_actions[TGSI_OPCODE_IABS].emit = build_tgsi_intrinsic_nomem;
>> >>       bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
>> >> +     bld_base->op_actions[TGSI_OPCODE_IBFE].emit = build_tgsi_intrinsic_nomem;
>> >> +     bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = "llvm.AMDGPU.bfe.i32";
>> >>       bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
>> >>       bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
>> >>       bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
>> >> @@ -1350,6 +1354,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>> >>       bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod;
>> >>       bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not;
>> >>       bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or;
>> >> +     bld_base->op_actions[TGSI_OPCODE_POPC].emit = build_tgsi_intrinsic_nomem;
>> >> +     bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32";
>> >>       bld_base->op_actions[TGSI_OPCODE_POW].emit = build_tgsi_intrinsic_nomem;
>> >>       bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32";
>> >>       bld_base->op_actions[TGSI_OPCODE_ROUND].emit = build_tgsi_intrinsic_nomem;
>> >> @@ -1389,6 +1395,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>> >>       bld_base->op_actions[TGSI_OPCODE_TRUNC].emit = build_tgsi_intrinsic_nomem;
>> >>       bld_base->op_actions[TGSI_OPCODE_TRUNC].intr_name = "llvm.AMDGPU.trunc";
>> >>       bld_base->op_actions[TGSI_OPCODE_UADD].emit = emit_uadd;
>> >> +     bld_base->op_actions[TGSI_OPCODE_UBFE].emit = build_tgsi_intrinsic_nomem;
>> >> +     bld_base->op_actions[TGSI_OPCODE_UBFE].intr_name = "llvm.AMDGPU.bfe.u32";
>> >>       bld_base->op_actions[TGSI_OPCODE_UDIV].emit = emit_udiv;
>> >>       bld_base->op_actions[TGSI_OPCODE_UMAX].emit = build_tgsi_intrinsic_nomem;
>> >>       bld_base->op_actions[TGSI_OPCODE_UMAX].intr_name = "llvm.AMDGPU.umax";
>> >> --
>> >> 2.1.0
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev@lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
I've looked into how to recognize BFM and BFI and discovered that if
TGSI_OPCODE_BFI is expanded, it's _impossible_ to recognize the
pattern in the backend due to LLVM transformations. The reason it's
impossible is that one particular simplification of the expanded IR
can always be done and it always changes the IR in a way that BFI
can't be recognized anymore.

The ideal transformation from TGSI to ISA is (note that this is also
how GLSL defines the opcode):

TGSI_OPCODE_BFI(base, insert, offset, bits)
= BFI(BFM(bits, offset), SHL(insert, offset), base) =
    s_lshl_b32 s1, s4, s6
    s_bfm_b32 s0, s0, s6
    v_mov_b32_e32 v0, s5
    v_mov_b32_e32 v1, s1
    v_bfi_b32 v0, s0, v1, v0
Ideally 3 instructions if all sources are vector registers.

However, if TGSI_OPCODE_BFI is expanded into basic bitwise operations
(BTW the result of BFM has 2 uses in BFI), LLVM applies this
transformation:
    (X << S) & (Y << S) ----> (X & Y) << S
Which breaks recognition of BFI and also the second use of BFM.
Therefore this version calculates the same BFM expression twice. The
first BFM is recognized by pattern matching, but the second BFM as
well as BFI is unrecognizable due to the transformation. The result
is:
    s_lshl_b32 s1, 1, s0
    s_bfm_b32 s0, s0, s6
    s_add_i32 s1, s1, -1
    s_not_b32 s0, s0
    s_and_b32 s1, s1, s4
    s_and_b32 s0, s0, s5
    s_lshl_b32 s1, s1, s6
    s_or_b32 s0, s1, s0

There are 2 ways out of this:

1) Use BFM and BFI intrinsics in Mesa. Simple and unlikely to break in
the future.

2) Try to recognize the expression tree seen by the backend. Changes
in LLVM core can break it. More complicated shaders with more
opportunities for transformations can break it too:

def : Pat <
  (i32 (or (i32 (shl (i32 (and (i32 (add (i32 (shl 1, i32:$a)),
            -1)), i32:$b)), i32:$c)),
           (i32 (and (i32 (xor (i32 (shl (i32 (add (i32 (shl 1, i32:$a)),
            -1)), i32:$c)), -1)), i32:$d)))),
  (V_BFI_B32 (S_BFM_B32 $a, $c),
             (S_LSHL_B32 $b, $c),
             $d)
>;

The pattern is ugly, so I choose 1. We can still have separate
patterns for BFI and BFM, but they would be mostly useless for GLSL.

Marek



On Tue, Mar 10, 2015 at 5:00 PM, Marek Olšák <maraeo@gmail.com> wrote:
> It seems overkill to use LLVM IR for BFE. There are several ways to
> express BFE(value, offset, bits) exactly, for example:
>
> # if bits == 0:
> #     0
> # else if offset + bits < 32:
> #     (value << (32 - offset - bits)) >> (32 - bits)
> # else:
> #     value >> offset
>
> Which can be simplified to either:
>
> # both = offset + bits
> # if bits == 0:
> #     0
> # else if both < 32:
> #     (value << (32 - both)) >> (32 - bits)
> # else:
> #     value >> offset
>
> Or:
>
> # if bits == 0:
> #     0
> # else if offset + bits < 32:
> #     inv_bits = 32 - bits
> #     (value << (inv_bits - offset)) >> inv_bits
> # else:
> #     value >> offset
>
> All 3 variants should use ASHR if the signed version is required.
> Alternatively, one can use this as well:
>
> # if bits == 32:
> #     value
> # else:
> #     (value >> offset) & ((1 << bits) - 1)
>
> And an explicit sign extension should be used for the signed version.
>
> The first three are so complex that I doubt it would be easy to
> recognize them and match them precisely. They are also very unlikely
> to appear open-coded in real applications, therefore, expanding them
> and then matching them again seems like a huge waste of time. Apps
> will likely just use LSHR+AND, which extracts a bitfield, but it's not
> the same as BFE, so BFE can't be used for that. (if the second operand
> of AND is a constant expression, then BFE can indeed be used)
>
> I agree that we could use LLVM IR for simple instructions like BFM and
> BFI, but I doubt it's a good idea for complex instructions like BFE,
> and any small optimization that changes the expression can break the
> matching.
>
> Marek
>
>
> On Tue, Mar 10, 2015 at 3:30 PM, Tom Stellard <tom@stellard.net> wrote:
>> On Tue, Mar 10, 2015 at 12:42:38PM +0100, Marek Olšák wrote:
>>> OK. What about patches 8 an 9?
>>>
>>
>> I think the intrinsics in 9 are OK, but 8 should be using LLVM IR.
>>
>> -Tom
>>
>>> Marek
>>>
>>> On Thu, Mar 5, 2015 at 8:30 PM, Tom Stellard <tom@stellard.net> wrote:
>>> > On Mon, Mar 02, 2015 at 12:54:21PM +0100, Marek Olšák wrote:
>>> >> From: Marek Olšák <marek.olsak@amd.com>
>>> >>
>>> >
>>> > Hi Marek,
>>> >
>>> > After discussing with Matt, I think we should use LLVM IR rather than
>>> > intrinsics for IBFE and UBFE and then add patterns for them either in
>>> > the TableGen Files or AMDGPUISelDAGToDAG.cpp.
>>> >
>>> > Using intrinsics for BREV and POPC is fine though.
>>> >
>>> > -Tom
>>> >
>>> >> ---
>>> >>  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 ++++++++
>>> >>  1 file changed, 8 insertions(+)
>>> >>
>>> >> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>>> >> index 385d3ad..034095f 100644
>>> >> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>>> >> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
>>> >> @@ -1293,6 +1293,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>> >>       bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and;
>>> >>       bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl;
>>> >>       bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit;
>>> >> +     bld_base->op_actions[TGSI_OPCODE_BREV].emit = build_tgsi_intrinsic_nomem;
>>> >> +     bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = "llvm.AMDGPU.brev";
>>> >>       bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit;
>>> >>       bld_base->op_actions[TGSI_OPCODE_CEIL].emit = build_tgsi_intrinsic_nomem;
>>> >>       bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil";
>>> >> @@ -1326,6 +1328,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>> >>       bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp;
>>> >>       bld_base->op_actions[TGSI_OPCODE_IABS].emit = build_tgsi_intrinsic_nomem;
>>> >>       bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = "llvm.AMDIL.abs.";
>>> >> +     bld_base->op_actions[TGSI_OPCODE_IBFE].emit = build_tgsi_intrinsic_nomem;
>>> >> +     bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = "llvm.AMDGPU.bfe.i32";
>>> >>       bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv;
>>> >>       bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit;
>>> >>       bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit;
>>> >> @@ -1350,6 +1354,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>> >>       bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod;
>>> >>       bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not;
>>> >>       bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or;
>>> >> +     bld_base->op_actions[TGSI_OPCODE_POPC].emit = build_tgsi_intrinsic_nomem;
>>> >> +     bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32";
>>> >>       bld_base->op_actions[TGSI_OPCODE_POW].emit = build_tgsi_intrinsic_nomem;
>>> >>       bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32";
>>> >>       bld_base->op_actions[TGSI_OPCODE_ROUND].emit = build_tgsi_intrinsic_nomem;
>>> >> @@ -1389,6 +1395,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context * ctx)
>>> >>       bld_base->op_actions[TGSI_OPCODE_TRUNC].emit = build_tgsi_intrinsic_nomem;
>>> >>       bld_base->op_actions[TGSI_OPCODE_TRUNC].intr_name = "llvm.AMDGPU.trunc";
>>> >>       bld_base->op_actions[TGSI_OPCODE_UADD].emit = emit_uadd;
>>> >> +     bld_base->op_actions[TGSI_OPCODE_UBFE].emit = build_tgsi_intrinsic_nomem;
>>> >> +     bld_base->op_actions[TGSI_OPCODE_UBFE].intr_name = "llvm.AMDGPU.bfe.u32";
>>> >>       bld_base->op_actions[TGSI_OPCODE_UDIV].emit = emit_udiv;
>>> >>       bld_base->op_actions[TGSI_OPCODE_UMAX].emit = build_tgsi_intrinsic_nomem;
>>> >>       bld_base->op_actions[TGSI_OPCODE_UMAX].intr_name = "llvm.AMDGPU.umax";
>>> >> --
>>> >> 2.1.0
>>> >>
>>> >> _______________________________________________
>>> >> mesa-dev mailing list
>>> >> mesa-dev@lists.freedesktop.org
>>> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev