#1216 Bank selection fails for registers with multiple names

closed-fixed
nobody
5
2013-05-25
2006-10-18
Simon Bryden
No

SDCC : pic16/pic14 2.6.1 #4408 (Oct 15 2006) (UNIX)

Because variables with different names are stored only
once by the function newReg(), attempts later to find
the associated register during bank fixing fail for
other names. For example, given the following code:

#include <pic16f628.h>

int main()
{
RCSTA=0;
TRISA=0;
ADEN=1;
while (1);
}

The variable RCSTA is stored, but the bit ADEN, which
resolves to RCSTA_bits is not stored, because it
already exists under the name RCSTA. Thus the bank
selection fails (silently) for that assignment, and the
bank (which was set to 1 for the assignment to TRISA is
incorrect.)

A fix is to modify the function newReg as follows:

--- ralloc.c.orig 2006-10-18 11:50:06.000000000 +0200
+++ ralloc.c 2006-10-18 11:15:53.000000000 +0200
@@ -351,6 +351,7 @@
//printf( "%s: already present: %s\n",
__FUNCTION__, name );
return (dReg);
}
+#if 0
dReg = regWithIdx( dynDirectRegs, rIdx, 0 );
if (!dReg) dReg = regWithIdx( dynDirectRegs,
rIdx, 1 );
if (dReg)
@@ -358,7 +359,7 @@
//printf( "%s: already present %s
(idx:%d/%x)", __FUNCTION__, name, rIdx, rIdx );
return (dReg);
}
-
+#endif
dReg = Safe_calloc(1,sizeof(regs));
dReg->type = type;
dReg->pc_type = pc_type;

The #if 0 block removes the check and causes the
multiply-named register to be hashed multiple times.
I'm not sure whether this breaks anything else, or
whether this is an optimisation which should be handled
otherwise.

Simon
simon -AT- brydens.net

Discussion

  • Raphael Neider
    Raphael Neider
    2006-10-28

    Logged In: YES
    user_id=1115835

    Fixed in r4441 by creating proper aliases for registers with
    multiple names.

    > Thus the bank selection fails (silently) for that assignment
    Also fixed, DoBankselect now fails 'noisily' ;-)

    Thanks and regards,
    Raphael

     
  • Raphael Neider
    Raphael Neider
    2006-10-28

    • milestone: --> fixed
    • status: open --> closed-fixed
     
  • Simon Bryden
    Simon Bryden
    2006-11-08

    • status: closed-fixed --> open-fixed
     
  • Simon Bryden
    Simon Bryden
    2006-11-08

    Logged In: YES
    user_id=222462

    Hi Raphael,

    Thanks for fixing this - alas there is still a problem. Try
    this:

    #include <pic16f628.h>

    int main()
    {
    GIE=0;
    }

    [compile with sdcc -apic14 -p16f628]

    This fails with:
    sdcc: pcode.c:4761: DoBankSelect: Assertion `!"Could not get
    register from instruction."' failed.

    This happens whenever a bit from INTCON or STATUS is
    referenced via the _bits bitfield.

    I suspect that the root cause might be gleaned by looking at
    getRegFromInstruction:

    regs * getRegFromInstruction(pCode *pc)
    {
    regs *r;
    if(!pc ||
    !isPCI(pc) ||
    !PCI(pc)->pcop ||
    PCI(pc)->num_ops == 0 )
    return NULL;

    switch(PCI(pc)->pcop->type) {
    case PO_STATUS:
    case PO_FSR:
    case PO_INDF:
    case PO_INTCON:
    case PO_BIT:
    case PO_GPR_TEMP:
    case PO_SFR_REGISTER:
    case PO_PCL:
    case PO_PCLATH:
    return PCOR(PCI(pc)->pcop)->r;

    case PO_GPR_REGISTER:
    case PO_GPR_BIT:
    case PO_DIR:
    r = PCOR(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    case PO_LITERAL:
    break;

    case PO_IMMEDIATE:
    r = PCOI(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    default:
    break;
    }

    return NULL;

    }

    In the case of an INTCON bit, the case PO_GPR_BIT is hit,
    but PCOR(PCI(pc)->pcop)->r is zero. Since this only fails
    with the registers INTCON and STATUS as far as I can tell,
    and since they are explicitly mentioned above as a separate
    case...

    In any case, I hope this helps you to track down the
    problem. Otherwise I'm really pleased about the stability of
    sdcc for pic14. You guys have done a great job!
    Regards,
    Simon.

     
  • Simon Bryden
    Simon Bryden
    2006-11-08

    Logged In: YES
    user_id=222462

    Hi Raphael,

    Thanks for fixing this - alas there is still a problem. Try
    this:

    #include <pic16f628.h>

    int main()
    {
    GIE=0;
    }

    [compile with sdcc -apic14 -p16f628]

    This fails with:
    sdcc: pcode.c:4761: DoBankSelect: Assertion `!"Could not get
    register from instruction."' failed.

    This happens whenever a bit from INTCON or STATUS is
    referenced via the _bits bitfield.

    I suspect that the root cause might be gleaned by looking at
    getRegFromInstruction:

    regs * getRegFromInstruction(pCode *pc)
    {
    regs *r;
    if(!pc ||
    !isPCI(pc) ||
    !PCI(pc)->pcop ||
    PCI(pc)->num_ops == 0 )
    return NULL;

    switch(PCI(pc)->pcop->type) {
    case PO_STATUS:
    case PO_FSR:
    case PO_INDF:
    case PO_INTCON:
    case PO_BIT:
    case PO_GPR_TEMP:
    case PO_SFR_REGISTER:
    case PO_PCL:
    case PO_PCLATH:
    return PCOR(PCI(pc)->pcop)->r;

    case PO_GPR_REGISTER:
    case PO_GPR_BIT:
    case PO_DIR:
    r = PCOR(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    case PO_LITERAL:
    break;

    case PO_IMMEDIATE:
    r = PCOI(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    default:
    break;
    }

    return NULL;

    }

    In the case of an INTCON bit, the case PO_GPR_BIT is hit,
    but PCOR(PCI(pc)->pcop)->r is zero. Since this only fails
    with the registers INTCON and STATUS as far as I can tell,
    and since they are explicitly mentioned above as a separate
    case...

    In any case, I hope this helps you to track down the
    problem. Otherwise I'm really pleased about the stability of
    sdcc for pic14. You guys have done a great job!
    Regards,
    Simon.

     
  • Simon Bryden
    Simon Bryden
    2006-11-08

    Logged In: YES
    user_id=222462

    Hi Raphael,

    Thanks for fixing this - alas there is still a problem. Try
    this:

    #include <pic16f628.h>

    int main()
    {
    GIE=0;
    }

    [compile with sdcc -apic14 -p16f628]

    This fails with:
    sdcc: pcode.c:4761: DoBankSelect: Assertion `!"Could not get
    register from instruction."' failed.

    This happens whenever a bit from INTCON or STATUS is
    referenced via the _bits bitfield.

    I suspect that the root cause might be gleaned by looking at
    getRegFromInstruction:

    regs * getRegFromInstruction(pCode *pc)
    {
    regs *r;
    if(!pc ||
    !isPCI(pc) ||
    !PCI(pc)->pcop ||
    PCI(pc)->num_ops == 0 )
    return NULL;

    switch(PCI(pc)->pcop->type) {
    case PO_STATUS:
    case PO_FSR:
    case PO_INDF:
    case PO_INTCON:
    case PO_BIT:
    case PO_GPR_TEMP:
    case PO_SFR_REGISTER:
    case PO_PCL:
    case PO_PCLATH:
    return PCOR(PCI(pc)->pcop)->r;

    case PO_GPR_REGISTER:
    case PO_GPR_BIT:
    case PO_DIR:
    r = PCOR(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    case PO_LITERAL:
    break;

    case PO_IMMEDIATE:
    r = PCOI(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    default:
    break;
    }

    return NULL;

    }

    In the case of an INTCON bit, the case PO_GPR_BIT is hit,
    but PCOR(PCI(pc)->pcop)->r is zero. Since this only fails
    with the registers INTCON and STATUS as far as I can tell,
    and since they are explicitly mentioned above as a separate
    case...

    In any case, I hope this helps you to track down the
    problem. Otherwise I'm really pleased about the stability of
    sdcc for pic14. You guys have done a great job!
    Regards,
    Simon.

     
  • Simon Bryden
    Simon Bryden
    2006-11-09

    Logged In: YES
    user_id=222462

    Hi Raphael,

    Thanks for fixing this - alas there is still a problem. Try
    this:

    #include <pic16f628.h>

    int main()
    {
    GIE=0;
    }

    [compile with sdcc -apic14 -p16f628]

    This fails with:
    sdcc: pcode.c:4761: DoBankSelect: Assertion `!"Could not get
    register from instruction."' failed.

    This happens whenever a bit from INTCON or STATUS is
    referenced via the _bits bitfield.

    I suspect that the root cause might be gleaned by looking at
    getRegFromInstruction:

    regs * getRegFromInstruction(pCode *pc)
    {
    regs *r;
    if(!pc ||
    !isPCI(pc) ||
    !PCI(pc)->pcop ||
    PCI(pc)->num_ops == 0 )
    return NULL;

    switch(PCI(pc)->pcop->type) {
    case PO_STATUS:
    case PO_FSR:
    case PO_INDF:
    case PO_INTCON:
    case PO_BIT:
    case PO_GPR_TEMP:
    case PO_SFR_REGISTER:
    case PO_PCL:
    case PO_PCLATH:
    return PCOR(PCI(pc)->pcop)->r;

    case PO_GPR_REGISTER:
    case PO_GPR_BIT:
    case PO_DIR:
    r = PCOR(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    case PO_LITERAL:
    break;

    case PO_IMMEDIATE:
    r = PCOI(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    default:
    break;
    }

    return NULL;

    }

    In the case of an INTCON bit, the case PO_GPR_BIT is hit,
    but PCOR(PCI(pc)->pcop)->r is zero. Since this only fails
    with the registers INTCON and STATUS as far as I can tell,
    and since they are explicitly mentioned above as a separate
    case...

    In any case, I hope this helps you to track down the
    problem. Otherwise I'm really pleased about the stability of
    sdcc for pic14. You guys have done a great job!
    Regards,
    Simon.

     
  • Maarten Brock
    Maarten Brock
    2006-11-09

    Logged In: YES
    user_id=888171

    Why are you posting the same over and over again. I don't know about Raphael, but it would not make me very keen
    on helping you. Stop bugging us unless you have something usefull to say.

     
  • Simon Bryden
    Simon Bryden
    2006-11-09

    Logged In: YES
    user_id=222462

    Hi Raphael,

    Thanks for fixing this - alas there is still a problem. Try
    this:

    #include <pic16f628.h>

    int main()
    {
    GIE=0;
    }

    [compile with sdcc -apic14 -p16f628]

    This fails with:
    sdcc: pcode.c:4761: DoBankSelect: Assertion `!"Could not get
    register from instruction."' failed.

    This happens whenever a bit from INTCON or STATUS is
    referenced via the _bits bitfield.

    I suspect that the root cause might be gleaned by looking at
    getRegFromInstruction:

    regs * getRegFromInstruction(pCode *pc)
    {
    regs *r;
    if(!pc ||
    !isPCI(pc) ||
    !PCI(pc)->pcop ||
    PCI(pc)->num_ops == 0 )
    return NULL;

    switch(PCI(pc)->pcop->type) {
    case PO_STATUS:
    case PO_FSR:
    case PO_INDF:
    case PO_INTCON:
    case PO_BIT:
    case PO_GPR_TEMP:
    case PO_SFR_REGISTER:
    case PO_PCL:
    case PO_PCLATH:
    return PCOR(PCI(pc)->pcop)->r;

    case PO_GPR_REGISTER:
    case PO_GPR_BIT:
    case PO_DIR:
    r = PCOR(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    case PO_LITERAL:
    break;

    case PO_IMMEDIATE:
    r = PCOI(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    default:
    break;
    }

    return NULL;

    }

    In the case of an INTCON bit, the case PO_GPR_BIT is hit,
    but PCOR(PCI(pc)->pcop)->r is zero. Since this only fails
    with the registers INTCON and STATUS as far as I can tell,
    and since they are explicitly mentioned above as a separate
    case...

    In any case, I hope this helps you to track down the
    problem. Otherwise I'm really pleased about the stability of
    sdcc for pic14. You guys have done a great job!
    Regards,
    Simon.

     
  • Simon Bryden
    Simon Bryden
    2006-11-09

    Logged In: YES
    user_id=222462

    Hi Raphael,

    Thanks for fixing this - alas there is still a problem. Try
    this:

    #include <pic16f628.h>

    int main()
    {
    GIE=0;
    }

    [compile with sdcc -apic14 -p16f628]

    This fails with:
    sdcc: pcode.c:4761: DoBankSelect: Assertion `!"Could not get
    register from instruction."' failed.

    This happens whenever a bit from INTCON or STATUS is
    referenced via the _bits bitfield.

    I suspect that the root cause might be gleaned by looking at
    getRegFromInstruction:

    regs * getRegFromInstruction(pCode *pc)
    {
    regs *r;
    if(!pc ||
    !isPCI(pc) ||
    !PCI(pc)->pcop ||
    PCI(pc)->num_ops == 0 )
    return NULL;

    switch(PCI(pc)->pcop->type) {
    case PO_STATUS:
    case PO_FSR:
    case PO_INDF:
    case PO_INTCON:
    case PO_BIT:
    case PO_GPR_TEMP:
    case PO_SFR_REGISTER:
    case PO_PCL:
    case PO_PCLATH:
    return PCOR(PCI(pc)->pcop)->r;

    case PO_GPR_REGISTER:
    case PO_GPR_BIT:
    case PO_DIR:
    r = PCOR(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    case PO_LITERAL:
    break;

    case PO_IMMEDIATE:
    r = PCOI(PCI(pc)->pcop)->r;
    if (r)
    return r;
    return dirregWithName(PCI(pc)->pcop->name);

    default:
    break;
    }

    return NULL;

    }

    In the case of an INTCON bit, the case PO_GPR_BIT is hit,
    but PCOR(PCI(pc)->pcop)->r is zero. Since this only fails
    with the registers INTCON and STATUS as far as I can tell,
    and since they are explicitly mentioned above as a separate
    case...

    In any case, I hope this helps you to track down the
    problem. Otherwise I'm really pleased about the stability of
    sdcc for pic14. You guys have done a great job!
    Regards,
    Simon.

     
  • Raphael Neider
    Raphael Neider
    2006-11-11

    • status: open-fixed --> closed-fixed
     
  • Raphael Neider
    Raphael Neider
    2006-11-11

    Logged In: YES
    user_id=1115835

    Fixed (until proven not to be) in SDCC r4471.

    The cause were incorrectly not allocated registers for the
    entities surrounding the accessed bits (INTCON_bits or
    STATUS_bits in your example). Now I allocate registers for
    them instead of returning NULL.

    Regards,
    Raphael Neider

     
  • Simon Bryden
    Simon Bryden
    2006-11-13

    Logged In: YES
    user_id=222462

    Rafael,

    Thanks - will test asap. If you don't hear it's ok.

    Maarten - keep your hair on - it was a mistake - sorry I
    refreshed a few times the POST by mistake. Didn't mean to
    bug you.

    Simon.
    ---

     
    • status: closed-fixed --> open-fixed
     
  • Logged In: YES
    user_id=589052
    Originator: NO

    Hello Raphael,

    it seems it still can be triggered (SDCC #4472).

    The regression test in "sdcc/src/regression"
    (not the one in "sdcc/support/regression/")
    fails on and1.c:

    ~/sdcc/src/regression> make
    Test b.stc
    PASSED
    Test add.stc
    PASSED
    Test add2.stc
    FAILED
    Test add3.stc
    PASSED
    sdcc: ../../../sdcc/src/pic/pcode.c:4761: DoBankSelect: Assertion `!"Could not get register from instruction."' failed.
    Caught signal 6: SIGABRT
    make: *** [and1.cod] Fehler 1

    Greetings,
    Frieder

     
    • status: open-fixed --> closed-fixed
     
  • Logged In: YES
    user_id=589052
    Originator: NO

    Hello Raphael,

    thanks a lot for fixing this (SDCC #4478).

    And also for fixing the other failing tests in
    the pic14 regression tests!

    This is really good news!)

    Greetings,
    Frieder