rampitec added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:4697
+ if (!ST.hasGFX10_AEncoding()) {
+ DiagnosticInfoUnsupported BadIntrin(B.getMF().getFunction(), "intrinsic
not supported on subtarget",
+ MI.getDebugLoc());
----------------
80 chars per line.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:4700
+ B.getMF().getFunction().getContext().diagnose(BadIntrin);
+ B.buildUndef(MI.getOperand(0));
+ MI.eraseFromParent();
----------------
Just return false like in other places.
================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:7341
+ if (!Subtarget->hasGFX10_AEncoding())
+ emitRemovedIntrinsicError(DAG, DL, Op.getValueType());
+
----------------
return emitRemovedIntrinsicError();
================
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: not llc -global-isel -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs
< %s -o /dev/null 2>&1 | FileCheck -check-prefix=ERR %s
----------------
not --crash llc ...
================
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:
> > 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.
================
Comment at: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll:3
+; RUN: llc -march=amdgcn -mcpu=gfx1013 -verify-machineinstrs < %s | FileCheck
-check-prefix=GCN %s
+; RUN: not llc -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s 2>&1 |
FileCheck -check-prefix=ERR %s
----------------
not --crash llc
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
