Bug 90056

Could you run the program with the environment variable R600_DEBUG=ps,vs,gs and post the output.

Created attachment 115136 [details]
valley with R600_DEBUG=ps,vs,gs

Control flow annotation seems to choke on a predecessor with "undef" incoming value, for some reason.

Created attachment 115333 [details] [review]
Reduced test case

Here's a reduced testcase. I'm not sure how undef branch conditions are supposed to be handled, make someone else can take a look?

If this can't be fixed soon, maybe the bisected change should be reverted for now?

Created attachment 115401 [details] [review]
Possible fix

How about this? Not tested yet, I don't have access to a radeon GPU right now.

(In reply to Grigori Goronzy from comment #6)
> Created attachment 115401 [details] [review] [review]
> Possible fix
> 
> How about this? Not tested yet, I don't have access to a radeon GPU right
> now.

It doesn't help valley.

Does this patch help: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20150202/257738.html

Created attachment 115409 [details] [review]
Full shader

That does not help either. Seems to break the SSA somehow. Full shader that triggers the bug attached.

The patch I posted earlier help with the the reduced testcase, but not with the full shader. Both undef incoming values in phi nodes and undef branch conditions cause problems in different ways.

(In reply to Grigori Goronzy from comment #9)
> Created attachment 115409 [details] [review] [review]
> Full shader
> 
> That does not help either. Seems to break the SSA somehow. Full shader that
> triggers the bug attached.
> 
> The patch I posted earlier help with the the reduced testcase, but not with
> the full shader. Both undef incoming values in phi nodes and undef branch
> conditions cause problems in different ways.

With the second patch + valley I get -

valley_x64: LiveVariables.cpp:114: void llvm::LiveVariables::MarkVirtRegAliveInBlock(llvm::LiveVariables::VarInfo &, llvm::MachineBasicBlock *, llvm::MachineBasicBlock *, std::vector<MachineBasicBlock *> &): Assertion `MBB != &MF->front() && "Can't find reaching def for virtreg"' failed.

Created attachment 115423 [details] [review]
Reduced test case

(In reply to Andy Furniss from comment #10)
> (In reply to Grigori Goronzy from comment #9)
> > Created attachment 115409 [details] [review] [review] [review]
> > Full shader
> > 
> > That does not help either. Seems to break the SSA somehow. Full shader that
> > triggers the bug attached.
> > 
> > The patch I posted earlier help with the the reduced testcase, but not with
> > the full shader. Both undef incoming values in phi nodes and undef branch
> > conditions cause problems in different ways.
> 
> With the second patch + valley I get -
> 
> valley_x64: LiveVariables.cpp:114: void
> llvm::LiveVariables::MarkVirtRegAliveInBlock(llvm::LiveVariables::VarInfo &,
> llvm::MachineBasicBlock *, llvm::MachineBasicBlock *,
> std::vector<MachineBasicBlock *> &): Assertion `MBB != &MF->front() &&
> "Can't find reaching def for virtreg"' failed.

Did test with only the patch from comment #8 or did you test with both patches?

(In reply to Tom Stellard from comment #12)
> (In reply to Andy Furniss from comment #10)
> > (In reply to Grigori Goronzy from comment #9)
> > > Created attachment 115409 [details] [review] [review] [review] [review]
> > > Full shader
> > > 
> > > That does not help either. Seems to break the SSA somehow. Full shader that
> > > triggers the bug attached.
> > > 
> > > The patch I posted earlier help with the the reduced testcase, but not with
> > > the full shader. Both undef incoming values in phi nodes and undef branch
> > > conditions cause problems in different ways.
> > 
> > With the second patch + valley I get -
> > 
> > valley_x64: LiveVariables.cpp:114: void
> > llvm::LiveVariables::MarkVirtRegAliveInBlock(llvm::LiveVariables::VarInfo &,
> > llvm::MachineBasicBlock *, llvm::MachineBasicBlock *,
> > std::vector<MachineBasicBlock *> &): Assertion `MBB != &MF->front() &&
> > "Can't find reaching def for virtreg"' failed.
> 
> Did test with only the patch from comment #8 or did you test with both
> patches?

Just #8 but I just tried with both and get the same fail.

Created attachment 115488 [details] [review]
Possible fix

Can you try this applying this patch only?

(In reply to Tom Stellard from comment #14)
> Created attachment 115488 [details] [review] [review]
> Possible fix
> 
> Can you try this applying this patch only?

This fixes it.

Fixed in llvm: r236306

Since mesa 10.6 is going RC soon, what's going to happen with this feature? If it remains enabled, and the fix isn't backported to llvm 3.6, this bug will end up being present in a stable release (unless llvm 3.7 is also going to be released soon).

Fez is also affected by the same bug (I tested and verified that it is fixed by the same patch), so there may be many more games affected:

Good (mesa 10.5): https://www.dropbox.com/s/h7qtlgln1xlnxrk/fez_good.png?dl=0
Bad (mesa 10.6): https://www.dropbox.com/s/f5yqqx2l6pz284z/fez_bad.png?dl=0

(In reply to Furkan from comment #17)
> Since mesa 10.6 is going RC soon, what's going to happen with this feature?
> If it remains enabled, and the fix isn't backported to llvm 3.6, this bug
> will end up being present in a stable release (unless llvm 3.7 is also going
> to be released soon).
> 
> Fez is also affected by the same bug (I tested and verified that it is fixed
> by the same patch), so there may be many more games affected:
> 
> Good (mesa 10.5): https://www.dropbox.com/s/h7qtlgln1xlnxrk/fez_good.png?dl=0
> Bad (mesa 10.6): https://www.dropbox.com/s/f5yqqx2l6pz284z/fez_bad.png?dl=0

This bug will be fixed in the LLVM 3.6.1 release.