Bug 106231 - llvmpipe blends produce bad code after llvm patch https://reviews.llvm.org/D44785
Summary: llvmpipe blends produce bad code after llvm patch https://reviews.llvm.org/D4...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/llvmpipe (show other bugs)
Version: 18.0
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
: 107653 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-04-25 08:23 UTC by Tom Hudson
Modified: 2018-08-24 06:02 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 Tom Hudson 2018-04-25 08:23:58 UTC
https://reviews.llvm.org/D44785 changed the way adds, addus, subs, subus are handled.

llvmpipe issues llvm.x86.sse2.padds and llvm.x86.sse2.psubs in src/gallium/auxiliary/gallivm/lp_bld_arit.c:lp_build_add() and lp_build_sub().

After D44785 landed, lp_test_blend.c started crash every time it entered LLVM-compiled code for type=u8nx16.

Commenting out the issues of padds/psubs avoids this crash. The LLVM project, in discussing the bug at https://reviews.llvm.org/D44785, suspects that the cause may be because llvmpipe is "missing the autoupgrade stage"?
Comment 1 Roland Scheidegger 2018-04-25 19:08:27 UTC
(In reply to Tom Hudson from comment #0)
> https://reviews.llvm.org/D44785 changed the way adds, addus, subs, subus are
> handled.
> 
> llvmpipe issues llvm.x86.sse2.padds and llvm.x86.sse2.psubs in
> src/gallium/auxiliary/gallivm/lp_bld_arit.c:lp_build_add() and
> lp_build_sub().
> 
> After D44785 landed, lp_test_blend.c started crash every time it entered
> LLVM-compiled code for type=u8nx16.
> 
> Commenting out the issues of padds/psubs avoids this crash. The LLVM
> project, in discussing the bug at https://reviews.llvm.org/D44785, suspects
> that the cause may be because llvmpipe is "missing the autoupgrade stage"?

Autoupgrade doesn't work for jit code (at least I wouldn't know how, and it never has in the past), so the way we handled disappearing of intrinsics in the past was to just not use them any more (for newer llvm versions) and do essentially the same as what autoupgrade would do, and we'll have to do the same here.

I am however sure that in the past when intrinsics disappeared, it would complain when compiling the IR, rather than just call 0 function in the compiled code. Which is of course much nicer...
For instance when the min/max (integer) intrinsics disappared:
https://bugs.llvm.org/show_bug.cgi?id=28176
But I'm not sure if just calling 0 function now is expected due to some llvm changes, but if it is that's definitely making everybody's life harder...
Comment 2 Roland Scheidegger 2018-04-25 22:48:57 UTC
FWIW I won't have time to look into this until next week, so volunteers welcome.
Not using the intrinsics for new llvm version is trivial, but we need to update our existing code (when intrinsics can't be used) to match what the auto-upgrader would do, otherwise we'll probably get really crappy code (at least I suspect in this case llvm won't recognize our patterns), which is of course the reason why we used intrinsics in the first place...
(Can't really say I'm all that happy to see the intrinsics go, as the code to emulate it is rather complex with 8 instructions, thus blowing up IR size and likely compile time as well, but I guess that's life when compilers getting smarter...)
Comment 3 Roland Scheidegger 2018-08-22 01:33:18 UTC
*** Bug 107653 has been marked as a duplicate of this bug. ***
Comment 4 Roland Scheidegger 2018-08-24 06:02:11 UTC
Fixed by 8e1be9a34ac8ce6f115eaf2ab0d99b6a0ce37630.


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.