[Mesa-dev] gallium bound checking patch broke r600g in weird way

[Brian Paul]

On 08/02/2010 04:49 PM, keith whitwell wrote:
> On Mon, Aug 2, 2010 at 3:53 PM, Brian Paul<brianp at vmware.com>  wrote:
>> On 08/01/2010 01:55 AM, Chia-I Wu wrote:
>>>
>>> On Sat, Jul 31, 2010 at 7:10 AM, Zack Rusin<zackr at vmware.com>    wrote:
>>>>
>>>> On Friday 30 July 2010 17:51:21 Jakob Bornecrantz wrote:
>>>>>
>>>>> On 30 jul 2010, at 14.02, Jakob Bornecrantz wrote:
>>>>>>
>>>>>> On 30 jul 2010, at 13.32, Brian Paul wrote:
>>>>>>>
>>>>>>> On 07/30/2010 12:38 PM, Jerome Glisse wrote:
>>>>>>>>
>>>>>>>> Hi Brian,
>>>>>>>>
>>>>>>>> I am facing a strange segfault with r600g on top of lastest git,
>>>>>>>> git bisect pointed to gallium: implement bounds checking for
>>>>>>>> constant buffers
>>>>>>>> My feeling is that it should only affect software pipeline but
>>>>>>>> somehow r600g seem to take different path now, attached if full
>>>>>>>> but i can't make much sense out of it, do you have a clue on what
>>>>>>>> might went wrong ?
>>>>>>>
>>>>>>> I took a quick look but didn't find anything.
>>>>>>>
>>>>>>> Maybe try a make clean and rebuild just in case?
>>>>>>
>>>>>> I'm getting the same with swrastg on in 32bit VM, "git clean -fdx":ed
>>>>>> even.
>>>>>
>>>>> Me and Jerome tracked it down to the SSE code generated by the tgsi
>>>>> runtime. Exporting GALLIUM_NOSSE avoids the bug. I'm guessing you are
>>>>> either using LLVM or 64bit which doesn't have SSE codegen.
>>>
>>> I am having this warning
>>>
>>>    draw/draw_vs_sse.c: In function ‘draw_create_vs_sse’:
>>>    draw/draw_vs_sse.c:172: warning: assignment from incompatible pointer
>>> type
>>>
>>> It seems the prototype of vs_sse_run_linear was not updated in the commit
>>> and
>>> the function body is accessing the wrong arguments.
>>
>> Dave Airlie fixed this and it seems to resolve the issue (I just tesed with
>> and without Dave's fix on 32-bit and it worked for me).
>>
>>
>>>> We actually talked about removing the SSE code from draw. We have the
>>>> interpreted paths (safe and easier to debug) and the LLVM paths (for
>>>> performance) and the no one is too keen to maintain the SSE code.
>>>>
>>>> Unless someone would like to maintain the SSE paths if I'll have a bit of
>>>> time
>>>> I'm probably going to remove them next week. They'll be still in the git
>>>> history if someone will ever want to bring them back but right now they
>>>> tend
>>>> to crash a bit (like in this case) which is more trouble than it's worth.
>>>
>>> Yeah, that sounds good.  This is not the first time (#28752) the SSE
>>> path gives weird bugs.
>>
>> I'd prefer to keep the SSE code for now.  It's sometimes useful for me when
>> I'm debugging things to do comparisons between SSE and the TGSI interpreter.
>>
>> The code is working fine again.  I'm sorry my change caused such a
>> commotion.  I was in a hurry on Friday and just didn't test on 32-bit.
>
> Would comparing between llvm and the interpreter work as well for you Brian?

Well, when switching to LLVM, more than just the shader code is 
brought into play so it's not quite the same.


> What about keeping the SSE code but hiding it behind an option and
> making LLVM the default?

If we want to make GALLIUM_NOSSE default to 1, that's OK.

-Brian