rampitec added inline comments.
================ Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll:4 +; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1013 -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s +; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s ---------------- bcahoon wrote: > rampitec wrote: > > bcahoon wrote: > > > rampitec wrote: > > > > rampitec wrote: > > > > > foad wrote: > > > > > > This test surely should not pass for gfx1012, since it does not > > > > > > have these instructions. And with your patch as written it should > > > > > > fail for gfx1013 too, since they are predicated on > > > > > > HasGFX10_BEncoding. > > > > > > > > > > > > @rampitec any idea what is wrong here? Apparently the backend will > > > > > > happily generate image_bvh_intersect_ray instructions even for > > > > > > gfx900! > > > > > Indeed. MIMG_IntersectRay has this: > > > > > > > > > > ``` > > > > > let SubtargetPredicate = HasGFX10_BEncoding, > > > > > AssemblerPredicate = HasGFX10_BEncoding, > > > > > ``` > > > > > but apparently SubtargetPredicate did not work. It needs to be fixed. > > > > > > > > > > gfx1012 does not have it, gfx1013 does though. That is what GFX10_A > > > > > encoding is about, 10_B it has to be replaced with 10_A in BVH and > > > > > MSAA load. > > > > Image lowering and selection is not really done like everything else. > > > > For BVH it just lowers intrinsic to opcode. I think the easiest fix is > > > > to add to SIISelLowering.cpp where we lower > > > > Intrinsic::amdgcn_image_bvh_intersect_ray something like this: > > > > > > > > > > > > ``` > > > > if (!Subtarget->hasGFX10_AEncoding()) > > > > report_fatal_error( > > > > "requested image instruction is not supported on this GPU"); > > > > ``` > > > I ended up using emitRemovedIntrinsicError, which uses > > > DiagnosticInfoUnsupported. This way the failure isn't a crash dump. > > > I ended up using emitRemovedIntrinsicError, which uses > > > DiagnosticInfoUnsupported. This way the failure isn't a crash dump. > > > > Diagnostics is a good thing, but we still have to fail the compilation. > The diagnostic is marked as an error, so the compilation fails in that llc > returns a non-zero return code. This mechanism is used in other places in the > back-end to report similar types of errors. The alternative, if I understand > correctly, is that a crash occurs with an error message that indicates that > the bug is in LLVM (rather the the input source file). We do not seem to be consistent here and return either undef or SDValue(), but as far as I can see we never continue selecting code though, like here in SIISelLowering and always return false from the AMDGPUInstructionSelector. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103663/new/ https://reviews.llvm.org/D103663 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
