Issue with faust2sndfile

[mryndzionek]

I've encountered a strange issue. With this dsp file:

import("stdfaust.lib");

process = (c : *(100) : os.osc)
    with {
        clk1 = ba.pulse(ma.SR * tempo / 1000);
        clk2 = ba.pulse(ma.SR * 2 * tempo / 1000);
        c1 = ba.counter(clk1) % 2;
        c2 = ba.counter(clk2) % 2;
        c = 1 + c1 + (2 * c2);
        tempo = 500;
    };

I should be getting/hearing 4 frequencies in a sequence and it all works when I run it in the online IDE. However when I use faust2sndfile locally (2.41.1, as well as master-dev) I'm only hearing 2 frequencies. It seems that for some reason c2 gets 'optimized out'. If I use c2 explicitly like:

process = (c : *(100) : os.osc), c2

the first channel has all 4 expected frequencies.

[mryndzionek]

Okay, some more background. Here are the differences in generated code: So precedence with modulo operator is not enforced. I've verified that changing c2 to:

c2 = (1.0 * ba.counter(clk2)) % 2;

makes the issue go away.

[sletz]

Very strange indeed: on master-dev it works with the LLVM and Interp backend, but not the C++ one. How do you generate the C++ code on the right? Which Faust version?

[mryndzionek]

This is the code generated by faust2sndfile. On the right is the code generated with c2 added to process. Adding c2 to process makes is 'representable' as separate variable (iTemp2). Problem occurs on 2.41.1 and master-dev.

[mryndzionek]

Actually I've found even simpler workaround: c = 1 + c1 + (2 * c2); -> c = 1 + c1 + (c2 * 2);. Still, this is a bug that needs fixing.

[sletz]

Sure. It's a operator priority/parenthesis issue.

[mryndzionek]

I've spent some time looking at the code. Not sure if this is a correct fix, but changing needParenthesis in cpp_instructions.hh to:

    virtual bool needParenthesis(BinopInst* inst, ValueInst* arg)
    {
        int p0 = gBinOpTable[inst->fOpcode]->fPriority;
        BinopInst* a = dynamic_cast<BinopInst*>(arg);
        int p1 = a ? gBinOpTable[a->fOpcode]->fPriority : INT_MAX;
        if (a) {
            return true;
        } else {
            return (isLogicalOpcode(inst->fOpcode) || (p0 > p1)) && !arg->isSimpleValue() &&
                   !dynamic_cast<CastInst*>(arg);
        }
    }

(forcing parenthesis if arg is a BinopInst) seems to have the desired effect.

[sletz]

Somewhere here yes, but not the proper fix. I'm on it.

[sletz]

Fixed in https://github.com/grame-cncm/faust/commit/b1dd487cd4d771f2277d96e9498ca252b5c63a2c