bcahoon marked 4 inline comments as done. bcahoon 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 ---------------- 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). 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
