#11920: Sympow needs to disable fused-multiply-add and should create datafiles
------------------------------+---------------------------------------------
Reporter: jdemeyer | Owner: tbd
Type: defect | Status: needs_review
Priority: major | Milestone: sage-5.0
Component: packages | Keywords:
Work_issues: | Upstream: None of the above - read trac
for reasoning.
Reviewer: Leif Leonhardy | Author: Jeroen Demeyer
Merged: | Dependencies:
------------------------------+---------------------------------------------
Comment(by leif):
Replying to [comment:25 leif]:
> P.S.: I can perhaps (re-)review the new spkg tomorrow evening; I'm not
sure whether there've been changes since I last looked at it.
At first glance looks ok, modulo some cosmetic inconsistencies and the
like (in `spkg-install`):
* Indentation
* "Sympow" vs. "sympow" vs. "SYMPOW"; `SPKG.txt` (mostly) uses the
latter.
* Not all error messages are redirected to stderr.
* Some error messages don't start with "`Error`" (and don't contain the
word elsewhere).
* Superfluous `if [ -d $FOO ]; then rm -rf $FOO; fi` (instead of just `rm
-rf $FOO`) etc.
* The viral trailing semicolon in
{{{
#!sh
echo "SAGE_LOCAL undefined ... exiting";
}}}
* A few typos (either ''"[fused] multiply-a'''d'''d instructions"'' or
''"multiply-and'''-add'''..."'' -- also without a hyphen before
''"instructions"''; ''"almost surely on a'''n''' x86 system"'', ...?)
A little more relevant:
* To go triple safe, you could add `-fno-fast-math` to your `FLAG` list.
* `$SCRIPT` (which contains `$SAGE_LOCAL`) should be quoted near the end
of `spkg-install`; perhaps also `$file`, which in contrast just depends on
Sympow['s data file names], not a user setting.
* The generated script should perhaps test `$SAGE_LOCAL` to give a
meaningful error message in case it is undefined.
* `if [ ! -f "$TARGET/sympow" ] ...` could be `if [ ! -x "$TARGET/sympow"
] ...`.
[[BR]]
I cannot (re)test this much; will probably do on cleo, perhaps silius, and
some "unaffected" systems; interesting would also be newer Intel CPUs with
AVX, although the early Sandy Bridges (for example) do not support
multiply-accumulate IIRC.
I also haven't looked at the new C code recently, assuming it hasn't
changed since I last did (a few month ago).
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11920#comment:30>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
