bcahoon marked 8 inline comments as done.
bcahoon added inline comments.
================
Comment at: llvm/docs/AMDGPUUsage.rst:389
- xnack
scratch - *pal-amdpal*
+ ``gfx1013`` ``amdgcn`` dGPU - cumode -
Absolute - *rocm-amdhsa* *TBA*
+ - wavefrontsize64 flat
- *pal-amdhsa*
----------------
foad wrote:
> Is it dGPU or APU?
>
> Every other entry with `*TBA*` also has a `TODO::` message
It is APU.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:468
+def FeatureGFX10_AEncoding : SubtargetFeature<"gfx10_a-encoding",
+ "GFX10_AEncoding",
----------------
foad wrote:
> What is this new encoding? It doesn't seem to be used for anything.
Fixed this. The BVH raytracing instructions use the encoding.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:1106
[FeatureGFX10,
FeatureGFX10_BEncoding,
FeatureGFX10_3Insts,
----------------
rampitec wrote:
> gfx1030 should now include FeatureGFX10_AEncoding as well. 10_B is an
> extension above 10_A.
I had added FeatureGFX10_AEncoding as an Implies feature for
FeatureGFX10_BEncoding in the previous patch. But, I've changed the patch so
that it's no longer an Implies feature.
================
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:
> 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.
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
