| Bug ID | 96488 |
|---|---|
| Summary | [r600g]OpenCL driver causes segfault in ImageMagick's Histogr= am kernel |
| Product | Mesa |
| Version | git |
| Hardware | Other |
| OS | All |
| Status | NEW |
| Severity | normal |
| Priority | medium |
| Component | Drivers/Gallium/r600 |
| Assignee | dri-devel@lists.freedesktop.org |
| Reporter | nixscripter@gmail.com |
| QA Contact | dri-devel@lists.freedesktop.org |
Created attachment 12= 4469 [details] Histogram kernel backtrace If I compile ImageMagick with OpenCL support, the current version segfaults when I attempt to do any operation which computes a histogram. This is actu= ally done frequently internally, so it's more crippling than it first appears. To reproduce: 1. Get the current latest ImageMagick from GitHub: https://github.com/I= mageMagick/ImageMagick 2. Compile it with OpenCL support (--enable-opencl flag) 3. Get an image, and try to equalize its colors (based on the histogram): convert input.png -equalize output.png 4. The segfault will occur LLVM version: r272184 Mesa version: commit d5491a8 ImageMagick version: ade0d8e The source code for the Histogram kernel is in MagickCore/accelerate-kernels-private.h, starting on line 1283.
| What | Removed | Added |
|---|---|---|
| OS | All | Linux (All) |
| Hardware | Other | x86-64 (AMD64) |
That's because global atomics are not implemented in LLVM back= end (despite driver advertising the extensions). Shouldn't be too hard to them.
(In reply to jano.vesely from comment #1) > That's because global atomics are not implemente= d in LLVM backend (despite > driver advertising the extensions). > Shouldn't be too hard to them. Are you sure the hardware has the necessary instructions? Tom Stellard commented on #radeon back in November that implementing global atomics for r600g will be very very difficult[1]. [1] https://people.freedesktop.org/~cbrill/dri= -log/index.php?date=3D2015-11-27&channel=3Dradeon
I suspected it was that, based on the word "Atomic" = being at fault. Unfortunately, I cannot assist, as I'm not an expert on parallel architectu= re and spending the last half hour reading AMD's documentation won't make me o= ne. But I would ask one question: is anyone even working on this? I'd hate to w= rite off ImageMagick on this platform.
The instructions are there for EG+. They're tricky to implemen= t because of the register requirements. However, there might be some weird interaction when using CB with the current stores, and loads from the same WI. I had a version that passed noret variant of global atomic piglit tests, but I'm not sure piglit tests cover all situations.
Can you dig the patch somewhere? I have a Caicos at hand and I= would really like to test atomic compare-and-swap.
(In reply to Vedran Mileti=C4=87 from comment #5) > Can you dig the patch somewhere? I have a Caicos= at hand and I would really > like to test atomic compare-and-swap. you can try my github branch [0], you'll need the last 5 patches (2-3 can be skipped since it's just revert). but I rebase a lot so things might brake. It passes noret piglit tests, and _noret should work OK, _return variants w= ill give LLVM ERROR: Cannot select. uncommenting the FIXME parts enables return variants, but it will brake for more than one WI as every WI uses address 0 as scratchpad. Jan [0] https://github.com/jvesely/= llvm
Normally, I would be asking in this comment if there is any pr= ogress on this, but I can tell there has been... change, at least. Now instead of crashing, it deadlocks the calling process waiting on a mute= x. Forever, as far as I can tell. LLVM version: r289770 Mesa version: commit 47351b843a Perhaps this is more easily fixed? Or is this getting to the heart of the matter implied in comment 2?
(In reply to nixscripter from comment #7) > Normally, I would be asking in this comment if t= here is any progress on > this, but I can tell there has been... change, at least. >=20 > Now instead of crashing, it deadlocks the calling process waiting on a > mutex. Forever, as far as I can tell. >=20 > LLVM version: r289770 > Mesa version: commit 47351b843a >=20 > Perhaps this is more easily fixed? Or is this getting to the heart of = the > matter implied in comment 2= ? the hang is probably a separate bug. ImageMagick test suite results on my T= urks GPU are: # TOTAL: 86 # PASS: 78 # SKIP: 0 # XFAIL: 0 # FAIL: 3 # XPASS: 0 # ERROR: 5 the errors and failures are accompanied by: Assertion `i < getNumRegs() && "Register number out of rang= e!"' failed. The patches to add support for noret atomics (should be good enough for histogram) are at: https://reviews.llvm.org/D27989= https://reviews.llvm.org/D28067=
I have downloaded LLVM r290690 which contains your first diff,= manually applied your 2nd diff, and compiled it. The resulting library seems to resolve the issue. Once that patch is reviewed and applied to the mainline, I will do one final verification and (hopefully) close this bug. Thanks for all your efforts on this!
(In reply to nixscripter from comment #9) > I have downloaded LLVM r290690 which contains yo= ur first diff, manually > applied your 2nd diff, and compiled it. The resulting library seems to > resolve the issue. >=20 > Once that patch is reviewed and applied to the mainline, I will do one= final > verification and (hopefully) close this bug. >=20 > Thanks for all your efforts on this! I have just landed the patch. Note that if the kernel uses sub-int(short,ch= ar) vectors. it will generate incorrect code until D27964 (or alternative) is merged. Jan
| What | Removed | Added |
|---|---|---|
| Resolution | --- | FIXED |
| Status | NEW | RESOLVED |
I have just tried LLVM r292714, and it does indeed work as adv= ertised. Marking RESOLVED FIXED.