Bug 86469 - [SNB] Unreal Engine demo doesn't run
Summary: [SNB] Unreal Engine demo doesn't run
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Ian Romanick
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
: 91259 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-11-19 16:47 UTC by Andrey Gursky
Modified: 2016-11-03 16:07 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Gursky 2014-11-19 16:47:34 UTC
Great, OpenGL 3.3 is now supported on intel sandybridge!

Considering Unreal Engine demos from [1]. Sun Temple Demo does work. But Effects Cave Demo [2] doesn't start. 

Related git commit: 537f183fe67e0cf9f5737106d914cdabcf5d002e

Is this fixable?

Thanks,
Andrey

[1] https://wiki.unrealengine.com/Linux_Demos
[2] http://ue4linux.raxxy.com/effects_cave_demo.tar.bz2

Log:

gursky@hpprobook:~/progs/Effects Cave Demo$ LD_LIBRARY_PATH=/home/gursky/progs/mesa/lib EffectsCave/Binaries/Linux/EffectsCave 
Using binned.
Increasing per-process limit of core file size to infinity.
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Mesa 10.4.0-devel implementation error: Failed to compile fragment shader: FS compile failed: Register spilling not supported with m14 used


Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa
Signal 11 caught.
EngineCrashHandler: Signal=11
Starting ../../../Engine/Binaries/Linux/CrashReportClient
Aborted (core dumped)
gursky@hpprobook:~/progs/Effects Cave Demo$
Comment 1 Kenneth Graunke 2014-11-30 04:32:07 UTC
This should actually be pretty easy to fix.  Sandybridge has more than 16 MRFs, so we could just adjust the spilling code to use the higher numbered ones which aren't used for anything else.

The check also seems a bit heavy handed...it may not actually conflict...
Comment 2 Tapani Pälli 2015-01-19 07:31:27 UTC
(In reply to Kenneth Graunke from comment #1)
> This should actually be pretty easy to fix.  Sandybridge has more than 16
> MRFs, so we could just adjust the spilling code to use the higher numbered
> ones which aren't used for anything else.
> 
> The check also seems a bit heavy handed...it may not actually conflict...

I've checked that Effects works on SNB by simply setting spill_base_mrf 15 (as 14 is used). I did not find documentation that would indicate more than 16 MRFs available though. Docs indicate there are 16 from which 1 .. 15 can be used.
Comment 3 Matt Turner 2015-03-07 00:20:35 UTC
(In reply to Tapani Pälli from comment #2)
> (In reply to Kenneth Graunke from comment #1)
> > This should actually be pretty easy to fix.  Sandybridge has more than 16
> > MRFs, so we could just adjust the spilling code to use the higher numbered
> > ones which aren't used for anything else.
> > 
> > The check also seems a bit heavy handed...it may not actually conflict...
> 
> I've checked that Effects works on SNB by simply setting spill_base_mrf 15
> (as 14 is used). I did not find documentation that would indicate more than
> 16 MRFs available though. Docs indicate there are 16 from which 1 .. 15 can
> be used.

SNB PRM Vol4 Part2's "Table 5-4. MRF Registers Available in Device Hardware" says "Number per Thread" - "24 registers"

This would be an easy experiment for someone with a Sandybridge -- modify the fs_visitor::spill_reg() function in src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
Comment 4 Tapani Pälli 2015-03-07 04:57:07 UTC
(In reply to Matt Turner from comment #3)
> (In reply to Tapani Pälli from comment #2)
> > (In reply to Kenneth Graunke from comment #1)
> > > This should actually be pretty easy to fix.  Sandybridge has more than 16
> > > MRFs, so we could just adjust the spilling code to use the higher numbered
> > > ones which aren't used for anything else.
> > > 
> > > The check also seems a bit heavy handed...it may not actually conflict...
> > 
> > I've checked that Effects works on SNB by simply setting spill_base_mrf 15
> > (as 14 is used). I did not find documentation that would indicate more than
> > 16 MRFs available though. Docs indicate there are 16 from which 1 .. 15 can
> > be used.
> 
> SNB PRM Vol4 Part2's "Table 5-4. MRF Registers Available in Device Hardware"
> says "Number per Thread" - "24 registers"
> 
> This would be an easy experiment for someone with a Sandybridge -- modify
> the fs_visitor::spill_reg() function in
> src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp

Yep, I saw this but bspec states that it might not be safe to use MRFs other than 1 .. 15. It feels a bit risky to go change this just because of one demo.
Comment 5 Matt Turner 2015-03-07 05:45:59 UTC
(In reply to Tapani Pälli from comment #4)
> Yep, I saw this but bspec states that it might not be safe to use MRFs other
> than 1 .. 15.

Where does it say that?
Comment 6 Tapani Pälli 2015-03-07 07:00:47 UTC
(In reply to Matt Turner from comment #5)
> (In reply to Tapani Pälli from comment #4)
> > Yep, I saw this but bspec states that it might not be safe to use MRFs other
> > than 1 .. 15.
> 
> Where does it say that?

SNB bspec, EU Overview -> Messages -> Message Register File (MRF), last paragraph.
Comment 7 Matt Turner 2015-03-07 07:28:34 UTC
(In reply to Tapani Pälli from comment #6)
> SNB bspec, EU Overview -> Messages -> Message Register File (MRF), last
> paragraph.

That page doesn't say "it's not safe". If anything it says MRFs > 15 don't exist ("There are 16 MRF registers").

It wouldn't surprise me if the registers don't exist and the "24" is a mistake, but it also wouldn't surprise me if SNB actually has 24 MRFs and they just didn't update a bunch of documentation. Experimentation is the only way to be sure.
Comment 8 Tapani Pälli 2015-03-07 08:00:04 UTC
(In reply to Matt Turner from comment #7)
> (In reply to Tapani Pälli from comment #6)
> > SNB bspec, EU Overview -> Messages -> Message Register File (MRF), last
> > paragraph.
> 
> That page doesn't say "it's not safe". If anything it says MRFs > 15 don't
> exist ("There are 16 MRF registers").
> 
> It wouldn't surprise me if the registers don't exist and the "24" is a
> mistake, but it also wouldn't surprise me if SNB actually has 24 MRFs and
> they just didn't update a bunch of documentation. Experimentation is the
> only way to be sure.

I dont have access right now but the last clause is something like "regardless of the actual hw implementation thread should not assume registers above 16 can be used" but yep, one can experiment
Comment 9 Jason Ekstrand 2015-03-07 18:46:25 UTC
(In reply to Tapani Pälli from comment #8)
> I dont have access right now but the last clause is something like
> "regardless of the actual hw implementation thread should not assume
> registers above 16 can be used" but yep, one can experiment

I seem to recall at least having started that experiment at some point.  I think I gave up because the generator asserts 16 MRFs all over the place and I didn't want to bother special-casing SNB all the way down.
Comment 10 Matt Turner 2015-03-07 20:48:58 UTC
(In reply to Tapani Pälli from comment #8)
> I dont have access right now but the last clause is something like
> "regardless of the actual hw implementation thread should not assume
> registers above 16 can be used" but yep, one can experiment

I still don't think you're understanding what I'm saying. The statement you're referring to says

> Regardless of actual hardware implementation, the thread should not assume that MRF addresses above m15 wrap to legal MRF registers.

My point is that you cannot draw any conclusions about MRF > 15 from a page that claims they do not exist.
Comment 11 Tapani Pälli 2015-03-07 21:06:30 UTC
(In reply to Matt Turner from comment #10)
> (In reply to Tapani Pälli from comment #8)
> > I dont have access right now but the last clause is something like
> > "regardless of the actual hw implementation thread should not assume
> > registers above 16 can be used" but yep, one can experiment
> 
> I still don't think you're understanding what I'm saying. The statement
> you're referring to says
> 
> > Regardless of actual hardware implementation, the thread should not assume that MRF addresses above m15 wrap to legal MRF registers.
> 
> My point is that you cannot draw any conclusions about MRF > 15 from a page
> that claims they do not exist.

Ok, sorry if this caused confusion, I just thought to answer that this is why I stopped experimentation with this bug, I was not sure if it is safe and worth the effort.
Comment 12 Iago Toral 2015-07-13 12:01:44 UTC
(In reply to Tapani Pälli from comment #2)
> (In reply to Kenneth Graunke from comment #1)
> > This should actually be pretty easy to fix.  Sandybridge has more than 16
> > MRFs, so we could just adjust the spilling code to use the higher numbered
> > ones which aren't used for anything else.
> > 
> > The check also seems a bit heavy handed...it may not actually conflict...
> 
> I've checked that Effects works on SNB by simply setting spill_base_mrf 15
> (as 14 is used). I did not find documentation that would indicate more than
> 16 MRFs available though. Docs indicate there are 16 from which 1 .. 15 can
> be used.

Even if we keep 16 as the limit for now, why not compute spill_base_mrf based on the actual data in mrf_used? Something like this:

for (int i = BRW_MAX_MRF - 1; !mrf_used[i]; i--);
spill_base_mrf = i + 1;
if (spill_base_mrf >= BRW_MAX_MRF) {
   fail("Register spilling not supported with m%d used", i);
   return;
}

We would have to do this in spill_reg and emit_spill, but that would make sure that we don't fail if there enough unused MRFs available within that limit, which is what is happening here.
Comment 13 Samuel Iglesias 2015-07-22 09:41:59 UTC
Just to mention that I tested it on my SNB laptop with today's master (800efb0690e962750b9a072bcbab279fdaae24a1). It runs the demo very slowly but without spilling errors.

I ran git-bisect. This is the commit that fixed it:

commit 9b577d57029bb643f2b48b80648b4f901818e93b
Author: Matt Turner <mattst88@gmail.com>
Date:   Mon Mar 16 21:33:31 2015 -0700

    glsl: Transform pow(x, 4) into (x*x)*(x*x).
    
    Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

However we still have this issue in SNB and we could hit it again in other programs (I have not checked current status of related bugs like bug #90631 and bug #91259).

Iago's proposal seems reasonable, What do you think about it?
Comment 14 Matt Turner 2015-08-10 18:04:00 UTC
*** Bug 91259 has been marked as a duplicate of this bug. ***
Comment 15 Iago Toral 2015-09-16 09:29:02 UTC
(In reply to Matt Turner from comment #10)
> (In reply to Tapani Pälli from comment #8)
> > I dont have access right now but the last clause is something like
> > "regardless of the actual hw implementation thread should not assume
> > registers above 16 can be used" but yep, one can experiment
> 
> I still don't think you're understanding what I'm saying. The statement
> you're referring to says
> 
> > Regardless of actual hardware implementation, the thread should not assume that MRF addresses above m15 wrap to legal MRF registers.
> 
> My point is that you cannot draw any conclusions about MRF > 15 from a page
> that claims they do not exist.
(In reply to Matt Turner from comment #7)
> (In reply to Tapani Pälli from comment #6)
> > SNB bspec, EU Overview -> Messages -> Message Register File (MRF), last
> > paragraph.
> 
> That page doesn't say "it's not safe". If anything it says MRFs > 15 don't
> exist ("There are 16 MRF registers").
> 
> It wouldn't surprise me if the registers don't exist and the "24" is a
> mistake, but it also wouldn't surprise me if SNB actually has 24 MRFs and
> they just didn't update a bunch of documentation. Experimentation is the
> only way to be sure.


FWIW, I've sent a RFC series to the mailing list that implements this test:
http://lists.freedesktop.org/archives/mesa-dev/2015-September/094584.html

Spoiler alert: SNB supports 24 MRF registers
Comment 16 Iago Toral 2015-09-23 06:41:18 UTC
This should be fixed in master since:

commit 5d23ce2f15bda866990750b49d7860144dff2e68
Author: Iago Toral Quiroga <itoral@igalia.com>
Date:   Thu Sep 17 13:43:52 2015 +0200

    i965/vec4: Use MRF registers 21-23 for spilling in gen6
    
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

commit 6789a32075774fc332eb7432910c7fbc21ee2026
Author: Iago Toral Quiroga <itoral@igalia.com>
Date:   Tue Sep 15 16:33:48 2015 +0200

    i965/fs: Use MRF registers 21-23 for spilling in gen6
    
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Comment 17 Andrey Gursky 2015-09-23 15:55:19 UTC
    Iago, thanks for the patches and Kenneth for the review! And to all
   for the fruitfull discussion. But I'm still wondering that nobody from
   intel could promptly definitely approve the assumptions about the
   number of registers. Or the documentation has been in meanwhile
   updated?
Comment 18 Iago Toral 2015-09-24 06:03:00 UTC
(In reply to Andrey Gursky from comment #17)
>     Iago, thanks for the patches and Kenneth for the review! And to all
>    for the fruitfull discussion. But I'm still wondering that nobody from
>    intel could promptly definitely approve the assumptions about the
>    number of registers. Or the documentation has been in meanwhile
>    updated?

The documentation has not been updated, what we did was to test if we could use 24 MRFs, which was easy enough to do. SNB has been out there for many years, so I imagine it might not be that easy to find the right person inside Intel to confirm something as specific as this.
Comment 19 Clément Guérin 2015-10-07 10:44:53 UTC
Any chance the fix could be backported in Mesa 11?
Comment 20 Eero Tamminen 2016-08-31 10:55:08 UTC
(In reply to Iago Toral from comment #16)
> This should be fixed in master since:
> 
> commit 5d23ce2f15bda866990750b49d7860144dff2e68
> Author: Iago Toral Quiroga <itoral@igalia.com>
> Date:   Thu Sep 17 13:43:52 2015 +0200

-> Marking the bug as fixed.
Comment 21 Juan A. Suarez 2016-11-03 16:04:32 UTC
(In reply to Eero Tamminen from comment #20)
> (In reply to Iago Toral from comment #16)
> > This should be fixed in master since:
> > 
> > commit 5d23ce2f15bda866990750b49d7860144dff2e68
> > Author: Iago Toral Quiroga <itoral@igalia.com>
> > Date:   Thu Sep 17 13:43:52 2015 +0200
> 
> -> Marking the bug as fixed.


Is this FIXED? Bug is still marked as NEW.
Comment 22 Iago Toral 2016-11-03 16:07:55 UTC
(In reply to Juan A. Suarez from comment #21)
> (In reply to Eero Tamminen from comment #20)
> > (In reply to Iago Toral from comment #16)
> > > This should be fixed in master since:
> > > 
> > > commit 5d23ce2f15bda866990750b49d7860144dff2e68
> > > Author: Iago Toral Quiroga <itoral@igalia.com>
> > > Date:   Thu Sep 17 13:43:52 2015 +0200
> > 
> > -> Marking the bug as fixed.
> 
> 
> Is this FIXED? Bug is still marked as NEW.

Eero forgot to flip the state when he updated the bug report. Marking as fixed now.


bug/show.html.tmpl processed on Jan 16, 2017 at 21:52:18.
(provided by the Example extension).