bug with if-else #3

kirill-terekhov · 2014-02-27T11:34:23Z

Test cases for testx86.cpp:

the following will work once you replace in X86X64Context::switchState

// Ignore if both states are equal.
if (cur == src)
with
if (cur == src || src == NULL)
I know this is the bad way, but it works.

struct X86Test_IfElseInt : public X86Test {
X86Test_IfElseInt() : X86Test("[IfElse] If-else statement") {}

static void add(PodVector<X86Test*>& tests) {
    tests.append(new X86Test_IfElseInt());
}

virtual void compile(Compiler& c) {
    c.addFunc(kFuncConvHost, FuncBuilder2<int,int,int>());


    GpVar v1(c,kVarTypeInt32);
    GpVar v2(c,kVarTypeInt32);
    Label l1(c), l2(c);

    c.setArg(0, v1);
    c.setArg(1, v2);

    c.cmp(v1, v2);
    c.jg(l1);
    c.mov(v1, imm(1));
    c.jmp(l2);
    c.bind(l1);
    c.mov(v1, imm(2));
    c.bind(l2);
    c.ret(v1);
    c.endFunc();
}

virtual bool run(void* _func, StringBuilder& result, StringBuilder& expect) 
{
    typedef int(*Func)(int,int);
    Func func = asmjit_cast<Func>(_func);
    return (func(5,0) == 2) && (func(0,5) == 1);
}

};

the following code will fail on analyze().
Commenting out removeUnreachableCode() in compile() helps a little, but then it fails anyway.

struct X86Test_IfElseInt2 : public X86Test {
X86Test_IfElseInt2() : X86Test("[IfElse] If-else statement 2") {}

static void add(PodVector<X86Test*>& tests) {
    tests.append(new X86Test_IfElseInt2());
}

virtual void compile(Compiler& c) {
    c.addFunc(kFuncConvHost, FuncBuilder2<int, int, int>());


    GpVar v1(c, kVarTypeInt32);
    GpVar v2(c, kVarTypeInt32);
    Label l1(c), l2(c), l3(c), l4(c);

    c.setArg(0, v1);
    c.setArg(1, v2);

    c.jmp(l1);
    c.bind(l2);
    c.jmp(l4);
    c.bind(l1);


    c.cmp(v1, v2);
    c.jg(l1);
    c.mov(v1, imm(1));
    c.jmp(l2);
    c.bind(l3);
    c.mov(v1, imm(2));
    c.jmp(l2);

    c.bind(l4);

    c.ret(v1);
    c.endFunc();
}

virtual bool run(void* _func, StringBuilder& result, StringBuilder& expect)
{
    typedef int(*Func)(int, int);
    Func func = asmjit_cast<Func>(_func);
    return (func(2, 1) == 2) && (func(1, 2) == 1);
}

};

There are some additional problems when comparing and working with floating-point values with Xmm variables. I have to do a redundant mov operation inside each if-else brench, otherwise it fails on assertion:
Assertion failed: vd->getState() == kVarStateReg, file x86context_p.h, line 254
probably because I modified code as in example 1.
I can't provide example for this compatible with testx86.cpp, because for x86 I don't know how to make movs between GpVar and XmmVar.

The text was updated successfully, but these errors were encountered:

kobalicek · 2014-02-28T23:45:29Z

Thank you for the report!

I will take a look tomorrow. But I'm surethat modifying the condition to "if (cur == src || src == NULL)" will not fix the problem, only this simplified integration test-case. It will be most probably related to x86context.cpp line 4680.

kirill-terekhov · 2014-03-01T10:08:18Z

Thank you!
it never enters empty scope after x86context.cpp:4680.

In the second example jg should go to l3, then it passes the test with "if (cur == src || src == NULL)"

kirill-terekhov · 2014-03-01T10:31:15Z

Here is the test for floating-point numbers.
It passes only on x64, it always fails on x86.
I left some commented code so that you may try different combinations.

The test fails on x64 if you try to call external function (print) or accept arguments from GpVars.

template<typename FP_t>
struct X86Test_IfElseFP : public X86Test {
    X86Test_IfElseFP() : X86Test("[FPIfElse] If-else statement for floating-point values") {}

    static void add(PodVector<X86Test*>& tests) {
        tests.append(new X86Test_IfElseFP());
    }
    static FP_t print(FP_t val)
    {
        printf("value = %g\n", val);
        return val;
    }
    template<typename T>
    void jitprint(Compiler & c, T & var)
    {
        GpVar address(c);
        c.mov(address, imm_ptr(print));
        X86X64CallNode * ctx = c.call(address, kFuncConvHost, FuncBuilder1<FP_t, FP_t>());
        ctx->setArg(0,var);
        ctx->setRet(0,var);
    }
    virtual void compile(Compiler& c) {
        c.addFunc(kFuncConvHost, FuncBuilder2<int, FP_t, FP_t>());

        GpVar ret(c,kVarTypeInt32);
        XmmVar x1(c);
        XmmVar x2(c);
        /*
        GpVar v1(c, sizeof(FP_t) == 4 ? kVarTypeInt32 : kVarTypeInt64);
        GpVar v2(c, sizeof(FP_t) == 4 ? kVarTypeInt32 : kVarTypeInt64);
        c.setArg(0, v1);
        c.setArg(1, v2);

        jitprint(c, v1);
        jitprint(c, v2);

#if defined(ASMJIT_HOST_X64)
        c.movq(x1, v1);
        c.movq(x2, v2);
#else
        c.movd(x1, v1);
        c.movd(x2, v2);
#endif
        */

        c.setArg(0, x1);
        c.setArg(1, x2);
        /*
        GpVar v1(c, sizeof(FP_t) == 4 ? kVarTypeInt32 : kVarTypeInt64);
        GpVar v2(c, sizeof(FP_t) == 4 ? kVarTypeInt32 : kVarTypeInt64);
#if defined(ASMJIT_HOST_X64)
        c.movq(v1, x1);
        c.movq(v2, x2);
#else
        c.movd(v1, x1);
        c.movd(v2, x2);
#endif
        jitprint(c, x1);
        jitprint(c, x2);
        */

        Label l1(c), l2(c);
        if (sizeof(FP_t) == 4) c.comiss(x1, x2); else c.comisd(x1, x2);
        c.ja(l1);
        c.mov(ret, imm(1));
        c.jmp(l2);
        c.bind(l1);
        c.mov(ret, imm(2));
        c.bind(l2);
        c.ret(ret);
        c.endFunc();
    }

    virtual bool run(void* _func, StringBuilder& result, StringBuilder& expect)
    {
        typedef int(*Func)(FP_t, FP_t);
        Func func = asmjit_cast<Func>(_func);
        return (func(5.0, 2.0) == 2) && (func(1.0, 3.0) == 1);
    }
};

Since this fail is related to function calling but not conditional branching here is a more simple example:

template<typename FP_t>
struct X86Test_MulFP : public X86Test {
    X86Test_MulFP() : X86Test("[FPMUL] mul test for floating-point values") {}

    static void add(PodVector<X86Test*>& tests) {
        tests.append(new X86Test_MulFP());
    }
    static FP_t print(FP_t val)
    {
        printf("value = %g\n", val);
        return val;
    }
    template<typename T>
    void jitprint(Compiler & c, T & var)
    {
        GpVar address(c.newGpVar());
        c.mov(address, imm_ptr(print));
        X86X64CallNode * ctx = c.call(address, kFuncConvHost, FuncBuilder1<FP_t, FP_t>());
        ctx->setArg(0, var);
        ctx->setRet(0, var);
    }
    virtual void compile(Compiler& c) {
        c.addFunc(kFuncConvHost, FuncBuilder2<FP_t, FP_t, FP_t>());

        XmmVar x1(c.newXmmVar());
        XmmVar x2(c.newXmmVar());
        c.setArg(0, x1);
        c.setArg(1, x2);
        jitprint(c, x1);
        jitprint(c, x2);
        if (sizeof(FP_t) == 4) c.mulss(x1, x2); else c.mulsd(x1, x2);
        c.ret(x1);
        c.endFunc();
    }

    virtual bool run(void* _func, StringBuilder& result, StringBuilder& expect)
    {
        typedef FP_t(*Func)(FP_t, FP_t);
        Func func = asmjit_cast<Func>(_func);
        return (func(5.0, 2.0) == 10.0) && (func(1.0, 3.0) == 3.0);
    }
};

kobalicek · 2014-03-01T13:32:23Z

Ok I located/fixed the first problem and can reproduce also the removeUnreachableCode() problem. Will work on fixing these today.

kobalicek · 2014-03-01T15:56:38Z

Ok so I can now run the second code, but I think there is a small bug in the test-case, because it will run forever, see:

c.bind(l1);
c.cmp(v1, v2);
c.jg(l1);

So I will fix a bit and commit these test cases and fixes.

Added more test cases based on Issue #3 Minor changes.

kobalicek · 2014-03-01T16:13:23Z

5fe81c4

kirill-terekhov · 2014-03-02T09:47:37Z

Thank you!

With new version I came across with two more bugs of conditional branching.
The following two tests fail on different places, first one on x86context_p.h:278, second one on x86context.cpp:1151.

struct X86Test_AllocIfLoopElse : public X86Test {
  X86Test_AllocIfLoopElse() : X86Test("[Alloc] If-Else #1") {}

  static void add(PodVector<X86Test*>& tests) {
    tests.append(new X86Test_AllocIfLoopElse());
  }

  virtual void compile(Compiler& c) {
    c.addFunc(kFuncConvHost, FuncBuilder2<int,int,int>());

    GpVar v1(c, kVarTypeInt32);
    GpVar v2(c, kVarTypeInt32);
    GpVar counter(c, kVarTypeInt32);

    Label brench(c);
    Label exit(c);

    c.setArg(0, v1);
    c.setArg(1, v2);

    c.cmp(v1, v2);
    c.jg(brench);

    c.mov(counter, 0);
    Label loop1(c);
    c.bind(loop1);
    c.mov(v1, counter);
    c.inc(counter);
    c.cmp(counter, 1);
    c.jle(loop1);

    c.jmp(exit);

    c.bind(brench);

    c.mov(v1, 2);

    c.bind(exit);
    c.ret(v1);
    c.endFunc();
  }

  virtual bool run(void* _func, StringBuilder& result, StringBuilder& expect)  {
    typedef int (*Func)(int, int);
    Func func = asmjit_cast<Func>(_func);

    int a = func(1, 0);
    int b = func(0, 1);

    result.appendFormat("ret={%d, %d}", a, b);
    result.appendFormat("ret={%d, %d}", 2, 1);

    return a == 2 && b == 1;
  }
};

struct X86Test_AllocIfLoopElseLoop : public X86Test {
    X86Test_AllocIfLoopElseLoop() : X86Test("[Alloc] If-Else #1") {}

    static void add(PodVector<X86Test*>& tests) {
        tests.append(new X86Test_AllocIfLoopElseLoop());
    }

    virtual void compile(Compiler& c) {
        c.addFunc(kFuncConvHost, FuncBuilder2<int, int, int>());

        GpVar v1(c, kVarTypeInt32);
        GpVar v2(c, kVarTypeInt32);
        GpVar counter(c, kVarTypeInt32);
        c.mov(counter, 0);
        Label brench(c);
        Label exit(c);

        c.setArg(0, v1);
        c.setArg(1, v2);

        c.cmp(v1, v2);
        c.jg(brench);

        Label loop1(c);
        c.bind(loop1);
        c.mov(v1, counter);
        c.inc(counter);
        c.cmp(counter, 1);
        c.jle(loop1);

        c.jmp(exit);

        c.bind(brench);

        Label loop2(c);
        c.bind(loop2);
        c.mov(v1, counter);
        c.inc(counter);
        c.cmp(counter, 2);
        c.jle(loop2);

        c.bind(exit);
        c.ret(v1);
        c.endFunc();
    }

    virtual bool run(void* _func, StringBuilder& result, StringBuilder& expect)  {
        typedef int(*Func)(int, int);
        Func func = asmjit_cast<Func>(_func);

        int a = func(1, 0);
        int b = func(0, 1);

        result.appendFormat("ret={%d, %d}", a, b);
        result.appendFormat("ret={%d, %d}", 2, 1);

        return a == 2 && b == 1;
    }
};

kobalicek · 2014-03-02T13:53:29Z

I think I can fix these today, can reproduce.

kobalicek · 2014-03-02T14:37:06Z

17c690c

kobalicek · 2014-03-02T14:38:19Z

Please check it out if the referred commit fixes the issues with register-allocator. The problem with floating point is not connected with these issues and I will fix it by another commit.

kirill-terekhov · 2014-03-03T07:20:08Z

Yes it works! Thank you!

There seems to be the general problem with an external function call that you don't know which registers the function is going to use. So you either have to backup and restore all the used registers or analyze an external bytecode to detect register usage.

There should be the same convention on external function calling for static compiler, because it faces the same problem when calling jitted code. Most probably it reuses registers if it can predict the usage inside external function (so called inlining) but for totally unknown functions it has no way but backup registers.

The following example should illustrate the problem, please see comments inside:

struct X86Test_CallDouble : public X86Test {

    double stack[2];
    int stacksize;

    X86Test_CallDouble() : X86Test("[Call] doubles with stack") , stacksize(0) {}


    static void add(PodVector<X86Test*>& tests) {
        tests.append(new X86Test_CallDouble());
    }

    static void push(double a, int mul, X86Test_CallDouble * self) {
        printf("push %g to %d\n", a*mul, self->stacksize); //multiplication uses xmm1 register and then copies to xmm0 (for function call?)
        self->stack[self->stacksize++] = a*mul;
    }
    static double pop(X86Test_CallDouble * self)
    {
        double ret = self->stack[--self->stacksize];
        printf("pop %g from %d\n", ret, self->stacksize);
        return ret;
    }

    virtual void compile(Compiler& c) {
        c.addFunc(kFuncConvHost, FuncBuilder2<double, double, double>());

        // Prepare.
        GpVar fnpush(c);
        GpVar fnpop(c);
        GpVar self(c, kVarTypeIntPtr);
        GpVar mul(c, kVarTypeInt32);
        GpVar zero(c);
        XmmVar a(c, kVarTypeXmmSd, "a");
        XmmVar b(c, kVarTypeXmmSd, "b");
        XmmVar ret(c, kVarTypeXmmSd, "ret");

        c.mov(fnpush, imm_ptr((void*)push));
        c.mov(fnpop, imm_ptr((void*)pop));
        c.mov(self, imm_ptr(this));

        c.mov(zero, 0);
        c.movd(ret, zero); // initialize return with zero
        c.mov(mul, 2); //multiply by two inside functions

        c.setArg(0,a);
        c.setArg(1,b);


        c.nop();
        c.spill(self);
        c.nop();
        c.spill(mul);
        c.nop();
        c.spill(b); // want to move b out of register, but this do nothing
        c.nop();

        X86X64CallNode* call = c.call(fnpush, kFuncConvHost,
            FuncBuilder3<void, double, int, X86Test_CallDouble *>());

        call->setArg(0, a);
        call->setArg(1, mul);
        call->setArg(2, self);

        //in push() multiplication was written to xmm1 register that was priviously occupied by b
        //and copied to xmm0, so both a and b contain 5

        c.mov(self, imm_ptr(this));  //this two were destroyed inside function
        c.mov(mul, 2);               //reload them to prevent sigfault

        //this will print 10 instead of 7, as soons as we got 5 in b after privious call
        call = c.call(fnpush, kFuncConvHost,
            FuncBuilder3<void, double, int, X86Test_CallDouble *>());
        call->setArg(0, b);
        call->setArg(1, mul);
        call->setArg(2, self);

        c.mov(self, imm_ptr(this)); //prevent sigfault

        call = c.call(fnpop, kFuncConvHost,
            FuncBuilder1<double, X86Test_CallDouble *>());
        call->setArg(0, self);
        call->setRet(0, a);

        c.addsd(ret, a);

        c.mov(self, imm_ptr(this));  //prevent sigfault

        call = c.call(fnpop, kFuncConvHost,
            FuncBuilder1<double, X86Test_CallDouble *>());
        call->setArg(0, self);
        call->setRet(0, a);

        c.addsd(ret, a);

        c.ret(ret);
        c.endFunc();
    }

    virtual bool run(void* _func, StringBuilder& result, StringBuilder& expect) {
        typedef double(*Func)(double, double);
        Func func = asmjit_cast<Func>(_func);

        double resultRet = func(2.5, 3.5);
        double expectRet = 2.5*2 + 3.5*2;

        result.setFormat("ret=%g", resultRet);
        expect.setFormat("ret=%g", expectRet);

        return resultRet == expectRet;
    }
};

kirill-terekhov · 2014-03-03T07:25:00Z

Please see the calling convention on page 10 here:
http://www.agner.org/optimize/calling_conventions.pdf

On windows xmm6-xmm15 are garanteed to be the same after exit from function, so those may be used to backup variables. Also, it seems that generated function should follow the convention. On unix there is no such garantee, so you should use memory.

kobalicek · 2014-03-03T08:10:08Z

Well, I'm still working on the issue, and it is basically caused by the fact that x86 returns float/double in st0, thus there has to be an extra check for this case. Compiler function-call automatically saves registers that are clobbered. Furthermore I did some other improvements and I'm still fixing the x86 compiler.

kobalicek · 2014-04-03T17:48:19Z

The status of this issue: Currently only unsafe returns are st(0) and st(1), which are only used on x86 (not on x64). I will fix these soon. Also unsafe is conversion of return value / function argument - was really thinking about removing this feature, but I think that it's necessary.

kobalicek · 2015-02-09T02:28:55Z

5f04cbb

kobalicek · 2015-02-09T02:33:18Z

Floating point values returning in st0/st1 are now supported.

kobalicek added a commit that referenced this issue Mar 1, 2014

Fixed some bugs described in Issue #3

5fe81c4

Added more test cases based on Issue #3 Minor changes.

kobalicek added the bug label Apr 1, 2014

kobalicek mentioned this issue Sep 21, 2014

How to write portable code? #51

Closed

kobalicek closed this as completed Feb 9, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug with if-else #3

bug with if-else #3

kirill-terekhov commented Feb 27, 2014

kobalicek commented Feb 28, 2014

kirill-terekhov commented Mar 1, 2014

kirill-terekhov commented Mar 1, 2014

kobalicek commented Mar 1, 2014

kobalicek commented Mar 1, 2014

kobalicek commented Mar 1, 2014

kirill-terekhov commented Mar 2, 2014

kobalicek commented Mar 2, 2014

kobalicek commented Mar 2, 2014

kobalicek commented Mar 2, 2014

kirill-terekhov commented Mar 3, 2014

kirill-terekhov commented Mar 3, 2014

kobalicek commented Mar 3, 2014

kobalicek commented Apr 3, 2014

kobalicek commented Feb 9, 2015

kobalicek commented Feb 9, 2015

bug with if-else #3

bug with if-else #3

Comments

kirill-terekhov commented Feb 27, 2014

kobalicek commented Feb 28, 2014

kirill-terekhov commented Mar 1, 2014

kirill-terekhov commented Mar 1, 2014

kobalicek commented Mar 1, 2014

kobalicek commented Mar 1, 2014

kobalicek commented Mar 1, 2014

kirill-terekhov commented Mar 2, 2014

kobalicek commented Mar 2, 2014

kobalicek commented Mar 2, 2014

kobalicek commented Mar 2, 2014

kirill-terekhov commented Mar 3, 2014

kirill-terekhov commented Mar 3, 2014

kobalicek commented Mar 3, 2014

kobalicek commented Apr 3, 2014

kobalicek commented Feb 9, 2015

kobalicek commented Feb 9, 2015