Re: [Piglit] [PATCH] cl: Add test for MUBUF access with a negative vaddr
On Fri, 2018-01-19 at 14:43 -0500, Matt Arsenault wrote: > > On Jan 18, 2018, at 15:02, Jan Vesely wrote: > > > > Why is this necessary? can't you just pass the offset argument as a > > kernel input? > > > > Jan > > It needs to specifically be in a VGPR OK LGTM. I'll push it in the next few days. wouldn't the same be achieved by adding get_local_id(0)? I'm trying to make these tests more interesting for non-amd targets to avoid adding bloat to the testing runs. maybe the right approach would be to have a separate amd specific list of tests that don't run by default (but still pass) on other devices. Jan -- Jan Vesely signature.asc Description: This is a digitally signed message part ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] cl: Add test for MUBUF access with a negative vaddr
> On Jan 18, 2018, at 15:02, Jan Vesely wrote: > > Why is this necessary? can't you just pass the offset argument as a > kernel input? > > Jan It needs to specifically be in a VGPR___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] cl: Add test for MUBUF access with a negative vaddr
On Tue, 2017-11-28 at 14:20 -0800, Matt Arsenault wrote: > Explanation in test comment. > --- > .../program/execute/amdgcn-mubuf-negative-vaddr.cl | 62 > ++ > 1 file changed, 62 insertions(+) > create mode 100644 tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > > diff --git a/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > new file mode 100644 > index 0..21f11bf66 > --- /dev/null > +++ b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > @@ -0,0 +1,62 @@ > +>/*! > + > +[config] > +name: MUBUF stack addressing behavior > +clc_version_min: 10 > + > +[test] > +name: MUBUF negative buffer offsets > +kernel_name: negative_mubuf_vaddr > +dimensions: 1 > +global_size: 16 0 0 > + > +arg_out: 0 buffer int[16]\ > + 5 5 5 5 \ > + 5 5 5 5 \ > + 5 5 5 5 \ > + 5 5 5 5 > + > +!*/ > + > +// Prior to gfx9, MUBUF instructions with the vaddr offset enabled > +// would always perform a range check. If a negative vaddr base index > +// was used, this would fail the range check. The overall address > +// computation would compute a valid address, but this doesn't happen > +// due to the range check. For out-of-bounds MUBUF loads, a 0 is > +// returned. > +// > +// Therefore it should be safe to fold any VGPR offset on gfx9 into > +// the MUBUF vaddr, but not on older subtargets which can only do this > +// if the sign bit is known 0. > +kernel void negative_mubuf_vaddr(global int* out0) > +{ > +volatile int array[16]; > + > +int id = get_global_id(0); > +for (int i = 0; i < 16; ++i) > +{ > +array[i] = i + 1; > +} > + > +// Directly addressing the same buffer address works without using vaddr: > +// > +// buffer_load_dword v2, off, s[0:3], s11 offset:20 > +// out0[id] = array[4]; > + > + > +// But having a negative computed base index would fail: > +// v_mov_b32_e32 v0, -8 > +// v_lshlrev_b32_e32 v0, 2, v0 > +// v_add_i32_e32 v0, vcc, 4, v0 > +// buffer_load_dword v2, v0, s[0:3], s11 offen offset:48 > + > +#ifdef __AMDGCN__ > +// Obscure the value so it can't be folded with other constant or > +// make known bits assumptions. > +int offset; > +__asm volatile("v_mov_b32 %0, -8" : "=v"(offset)); Why is this necessary? can't you just pass the offset argument as a kernel input? Jan > +#else > +int offset = -8; > +#endif > +out0[id] = array[offset + 12]; > +} signature.asc Description: This is a digitally signed message part ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] cl: Add test for MUBUF access with a negative vaddr
ping > On Dec 15, 2017, at 14:01, Matt Arsenault wrote: > > ping > >> On Nov 28, 2017, at 17:20, Matt Arsenault wrote: >> >> Explanation in test comment. >> --- >> .../program/execute/amdgcn-mubuf-negative-vaddr.cl | 62 >> ++ >> 1 file changed, 62 insertions(+) >> create mode 100644 tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl >> >> diff --git a/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl >> b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl >> new file mode 100644 >> index 0..21f11bf66 >> --- /dev/null >> +++ b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl >> @@ -0,0 +1,62 @@ >> +>/*! >> + >> +[config] >> +name: MUBUF stack addressing behavior >> +clc_version_min: 10 >> + >> +[test] >> +name: MUBUF negative buffer offsets >> +kernel_name: negative_mubuf_vaddr >> +dimensions: 1 >> +global_size: 16 0 0 >> + >> +arg_out: 0 buffer int[16]\ >> + 5 5 5 5 \ >> + 5 5 5 5 \ >> + 5 5 5 5 \ >> + 5 5 5 5 >> + >> +!*/ >> + >> +// Prior to gfx9, MUBUF instructions with the vaddr offset enabled >> +// would always perform a range check. If a negative vaddr base index >> +// was used, this would fail the range check. The overall address >> +// computation would compute a valid address, but this doesn't happen >> +// due to the range check. For out-of-bounds MUBUF loads, a 0 is >> +// returned. >> +// >> +// Therefore it should be safe to fold any VGPR offset on gfx9 into >> +// the MUBUF vaddr, but not on older subtargets which can only do this >> +// if the sign bit is known 0. >> +kernel void negative_mubuf_vaddr(global int* out0) >> +{ >> +volatile int array[16]; >> + >> +int id = get_global_id(0); >> +for (int i = 0; i < 16; ++i) >> +{ >> +array[i] = i + 1; >> +} >> + >> +// Directly addressing the same buffer address works without using >> vaddr: >> +// >> +// buffer_load_dword v2, off, s[0:3], s11 offset:20 >> +// out0[id] = array[4]; >> + >> + >> +// But having a negative computed base index would fail: >> +// v_mov_b32_e32 v0, -8 >> +// v_lshlrev_b32_e32 v0, 2, v0 >> +// v_add_i32_e32 v0, vcc, 4, v0 >> +// buffer_load_dword v2, v0, s[0:3], s11 offen offset:48 >> + >> +#ifdef __AMDGCN__ >> +// Obscure the value so it can't be folded with other constant or >> +// make known bits assumptions. >> +int offset; >> +__asm volatile("v_mov_b32 %0, -8" : "=v"(offset)); >> +#else >> +int offset = -8; >> +#endif >> +out0[id] = array[offset + 12]; >> +} >> -- >> 2.11.0 >> > ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] cl: Add test for MUBUF access with a negative vaddr
ping > On Nov 28, 2017, at 17:20, Matt Arsenault wrote: > > Explanation in test comment. > --- > .../program/execute/amdgcn-mubuf-negative-vaddr.cl | 62 ++ > 1 file changed, 62 insertions(+) > create mode 100644 tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > > diff --git a/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > new file mode 100644 > index 0..21f11bf66 > --- /dev/null > +++ b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl > @@ -0,0 +1,62 @@ > +>/*! > + > +[config] > +name: MUBUF stack addressing behavior > +clc_version_min: 10 > + > +[test] > +name: MUBUF negative buffer offsets > +kernel_name: negative_mubuf_vaddr > +dimensions: 1 > +global_size: 16 0 0 > + > +arg_out: 0 buffer int[16]\ > + 5 5 5 5 \ > + 5 5 5 5 \ > + 5 5 5 5 \ > + 5 5 5 5 > + > +!*/ > + > +// Prior to gfx9, MUBUF instructions with the vaddr offset enabled > +// would always perform a range check. If a negative vaddr base index > +// was used, this would fail the range check. The overall address > +// computation would compute a valid address, but this doesn't happen > +// due to the range check. For out-of-bounds MUBUF loads, a 0 is > +// returned. > +// > +// Therefore it should be safe to fold any VGPR offset on gfx9 into > +// the MUBUF vaddr, but not on older subtargets which can only do this > +// if the sign bit is known 0. > +kernel void negative_mubuf_vaddr(global int* out0) > +{ > +volatile int array[16]; > + > +int id = get_global_id(0); > +for (int i = 0; i < 16; ++i) > +{ > +array[i] = i + 1; > +} > + > +// Directly addressing the same buffer address works without using vaddr: > +// > +// buffer_load_dword v2, off, s[0:3], s11 offset:20 > +// out0[id] = array[4]; > + > + > +// But having a negative computed base index would fail: > +// v_mov_b32_e32 v0, -8 > +// v_lshlrev_b32_e32 v0, 2, v0 > +// v_add_i32_e32 v0, vcc, 4, v0 > +// buffer_load_dword v2, v0, s[0:3], s11 offen offset:48 > + > +#ifdef __AMDGCN__ > +// Obscure the value so it can't be folded with other constant or > +// make known bits assumptions. > +int offset; > +__asm volatile("v_mov_b32 %0, -8" : "=v"(offset)); > +#else > +int offset = -8; > +#endif > +out0[id] = array[offset + 12]; > +} > -- > 2.11.0 > ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH] cl: Add test for MUBUF access with a negative vaddr
Explanation in test comment. --- .../program/execute/amdgcn-mubuf-negative-vaddr.cl | 62 ++ 1 file changed, 62 insertions(+) create mode 100644 tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl diff --git a/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl new file mode 100644 index 0..21f11bf66 --- /dev/null +++ b/tests/cl/program/execute/amdgcn-mubuf-negative-vaddr.cl @@ -0,0 +1,62 @@ +>/*! + +[config] +name: MUBUF stack addressing behavior +clc_version_min: 10 + +[test] +name: MUBUF negative buffer offsets +kernel_name: negative_mubuf_vaddr +dimensions: 1 +global_size: 16 0 0 + +arg_out: 0 buffer int[16]\ + 5 5 5 5 \ + 5 5 5 5 \ + 5 5 5 5 \ + 5 5 5 5 + +!*/ + +// Prior to gfx9, MUBUF instructions with the vaddr offset enabled +// would always perform a range check. If a negative vaddr base index +// was used, this would fail the range check. The overall address +// computation would compute a valid address, but this doesn't happen +// due to the range check. For out-of-bounds MUBUF loads, a 0 is +// returned. +// +// Therefore it should be safe to fold any VGPR offset on gfx9 into +// the MUBUF vaddr, but not on older subtargets which can only do this +// if the sign bit is known 0. +kernel void negative_mubuf_vaddr(global int* out0) +{ +volatile int array[16]; + +int id = get_global_id(0); +for (int i = 0; i < 16; ++i) +{ +array[i] = i + 1; +} + +// Directly addressing the same buffer address works without using vaddr: +// +// buffer_load_dword v2, off, s[0:3], s11 offset:20 +// out0[id] = array[4]; + + +// But having a negative computed base index would fail: +// v_mov_b32_e32 v0, -8 +// v_lshlrev_b32_e32 v0, 2, v0 +// v_add_i32_e32 v0, vcc, 4, v0 +// buffer_load_dword v2, v0, s[0:3], s11 offen offset:48 + +#ifdef __AMDGCN__ +// Obscure the value so it can't be folded with other constant or +// make known bits assumptions. +int offset; +__asm volatile("v_mov_b32 %0, -8" : "=v"(offset)); +#else +int offset = -8; +#endif +out0[id] = array[offset + 12]; +} -- 2.11.0 ___ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit