Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unaligned testcases fail on arm #134

Closed
juliantaylor opened this issue Feb 29, 2012 · 3 comments
Closed

unaligned testcases fail on arm #134

juliantaylor opened this issue Feb 29, 2012 · 3 comments
Assignees
Milestone

Comments

@juliantaylor
Copy link

I have disabled the blosc compression which does not work on architectures without unaligned memory access.
Unfortunately the testsuite still fails at "Comparing written unaligned time data with read data in a Table" tests:
https://launchpadlibrarian.net/94731766/buildlog_ubuntu-precise-armel.pytables_2.3.1-2ubuntu2_FAILEDTOBUILD.txt.gz

do these tests require unaligned memory access too?

@FrancescAlted
Copy link
Member

On Feb 28, 2012, at 11:33 PM, Julian Taylor wrote:

I have disabled the blosc compression which does not work on architectures without unaligned memory access.

That's good. Thanks.

Unfortunately the testsuite still fails at "Comparing written unaligned time data with read data in a Table" tests:
https://launchpadlibrarian.net/94731766/buildlog_ubuntu-precise-armel.pytables_2.3.1-2ubuntu2_FAILEDTOBUILD.txt.gz

do these tests require unaligned memory access too?

Yes, they do. What I don't quite understand is that this test is there since 2006, and it always passed tests on many different platforms. But anyway, feel free to produce a patch so that this test can be skipped.

-- Francesc Alted

@juliantaylor
Copy link
Author

I don't see how that test could ever have succeeded, except for random chance. There is even a comment in the problematic file that it will break.

in the mean time I have applied this stupid patch to the problematic file to avoid the unaligned access.
With it and disabled blosc now the whole testsuite succeeds on a sparc64 machine and it will hopefully also work on other arches like arm.

--- pytables-2.3.1.orig/src/typeconv.c  2011-10-28 22:11:14.000000000 +0200
+++ pytables-2.3.1/src/typeconv.c       2012-03-01 00:52:34.000000000 +0200
@@ -66,18 +66,21 @@

   for (record = 0;  record < nrecords;  record++) {
     for (element = 0;  element < nelements;  element++) {
+      double fb;
+      memcpy(&fb, fieldbase, sizeof(*fieldbase));
       if (sense == 0) {
        /* Convert from float64 to timeval32. */
-       tv.i64 = (((PY_LONG_LONG)(*fieldbase) << 32)
-                 | (lround((*fieldbase - (int)(*fieldbase)) * 1e+6)
+       tv.i64 = (((PY_LONG_LONG)(fb) << 32)
+                 | (lround((fb - (int)(fb)) * 1e+6)
                     & 0x0ffffffff));
-       *fieldbase = tv.f64;
+       fb = tv.f64;
       } else {
        /* Convert from timeval32 to float64. */
-       tv.f64 = *fieldbase;
+       tv.f64 = fb;
        /* the next computation is 64 bit-platforms aware */
-       *fieldbase = 1e-6 * (int)tv.i64 + (tv.i64 >> 32);
+       fb = 1e-6 * (int)tv.i64 + (tv.i64 >> 32);
       }   
+      memcpy(fieldbase, &fb, sizeof(*fieldbase));
       fieldbase++;
     }   

@FrancescAlted
Copy link
Member

On Feb 29, 2012, at 4:06 PM, Julian Taylor wrote:

I don't see how that test could ever have succeeded, except for random chance. There is even a comment in the problematic file that it will break.

Yeah, it should have been probably just random chance, but still...

in the mean time I have applied this stupid patch to the problematic file to avoid the unaligned access.

That's great. I'm fine with the patch, and should be definitely applied to upstream too.

With it and disabled blosc now the whole testsuite succeeds on a sparc64 machine and it will hopefully also work on other arches like arm.

Excellent! Thanks for your contribution!

-- Francesc Alted

@ghost ghost assigned avalentino Mar 3, 2012
avalentino added a commit to avalentino/PyTables that referenced this issue Mar 3, 2012
avalentino added a commit to avalentino/PyTables that referenced this issue Mar 3, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants