GNOME Bugzilla – Bug 739354
program-c: loadpq broken on big-endian systems (makes gst volume tests fail for f64)
Last modified: 2014-11-13 16:19:19 UTC
In -base/tests/check/elements/volume.c the f64 format tests fail on powerpc because orc generates broken backup code: -base/gst/volume/tmp-orc.c: static void _backup_volume_orc_scalarmultiply_f64_ns (OrcExecutor * ORC_RESTRICT ex) { ... orc_union64 var33; /* 1: loadpq */ var33.i = (ex->params[24] & 0xffffffff) | ((orc_uint64)(ex->params[24 + (ORC_VAR_T1 - ORC_VAR_P1)]) << 32); for (i = 0; i < n; i++) { ... } } void volume_orc_scalarmultiply_f64_ns (double * ORC_RESTRICT d1, double p1, int n) { ... ex->arrays[ORC_VAR_A2] = c; ex->program = 0; ex->n = n; ex->arrays[ORC_VAR_D1] = d1; { orc_union64 tmp; tmp.f = p1; ex->params[ORC_VAR_P1] = tmp.x2[0]; <==== ex->params[ORC_VAR_T1] = tmp.x2[1]; <==== } .... } It doesn't save/restore the 64-bit double by which to multiply all the samples with correctly on big-endian architectures, the higher/upper 32-bits are swapped, which makes everything something around 0 after processing. What I'm not sure about is whether the value is stored incorrectly, or read incorrectly. There's a comment in struct _OrcExecutor saying /* high half of params is stored in params[ORC_VAR_T1..] */ which I would interpret such that the values should be stored differently on big-endian. (But then I don't really understand why they're stored that way via .x2 in the first place I suppose).
Created attachment 290214 [details] [review] orcc: program-c: fix 64-bit parameter loading (loadpq) on big-endian systems When passing 64-bit parameters through OrcExecutor, we have to split them up into two 32-bit parameters for backwards compatibility reasons. When generating C code, make sure that we split up 64-bit parameters in the same way as loadpq will read them back later. The lower 32 bits should end up in params[ORC_VAR_D1+i] and the higher bits should end up in params[ORC_VAR_T1+i]. The way it was done so far, the higher/lower bits ended up swapped on big endian systems, and then got deserialised in swapped order by loadpq. This resulted in bogus parameters being used. In particular, this broke the gstreamer volume element and its unit tests on big endian systems when handling samples in F64 format (i.e. doubles). --- Tested on powerpc32 and 64-bit x86 with gst-plugins-base from git.
commit 463295f6486badafa389008c0339386e2bf9d4c9 Author: Tim-Philipp Müller <tim@centricular.com> Date: Sat Nov 8 13:23:12 2014 +0000 orcc: program-c: fix 64-bit parameter loading (loadpq) on big-endian systems When passing 64-bit parameters through OrcExecutor, we have to split them up into two 32-bit parameters for backwards compatibility reasons. When generating C code, make sure that we split up 64-bit parameters in the same way as loadpq will read them back later. The lower 32 bits should end up in params[ORC_VAR_D1+i] and the higher bits should end up in params[ORC_VAR_T1+i]. The way it was done so far, the higher/lower bits ended up swapped on big endian systems, and then got deserialised in swapped order by loadpq. This resulted in bogus parameters being used. In particular, this broke the gstreamer volume element and its unit tests on big endian systems when handling samples in F64 format (i.e. doubles). https://bugzilla.gnome.org/show_bug.cgi?id=739354