[Mesa-dev] [PATCH 10/12] R600/SI: cleanup literal handling

[Christian König]

Am 13.02.2013 08:00, schrieb Michel Dänzer:
> On Die, 2013-02-12 at 19:39 -0500, Tom Stellard wrote:
>> On Tue, Feb 12, 2013 at 06:13:22PM +0100, Christian König wrote:
>>> From: Christian König <christian.koenig at amd.com>
>>>
>>> Seems to be allot simpler, and also paves the
>>> way for further improvements.
> [...]
>
>>> diff --git a/lib/Target/R600/SIInstructions.td b/lib/Target/R600/SIInstructions.td
>>> index 63ef9ef..e68bbe7 100644
>>> --- a/lib/Target/R600/SIInstructions.td
>>> +++ b/lib/Target/R600/SIInstructions.td
>>> [...]
>>> @@ -1180,7 +1141,7 @@ def : Pat<
>>>                           VReg_32:$buf_idx_vgpr),
>>>     (BUFFER_LOAD_FORMAT_XYZW imm:$attr_offset, 0, 1, 0, 0, 0,
>>>                              VReg_32:$buf_idx_vgpr, SReg_128:$tlst,
>>> -                           0, 0, (i32 SREG_LIT_0))
>>> +                           0, 0, 0x80)
>> What does this 0x80 mean?
> It's the encoding of inline constant 0, i.e. the same as SREG_LIT_0 was.

Actually I wanted to clean that up before sending out the patches and 
correctly use SSrc_32 for SOFFSET, but somehow forgot about it.

Going to clean that up for the second version.

>>> diff --git a/lib/Target/R600/SILowerControlFlow.cpp b/lib/Target/R600/SILowerControlFlow.cpp
>>> index 3780e40..051f460 100644
>>> --- a/lib/Target/R600/SILowerControlFlow.cpp
>>> +++ b/lib/Target/R600/SILowerControlFlow.cpp
>>> @@ -158,10 +158,10 @@ void SILowerControlFlowPass::SkipIfDead(MachineInstr &MI) {
>>>             .addImm(0)
>>>             .addImm(1)
>>>             .addImm(1)
>>> -          .addReg(AMDGPU::SREG_LIT_0)
>>> -          .addReg(AMDGPU::SREG_LIT_0)
>>> -          .addReg(AMDGPU::SREG_LIT_0)
>>> -          .addReg(AMDGPU::SREG_LIT_0);
>>> +          .addImm(0)
>>> +          .addImm(0)
>>> +          .addImm(0)
>>> +          .addImm(0);
> Because the EXP definition declares these operands as VReg_32, not
> VSrc_32, this results in encoding 0 directly, i.e. VGPR0. It doesn't
> really matter here for exporting to the NULL target, but then it might
> be better to make that explicit by using an undefined value.

SI doesn't support inline literals in the EXP command (in opposition to 
R600), so using SREG_LIT_0 here was completely wrong in the first place 
and actually tried to export VGPR 128.

But as you mentioned it doesn't really matter cause we are exporting to 
the NULL target anyway. Going to change that to use VGPR0 here instead, 
that should make clear what's going on here.

Christian.