[Mesa-dev] [PATCH 2/9] radeonsi: use V_BFE for extracting a sample index

[Matt Arsenault]

> On Mar 5, 2015, at 6:50 AM, Tom Stellard <tom at stellard.net> wrote:
> 
> On Mon, Mar 02, 2015 at 02:09:29PM -0800, Matt Arsenault wrote:
>> 
>>> On Mar 2, 2015, at 1:19 PM, Tom Stellard <tom at stellard.net> wrote:
>>> 
>>> On Mon, Mar 02, 2015 at 10:14:00PM +0100, Marek Olšák wrote:
>>>> On Mon, Mar 2, 2015 at 10:05 PM, Tom Stellard <tom at stellard.net> wrote:
>>>>> On Mon, Mar 02, 2015 at 12:54:16PM +0100, Marek Olšák wrote:
>>>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>>> 
>>>>>> ---
>>>>>> src/gallium/drivers/radeonsi/si_shader.c | 22 ++++++++++++++++------
>>>>>> 1 file changed, 16 insertions(+), 6 deletions(-)
>>>>>> 
>>>>>> diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c
>>>>>> index b0417ed..f125483 100644
>>>>>> --- a/src/gallium/drivers/radeonsi/si_shader.c
>>>>>> +++ b/src/gallium/drivers/radeonsi/si_shader.c
>>>>>> @@ -192,6 +192,20 @@ static int get_param_index(unsigned semantic_name, unsigned index,
>>>>>> }
>>>>>> 
>>>>>> /**
>>>>>> + * BitField Extract: ((value >> rshift) & ((1 << bitwidth) - 1))
>>>>>> + */
>>>>> 
>>>>> Ideally, we would just add a pattern for this in the backend and emit generic
>>>>> LLVM IR here.  This would also make it possible to share the code with llvmpipe.
>>>>> 
>>>>> I think the best place to do this would be in AMDGPUTargetLowering::performDAGCombine().
>>>> 
>>>> Why not SIInstructions.td?
>>>> 
>>> 
>>> Because for patterns like this, I think it is important to match them as
>>> early as possible, because there may be another optimization which reduces
>>> the sequence from 5 to 4 instructions which would cause the pattern not to match.
>>> 
>>> -Tom
>> 
>> 
>> I think the opposite in this case. The basic bit operations have a lot of existing combines on them, and the computeKnownBits implementations are more complete. The BFE nodes are not as well understood, and trickier to deal with. AArch64 and NVPTX both have essentially the same instruction, and they have a large bit of code to match them in their ISelDAGToDAGs. I’ve wanted to add a generic BFE node to be matched after legalization, but I haven’t had time to do it.
>> 
> 
> Isn't there a potential for a DAG combine on bit operations, to 'break'
> the pattern so it can't be recognized?
> 
> -Tom
> 

Yes and no. There isn’t really only one pattern for this. Ideally the different ∑ays to use this would each form a single canonical form the pattern would need to match, but that isn’t always possible. The AArch64 code for example tries many different patterns (see isBitfieldExtractOp in AArch64ISelDAGToDAG)