I have wrote a patch some time ago, sorry for not posting
it earlier.
https://github.com/oldk1331/fricas/commit/77b412ff71ac90de496be99c6ab3dd3c7e04eea8.patch
[PATCH] don't apply ill-formed inline optimization
diff --git a/src/interp/nruncomp.boot b/src/interp/nruncomp.boot
index b5f18ba7..56330d23 100644
--- a/src/interp/nruncomp.boot
+++ b/src/interp/nruncomp.boot
@@ -155,7 +155,29 @@ optDeltaEntry(op,sig,dc,eltOrConst) ==
fn := compiledLookup(op,nsig,dcval)
if null fn then return nil
eltOrConst="CONST" => ['XLAM,'ignore,MKQ SPADCALL fn]
- GETL(compileTimeBindingOf first fn,'SPADreplace)
+ spadreplace := GETL(compileTimeBindingOf first fn,'SPADreplace)
+ if CONSP spadreplace and first spadreplace = 'XLAM then
+ -- if the optimization is a XLAM form, make sure it's a "proper macro",
+ -- i.e. doesn't ignore its argument or evaluate it more than once.
+ lhs := CADR spadreplace
+ rhs := CADDR spadreplace
+ if # lhs = 1 and countXLAM(var := first lhs, rhs) = 0 then
+ -- deal with cases like "minIndex l == 0", which translates to
+ -- "(XLAM (|l|) 0)", prevents argument from evaluation.
+ return ['XLAM, lhs, ['PROGN, var, rhs]]
+ for var in lhs repeat
+ -- ignore argument that is string, e.g. 'elt(x, "first")'
+ if not STRINGP var and (n := countXLAM(var, rhs)) ~= 1 then
+ -- in current code base there are no cases like "f(x, y) == x"
+ -- so throw an error if such case emerges.
+ stackAndThrow [op, " can not be properly inline optimized"]
+ return nil
+ spadreplace
+
+countXLAM(var, rhs) ==
+ -- return how many times does var appear in rhs
+ not CONSP rhs => if var = rhs then 1 else 0
+ COUNT(var, rhs)
genDeltaEntry opMmPair ==
--called from compApplyModemap
On Sat, Jan 13, 2018 at 1:09 AM, Waldek Hebisch
<[email protected]> wrote:
>
> This should be easily fixable by change to inlining: assign
> argument to new symbol and then pass the symbol to XLAM.
> Actually the "minIndex(f := empty())" case does not need
> a new symbol, during inlining Spad compiler should see 'f'
> and pass it to XLAM.
For the case of "minIndex", we don't need to pass anything,
because it doesn't use the argument. So a simple "progn"
will do, as I wrote in the patch.
> So we need new symbol only in cases
> when the toplevel operator in argument is _not_ an assignment.
> This will probably force calls to various side-effect free
> functions which are skipped now, but as long as we are not
> _sure_ that a function is side-effect free we should do the
> call.
I think this "side-effect free elimination" should be a different
optimization pass.
Anyway, as my patch shows, the only case happening in our
src/algebra code base is the previous case
(argument is not used); the other cases (argument is used multiple
times) doesn't happen. So I simply gives out an error in such cases.
> --
> Waldek Hebisch
>
--
You received this message because you are subscribed to the Google Groups
"FriCAS - computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/fricas-devel.
For more options, visit https://groups.google.com/d/optout.
