Bug 89416

Summary: UE4Editor crash after load project
Product: Mesa Reporter: Yury Zhuravlev <stalkerg>
Component: Drivers/DRI/i965Assignee: Matt Turner <mattst88>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium    
Version: git   
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: gdb backtrace
Some logs from UE4Editor
patch

Description Yury Zhuravlev 2015-03-03 20:59:16 UTC
Created attachment 113962 [details]
gdb backtrace

UE4Editor 4.7.1 crash after load simple project. I have:
OpenGL renderer string: Mesa DRI Intel(R) Haswell Mobile 
OpenGL core profile version string: 3.3 (Core Profile) Mesa 10.6.0-devel (git-e437299)


From UE4Editor logs:
UE4Editor: intel_tiled_memcpy.c:88: rgba8_copy: Assertion `!(((uintptr_t)dst) & 0xf)' failed.

Now UE4 is free and you can simple "git clone" after register (https://www.unrealengine.com/).
Comment 1 Yury Zhuravlev 2015-03-03 21:00:02 UTC
Created attachment 113963 [details]
Some logs from UE4Editor
Comment 2 Matt Turner 2015-03-03 23:39:17 UTC
In case you're dying to work around this, you can compile with CFLAGS that do not enable SSSE3 and it will avoid this code path.
Comment 3 Yury Zhuravlev 2015-03-04 00:32:03 UTC
You right with -mno-ssse3 is work well. Why we can't use _mm_shuffle_epi8 if first 4 bit of addr dst is 0? And how could this happen?

Sorry for my questions.
Comment 4 Yury Zhuravlev 2015-03-04 16:32:43 UTC
Ok, alignment... maybe I can fix it.
Comment 5 Jason Ekstrand 2015-03-04 20:12:37 UTC
(In reply to Uriy Zhuravlev from comment #4)
> Ok, alignment... maybe I can fix it.

Uriy,
The problem here is that the code originally assumed that the destination would always be nicely aligned but the source may not.  When rgba8_copy function started getting used for texture *download* via ReadPixels and GetTexImage the assumption now fails and it is the source that is aligned and not the destination.  To fix this we need to do one of two things.  Either a) make the rgba8_copy function use unaligned reads (which would b slower) or b) make two versions of the rgba8_copy function: one for upload and one for download.

I think option b) is best but if we're going to do that then we need to add a flag to intel_get_memcpy to make sure it returns the correct one and then thread it through the tiled_to_linear_faster functions as well.
Comment 6 Matt Turner 2015-03-04 20:28:05 UTC
(In reply to Jason Ekstrand from comment #5)
> (In reply to Uriy Zhuravlev from comment #4)
> > Ok, alignment... maybe I can fix it.
> 
> Uriy,
> The problem here is that the code originally assumed that the destination
> would always be nicely aligned but the source may not.  When rgba8_copy
> function started getting used for texture *download* via ReadPixels and
> GetTexImage the assumption now fails and it is the source that is aligned
> and not the destination.  To fix this we need to do one of two things. 
> Either a) make the rgba8_copy function use unaligned reads (which would b
> slower) or b) make two versions of the rgba8_copy function: one for upload
> and one for download.

Basically, this all stems from the fact that there's no guarantee that the pointer given to glReadPixels/glGetTexImage is suitably aligned?

Do you have time to fix this before the release on Friday? Trying to figure if I need to work on this.
Comment 7 Jason Ekstrand 2015-03-04 20:32:04 UTC
(In reply to Matt Turner from comment #6)
> (In reply to Jason Ekstrand from comment #5)
> > (In reply to Uriy Zhuravlev from comment #4)
> > > Ok, alignment... maybe I can fix it.
> > 
> > Uriy,
> > The problem here is that the code originally assumed that the destination
> > would always be nicely aligned but the source may not.  When rgba8_copy
> > function started getting used for texture *download* via ReadPixels and
> > GetTexImage the assumption now fails and it is the source that is aligned
> > and not the destination.  To fix this we need to do one of two things. 
> > Either a) make the rgba8_copy function use unaligned reads (which would b
> > slower) or b) make two versions of the rgba8_copy function: one for upload
> > and one for download.
> 
> Basically, this all stems from the fact that there's no guarantee that the
> pointer given to glReadPixels/glGetTexImage is suitably aligned?
> 
> Do you have time to fix this before the release on Friday? Trying to figure
> if I need to work on this.

I think it would probably be more efficient if you did.  The above should be enough of a brain dump on what needs to happen in the tiled_memcpy paths.  The rest is writing suitable mmx code and you probably know more about that than I do.  TBH: I'd probably be asking you on IRC what instructions to use anyway.  If you don't want to bother with threading it through the tiled_memcpy code, I can do that part quick enough.
Comment 8 Matt Turner 2015-03-04 20:45:24 UTC
Yeah, sounds like (b) is a good option. I don't think it should take me too much time.
Comment 9 Yury Zhuravlev 2015-03-04 20:50:19 UTC
I will always be happy to help at least testing.
Comment 10 Matt Turner 2015-03-05 00:20:47 UTC
Created attachment 114014 [details] [review]
patch

Please test this patch. I sent it to the mailing list and Cc'd you as well.
Comment 11 Yury Zhuravlev 2015-03-05 00:53:32 UTC
This patch works perfectly. Thank U.
Comment 12 Matt Turner 2015-03-05 18:18:57 UTC
Fixed by

commit 2e4c95dfe2cb205c327ceaa12b44a9273bdb20dc
Author: Matt Turner <mattst88@gmail.com>
Date:   Wed Mar 4 15:21:53 2015 -0800

    i965: Tell intel_get_memcpy() which direction the memcpy() is going.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.