Thanks Jim for looking into that, I was really starting to wonder what was going on. Let me know if you find a way to tell the compiler to stop complaining. Jean-Marc Jim Crichton a ?crit :> I looked back at my old C55 EC build, and I had the same warning in > mdf.c which Mike found. The assembly code did have a valid shift, and > this build did cancel echo. > > When I built with the SVN head, I saw the errors in kiss_fft.c also. > The assembly there also has valid shifts. > > So, I suspect that these warnings do not indicate a real problem. It > might be interesting to break down the offending lines into a sequence > of simpler instructions, to see if the compiler can be placated in that > way. > > - Jim > > ----- Original Message ----- From: "Jean-Marc Valin" > <jean-marc.valin@usherbrooke.ca> > To: "Jim Crichton" <jim.crichton@comcast.net> > Cc: "Michael Jacobson" <Michael.Jacobson@ultratec.com>; > <speex-dev@xiph.org> > Sent: Tuesday, January 22, 2008 4:00 PM > Subject: Re: [Speex-dev] Shift count warning messages > > >> Jim Crichton a ?crit : >>> I played briefly with the echo canceller on the C5509A back in May >>> 2006. I got the same compiler warnings, and sent a message to the >>> list which included this: >>> >>> "I got several compiler warnings for "shift out of range" in mdf.c, >>> which I fixed by adding EXTEND32 to all of the SHL32s with 16 bit >>> operands (st->frame_size in 6 places, st->wtmp2 in 1 place)." >> >> Yes, I remember those and I'm pretty sure they're fixed now. I think the >> problem comes from other changes I made later on, but I can't understand >> what's wrong. >> >>> I sent a patch with some fixes later that month, but I have not built >>> the echo canceller since then (no room for it). It is very likely >>> that you are seeing the same issue, and the fix should be the same >>> also. The trickier problems are the ones that the compiler does not >>> find. EXTEND32 is usually the cure there also. >>> >>> Now that I look at mdf.c, though, W is declares as spx_word32_t, so >>> there should not be a problem, unless spx_word32_t is being mapped to >>> 16-bits. That should not happen if you compile with CONFIG_TI_C54X >>> defined. This is odd. >> >> In both cases, what I'm seeing is that the code does a shift right by >> more than 16 bits. Is it possible that the C5x doesn't want to do that, >> even for 32-bit operands? It's either that or there's a build problem >> that causes the operands to be 16 bit instead of 32. This is indeed very >> odd. Can you investigate a bit and check what the types are on both >> sides of the shift? >> >> Cheers, >> >> Jean-Marc >> >> >> >> >>> - Jim >>> >>> >>> ----- Original Message ----- From: Michael Jacobson To: Jean-Marc >>> Valin Cc: speex-dev@xiph.org Sent: Tuesday, January 22, 2008 9:54 AM >>> Subject: Re: [Speex-dev] Shift count warning messages >>> >>> >>> yes, our DSP is 16 bit based, and if those bits of code aren't using >>> 32 bit variables then yes that is what the warnings would be. Does >>> this mean that the preprocessor and echo canceller won't work as well >>> because of this? >>> >>> -Mike >>> >>>>>> Jean-Marc Valin <jean-marc.valin@usherbrooke.ca> 01/18/08 8:42 >>>>>> PM >>> >>> >>> I think these warnings are due to your DSP being 16-bit and that bit >>> of code not being 16-bit safe (unlike the encoder and decoder). Can >>> you confirm? >>> >>> Jean-Marc >>> >>> Michael Jacobson a ?crit : >>>> Hi, I'm using 1.2beta3 on a 5416 DSP >>>> >>>> I have been getting warning messages that say: "kiss_fft.c", line >>>> 142: warning: shift count is too large >>>> >>>> >>>> I've noticed this on the echo canceller and the preprocessor. all >>>> pretty much related to these two lines of code: >>>> >>>> kiss_fft C_MUL4(scratch[0],Fout[m] , *tw1 ); >>>> >>>> mdf.c st->wtmp2[i] >>>> EXTRACT16(PSHR32(st->W[j*N+i],NORMALIZE_SCALEDOWN+16)); >>>> >>>> Is there some kind of switch I was supposed to throw to not get >>>> these messages? if not-are they hurting anything? >>>> >>>> -Mike >>>> >>>> >>>> >>>> ------------------------------------------------------------------------ >>>> >>>> >>>> >>>> _______________________________________________ Speex-dev mailing >>>> list Speex-dev@xiph.org >>>> http://lists.xiph.org/mailman/listinfo/speex-dev >>> >>> >>> >>> __________ Information from ESET NOD32 Antivirus, version of virus >>> signature database 2815 (20080122) __________ >>> >>> The message was checked by ESET NOD32 Antivirus. >>> >>> http://www.eset.com >>> >>> >>> >>> ------------------------------------------------------------------------------ >>> >>> >>> >>> >>> _______________________________________________ Speex-dev mailing >>> list Speex-dev@xiph.org >>> http://lists.xiph.org/mailman/listinfo/speex-dev >>> >>> >>> >>> __________ Information from ESET NOD32 Antivirus, version of virus >>> signature database 2815 (20080122) __________ >>> >>> The message was checked by ESET NOD32 Antivirus. >>> >>> http://www.eset.com >>> >>> >>> >>> ------------------------------------------------------------------------ >>> >>> >>> _______________________________________________ Speex-dev mailing >>> list Speex-dev@xiph.org >>> http://lists.xiph.org/mailman/listinfo/speex-dev >> >> __________ Information from ESET NOD32 Antivirus, version of virus >> signature database 2816 (20080123) __________ >> >> The message was checked by ESET NOD32 Antivirus. >> >> http://www.eset.com >> >> > > _______________________________________________ > Speex-dev mailing list > Speex-dev@xiph.org > http://lists.xiph.org/mailman/listinfo/speex-dev > >
Jean-Marc, I dug into this further, and found that the warning occurred when PSHR32 had a shift greater than 15. in fixed_generic.h, PSHR32 is defined as: #define PSHR32(a,shift) (SHR32((a)+((1<<((shift))>>1)),shift)) For 16-bit compilers the "1" needs a cast: #define PSHR32(a,shift) (SHR32((a)+((EXTEND32(1)<<((shift))>>1)),shift)) This change fixed the warning, and it did change the assembled code. With the original file, the ((1<<((shift))>>1)) was optimized away. Note that fixed_debug.h has the same problem. - Jim ----- Original Message ----- From: "Jean-Marc Valin" <jean-marc.valin@usherbrooke.ca> To: "Jim Crichton" <jim.crichton@comcast.net> Cc: <speex-dev@xiph.org>; "Michael Jacobson" <Michael.Jacobson@ultratec.com> Sent: Wednesday, January 23, 2008 12:16 PM Subject: Re: [Speex-dev] Shift count warning messages> Thanks Jim for looking into that, I was really starting to wonder what > was going on. Let me know if you find a way to tell the compiler to stop > complaining. > > Jean-Marc > > Jim Crichton a ?crit : >> I looked back at my old C55 EC build, and I had the same warning in >> mdf.c which Mike found. The assembly code did have a valid shift, and >> this build did cancel echo. >> >> When I built with the SVN head, I saw the errors in kiss_fft.c also. >> The assembly there also has valid shifts. >> >> So, I suspect that these warnings do not indicate a real problem. It >> might be interesting to break down the offending lines into a sequence >> of simpler instructions, to see if the compiler can be placated in that >> way. >> >> - Jim >> >> ----- Original Message ----- From: "Jean-Marc Valin" >> <jean-marc.valin@usherbrooke.ca> >> To: "Jim Crichton" <jim.crichton@comcast.net> >> Cc: "Michael Jacobson" <Michael.Jacobson@ultratec.com>; >> <speex-dev@xiph.org> >> Sent: Tuesday, January 22, 2008 4:00 PM >> Subject: Re: [Speex-dev] Shift count warning messages >> >> >>> Jim Crichton a ?crit : >>>> I played briefly with the echo canceller on the C5509A back in May >>>> 2006. I got the same compiler warnings, and sent a message to the >>>> list which included this: >>>> >>>> "I got several compiler warnings for "shift out of range" in mdf.c, >>>> which I fixed by adding EXTEND32 to all of the SHL32s with 16 bit >>>> operands (st->frame_size in 6 places, st->wtmp2 in 1 place)." >>> >>> Yes, I remember those and I'm pretty sure they're fixed now. I think the >>> problem comes from other changes I made later on, but I can't understand >>> what's wrong. >>> >>>> I sent a patch with some fixes later that month, but I have not built >>>> the echo canceller since then (no room for it). It is very likely >>>> that you are seeing the same issue, and the fix should be the same >>>> also. The trickier problems are the ones that the compiler does not >>>> find. EXTEND32 is usually the cure there also. >>>> >>>> Now that I look at mdf.c, though, W is declares as spx_word32_t, so >>>> there should not be a problem, unless spx_word32_t is being mapped to >>>> 16-bits. That should not happen if you compile with CONFIG_TI_C54X >>>> defined. This is odd. >>> >>> In both cases, what I'm seeing is that the code does a shift right by >>> more than 16 bits. Is it possible that the C5x doesn't want to do that, >>> even for 32-bit operands? It's either that or there's a build problem >>> that causes the operands to be 16 bit instead of 32. This is indeed very >>> odd. Can you investigate a bit and check what the types are on both >>> sides of the shift? >>> >>> Cheers, >>> >>> Jean-Marc >>> >>> >>> >>> >>>> - Jim >>>> >>>> >>>> ----- Original Message ----- From: Michael Jacobson To: Jean-Marc >>>> Valin Cc: speex-dev@xiph.org Sent: Tuesday, January 22, 2008 9:54 AM >>>> Subject: Re: [Speex-dev] Shift count warning messages >>>> >>>> >>>> yes, our DSP is 16 bit based, and if those bits of code aren't using >>>> 32 bit variables then yes that is what the warnings would be. Does >>>> this mean that the preprocessor and echo canceller won't work as well >>>> because of this? >>>> >>>> -Mike >>>> >>>>>>> Jean-Marc Valin <jean-marc.valin@usherbrooke.ca> 01/18/08 8:42 >>>>>>> PM >>> >>>> >>>> I think these warnings are due to your DSP being 16-bit and that bit >>>> of code not being 16-bit safe (unlike the encoder and decoder). Can >>>> you confirm? >>>> >>>> Jean-Marc >>>> >>>> Michael Jacobson a ?crit : >>>>> Hi, I'm using 1.2beta3 on a 5416 DSP >>>>> >>>>> I have been getting warning messages that say: "kiss_fft.c", line >>>>> 142: warning: shift count is too large >>>>> >>>>> >>>>> I've noticed this on the echo canceller and the preprocessor. all >>>>> pretty much related to these two lines of code: >>>>> >>>>> kiss_fft C_MUL4(scratch[0],Fout[m] , *tw1 ); >>>>> >>>>> mdf.c st->wtmp2[i] >>>>> EXTRACT16(PSHR32(st->W[j*N+i],NORMALIZE_SCALEDOWN+16)); >>>>> >>>>> Is there some kind of switch I was supposed to throw to not get >>>>> these messages? if not-are they hurting anything? >>>>> >>>>> -Mike >>>>> >>>>> >>>>> >>>>> ------------------------------------------------------------------------ >>>>> >>>>> >>>>> >>>>> _______________________________________________ Speex-dev mailing >>>>> list Speex-dev@xiph.org >>>>> http://lists.xiph.org/mailman/listinfo/speex-dev >>>> >>>> >>>> >>>> __________ Information from ESET NOD32 Antivirus, version of virus >>>> signature database 2815 (20080122) __________ >>>> >>>> The message was checked by ESET NOD32 Antivirus. >>>> >>>> http://www.eset.com >>>> >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> >>>> >>>> >>>> >>>> _______________________________________________ Speex-dev mailing >>>> list Speex-dev@xiph.org >>>> http://lists.xiph.org/mailman/listinfo/speex-dev >>>> >>>> >>>> >>>> __________ Information from ESET NOD32 Antivirus, version of virus >>>> signature database 2815 (20080122) __________ >>>> >>>> The message was checked by ESET NOD32 Antivirus. >>>> >>>> http://www.eset.com >>>> >>>> >>>> >>>> ------------------------------------------------------------------------ >>>> >>>> >>>> _______________________________________________ Speex-dev mailing >>>> list Speex-dev@xiph.org >>>> http://lists.xiph.org/mailman/listinfo/speex-dev >>> >>> __________ Information from ESET NOD32 Antivirus, version of virus >>> signature database 2816 (20080123) __________ >>> >>> The message was checked by ESET NOD32 Antivirus. >>> >>> http://www.eset.com >>> >>> >> >> _______________________________________________ >> Speex-dev mailing list >> Speex-dev@xiph.org >> http://lists.xiph.org/mailman/listinfo/speex-dev >> >> > > __________ Information from ESET NOD32 Antivirus, version of virus > signature database 2817 (20080123) __________ > > The message was checked by ESET NOD32 Antivirus. > > http://www.eset.com > >
Hi Jim, Thanks a lot for investigating. It definitely makes sense now. I'll fix the problem now. Is there any other place where you see that same (or similar) problem? Jean-Marc Jim Crichton a ?crit :> Jean-Marc, > > I dug into this further, and found that the warning occurred when PSHR32 > had a shift greater than 15. > > in fixed_generic.h, PSHR32 is defined as: > > #define PSHR32(a,shift) (SHR32((a)+((1<<((shift))>>1)),shift)) > > For 16-bit compilers the "1" needs a cast: > > #define PSHR32(a,shift) (SHR32((a)+((EXTEND32(1)<<((shift))>>1)),shift)) > > This change fixed the warning, and it did change the assembled code. > With the original file, the ((1<<((shift))>>1)) was optimized away. > Note that fixed_debug.h has the same problem. > > - Jim > > ----- Original Message ----- From: "Jean-Marc Valin" > <jean-marc.valin@usherbrooke.ca> > To: "Jim Crichton" <jim.crichton@comcast.net> > Cc: <speex-dev@xiph.org>; "Michael Jacobson" > <Michael.Jacobson@ultratec.com> > Sent: Wednesday, January 23, 2008 12:16 PM > Subject: Re: [Speex-dev] Shift count warning messages > > >> Thanks Jim for looking into that, I was really starting to wonder what >> was going on. Let me know if you find a way to tell the compiler to stop >> complaining. >> >> Jean-Marc >> >> Jim Crichton a ?crit : >>> I looked back at my old C55 EC build, and I had the same warning in >>> mdf.c which Mike found. The assembly code did have a valid shift, and >>> this build did cancel echo. >>> >>> When I built with the SVN head, I saw the errors in kiss_fft.c also. >>> The assembly there also has valid shifts. >>> >>> So, I suspect that these warnings do not indicate a real problem. It >>> might be interesting to break down the offending lines into a sequence >>> of simpler instructions, to see if the compiler can be placated in that >>> way. >>> >>> - Jim >>> >>> ----- Original Message ----- From: "Jean-Marc Valin" >>> <jean-marc.valin@usherbrooke.ca> >>> To: "Jim Crichton" <jim.crichton@comcast.net> >>> Cc: "Michael Jacobson" <Michael.Jacobson@ultratec.com>; >>> <speex-dev@xiph.org> >>> Sent: Tuesday, January 22, 2008 4:00 PM >>> Subject: Re: [Speex-dev] Shift count warning messages >>> >>> >>>> Jim Crichton a ?crit : >>>>> I played briefly with the echo canceller on the C5509A back in May >>>>> 2006. I got the same compiler warnings, and sent a message to the >>>>> list which included this: >>>>> >>>>> "I got several compiler warnings for "shift out of range" in mdf.c, >>>>> which I fixed by adding EXTEND32 to all of the SHL32s with 16 bit >>>>> operands (st->frame_size in 6 places, st->wtmp2 in 1 place)." >>>> >>>> Yes, I remember those and I'm pretty sure they're fixed now. I think >>>> the >>>> problem comes from other changes I made later on, but I can't >>>> understand >>>> what's wrong. >>>> >>>>> I sent a patch with some fixes later that month, but I have not built >>>>> the echo canceller since then (no room for it). It is very likely >>>>> that you are seeing the same issue, and the fix should be the same >>>>> also. The trickier problems are the ones that the compiler does not >>>>> find. EXTEND32 is usually the cure there also. >>>>> >>>>> Now that I look at mdf.c, though, W is declares as spx_word32_t, so >>>>> there should not be a problem, unless spx_word32_t is being mapped to >>>>> 16-bits. That should not happen if you compile with CONFIG_TI_C54X >>>>> defined. This is odd. >>>> >>>> In both cases, what I'm seeing is that the code does a shift right by >>>> more than 16 bits. Is it possible that the C5x doesn't want to do that, >>>> even for 32-bit operands? It's either that or there's a build problem >>>> that causes the operands to be 16 bit instead of 32. This is indeed >>>> very >>>> odd. Can you investigate a bit and check what the types are on both >>>> sides of the shift? >>>> >>>> Cheers, >>>> >>>> Jean-Marc >>>> >>>> >>>> >>>> >>>>> - Jim >>>>> >>>>> >>>>> ----- Original Message ----- From: Michael Jacobson To: Jean-Marc >>>>> Valin Cc: speex-dev@xiph.org Sent: Tuesday, January 22, 2008 9:54 AM >>>>> Subject: Re: [Speex-dev] Shift count warning messages >>>>> >>>>> >>>>> yes, our DSP is 16 bit based, and if those bits of code aren't using >>>>> 32 bit variables then yes that is what the warnings would be. Does >>>>> this mean that the preprocessor and echo canceller won't work as well >>>>> because of this? >>>>> >>>>> -Mike >>>>> >>>>>>>> Jean-Marc Valin <jean-marc.valin@usherbrooke.ca> 01/18/08 8:42 >>>>>>>> PM >>> >>>>> >>>>> I think these warnings are due to your DSP being 16-bit and that bit >>>>> of code not being 16-bit safe (unlike the encoder and decoder). Can >>>>> you confirm? >>>>> >>>>> Jean-Marc >>>>> >>>>> Michael Jacobson a ?crit : >>>>>> Hi, I'm using 1.2beta3 on a 5416 DSP >>>>>> >>>>>> I have been getting warning messages that say: "kiss_fft.c", line >>>>>> 142: warning: shift count is too large >>>>>> >>>>>> >>>>>> I've noticed this on the echo canceller and the preprocessor. all >>>>>> pretty much related to these two lines of code: >>>>>> >>>>>> kiss_fft C_MUL4(scratch[0],Fout[m] , *tw1 ); >>>>>> >>>>>> mdf.c st->wtmp2[i] >>>>>> EXTRACT16(PSHR32(st->W[j*N+i],NORMALIZE_SCALEDOWN+16)); >>>>>> >>>>>> Is there some kind of switch I was supposed to throw to not get >>>>>> these messages? if not-are they hurting anything? >>>>>> >>>>>> -Mike >>>>>> >>>>>> >>>>>> >>>>>> ------------------------------------------------------------------------ >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> _______________________________________________ Speex-dev mailing >>>>>> list Speex-dev@xiph.org >>>>>> http://lists.xiph.org/mailman/listinfo/speex-dev >>>>> >>>>> >>>>> >>>>> __________ Information from ESET NOD32 Antivirus, version of virus >>>>> signature database 2815 (20080122) __________ >>>>> >>>>> The message was checked by ESET NOD32 Antivirus. >>>>> >>>>> http://www.eset.com >>>>> >>>>> >>>>> >>>>> ------------------------------------------------------------------------------ >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> _______________________________________________ Speex-dev mailing >>>>> list Speex-dev@xiph.org >>>>> http://lists.xiph.org/mailman/listinfo/speex-dev >>>>> >>>>> >>>>> >>>>> __________ Information from ESET NOD32 Antivirus, version of virus >>>>> signature database 2815 (20080122) __________ >>>>> >>>>> The message was checked by ESET NOD32 Antivirus. >>>>> >>>>> http://www.eset.com >>>>> >>>>> >>>>> >>>>> ------------------------------------------------------------------------ >>>>> >>>>> >>>>> >>>>> _______________________________________________ Speex-dev mailing >>>>> list Speex-dev@xiph.org >>>>> http://lists.xiph.org/mailman/listinfo/speex-dev >>>> >>>> __________ Information from ESET NOD32 Antivirus, version of virus >>>> signature database 2816 (20080123) __________ >>>> >>>> The message was checked by ESET NOD32 Antivirus. >>>> >>>> http://www.eset.com >>>> >>>> >>> >>> _______________________________________________ >>> Speex-dev mailing list >>> Speex-dev@xiph.org >>> http://lists.xiph.org/mailman/listinfo/speex-dev >>> >>> >> >> __________ Information from ESET NOD32 Antivirus, version of virus >> signature database 2817 (20080123) __________ >> >> The message was checked by ESET NOD32 Antivirus. >> >> http://www.eset.com >> >> > > >