Hi Oleg,
Thanks for discovering the problem. Mathematically equivalent Faust code
should, as much as possible, result in the exact same generated code. It is
clearly not the case here. We will take a look…
Cheers,
Yann
Le dim. 8 oct. 2023 à 18:52, Oleg Nesterov <o...@redhat.com> a écrit :
> Hello,
>
> simplification() does
>
> ...
>
> else if (isSigBinOp(sig, &opnum, t1, t2)) {
> BinOp* op = gBinOpTable[opnum];
> Node n1 = t1->node();
> Node n2 = t2->node();
>
> ...
>
> else if (opnum == kSub && isZero(n1))
> return sigBinOp(kMul, sigInt(-1), t2);
>
> and this turns 0-x into -1*x .
>
> To be honest, I don't understand the point, but at least this
> transformation is
> correct. and perhaps C++ compiler will generates the same code with
> -ffast-math.
>
> However, this is inconsistent wrt normalizeAddTerm() which turns -1*x into
> 0-x !
>
> Example:
>
> process(x) = 0-x, -1*x;
>
> C++ code:
>
> for (int i0 = 0; i0 < count; i0 = i0 + 1) {
> float fTemp0 = float(input0[i0]);
> output0[i0] = FAUSTFLOAT(-1.0f * fTemp0); // 0 - x
> output1[i0] = FAUSTFLOAT(0.0f - fTemp0); // -1 * x
> }
>
> looks obviously suboptimal (even if, again, -ffast-math can help).
>
> If I remove the kSub && isZero(n1) "optimization" above I get
>
> for (int i0 = 0; i0 < count; i0 = i0 + 1) {
> float fTemp0 = 0.0f - float(input0[i0]);
> output0[i0] = FAUSTFLOAT(fTemp0);
> output1[i0] = FAUSTFLOAT(fTemp0);
> }
>
> which looks obviously better to me.
>
>
> -------------------------------------------------------------------------------
>
> And this is one of the reasons (there are more) why the reverted commit
> 428eee11896556707b78f9044d66d2397c9884fa which changed realPropagate()
>
> --- a/compiler/propagate/propagate.cpp
> +++ b/compiler/propagate/propagate.cpp
> @@ -537,7 +537,7 @@ static siglist realPropagate(Tree slotenv,
> Tree path, Tree box, const siglist& l
> Tree exp = outsigs[dst - 1];
> if ((src > 0) & (src <= ins)) {
> // we have a source
> - outsigs[dst - 1] = sigAdd(exp, lsig[src -
> 1]);
> + outsigs[dst - 1] = simplify(sigAdd(exp,
> lsig[src - 1]));
> }
> }
> }
>
> can make a difference.
>
> Example:
>
> process(x) = -x : route(1,1,1,1);
>
> diff:
>
> - output0[i0] = FAUSTFLOAT(-1.0f *
> float(input0[i0]));
> + output0[i0] = FAUSTFLOAT(0.0f - float(input0[i0]));
>
> But:
>
> process(x) = -x : route(1,1,1,1) : route(1,1,1,1);
>
> generates the same code again, with or without the commit above.
>
> And of course,
>
> process(x) = -x : route(1,1,1,1) : route(1,1,1,1) : route(1,1,1,1);
>
> leads to the same diff:
>
> - output0[i0] = FAUSTFLOAT(-1.0f *
> float(input0[i0]));
> + output0[i0] = FAUSTFLOAT(0.0f - float(input0[i0]));
>
> Oleg.
>
>
>
> _______________________________________________
> Faudiostream-devel mailing list
> Faudiostream-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/faudiostream-devel
>
_______________________________________________
Faudiostream-devel mailing list
Faudiostream-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/faudiostream-devel
