Roland Scheidegger <srol...@vmware.com> writes: > Am 08.08.2014 22:56, schrieb Neil Roberts: >> The i965 driver uses a float pointer to point to the value of a uniform and >> also as the destination within the batch buffer. However the same locations >> can also be used to store values for integer uniforms. Previously the value >> was being copied into the batch buffer with a regular assignment. This breaks >> if the compiler does this by loading and saving through an x87 register >> because the fst instruction tries to fix up invalid float values. That can >> corrupt some specific integer values. This patch changes it to use a memcpy >> instead so that it won't use a floating-point register. >> >> Bugzilla: >> https://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D81150&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=5ZyizVpb8bGGN4qANo378HB6GFoFoBABoDXdPAyIszE%3D%0A&s=107a04b60233e66abfb9e83a62b90d128d60d6c61eb41b1a08ec7ab537283903 >> --- >> src/mesa/drivers/dri/i965/gen6_vs_state.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c >> b/src/mesa/drivers/dri/i965/gen6_vs_state.c >> index 905e123..cdbc083 100644 >> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c >> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c >> @@ -81,7 +81,13 @@ gen6_upload_push_constants(struct brw_context *brw, >> * wouldn't be set for them. >> */ >> for (i = 0; i < prog_data->nr_params; i++) { >> - param[i] = *prog_data->param[i]; >> + /* A memcpy is used here instead of a regular assignment because >> + * otherwise the value may end up being copied through a >> + * floating-point register. This will break if the x87 registers >> are >> + * used and we are storing an integer value here because the fst >> + * instruction tries to fix up invalid values and that would >> corrupt >> + * an integer value */ >> + memcpy(param + i, prog_data->param[i], sizeof param[i]); >> }
Or just change the pointer type to uint32_t, right? > Wow, crazy. > Maybe it would make sense to just use a void pointer and do a single > memcpy instead of looping through all params? > I wonder why this isn't hit elsewhere, I'm pretty sure there's other > places (not necessarily in i965 driver) which assign potential int data > through floats. Seems to me the compiler is pretty crazy to use fst > though to begin with... Well, the uniforms aren't in a single linear allocation. You need to gather them from their separate storage locations.
pgp2jarUSx2iW.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
