Re: optimization question

[Martin Uecker]

 mark maule <mark.ma...@oracle.com>:
> On 5/20/2015 3:27 AM, Richard Biener wrote:
> > On Mon, May 18, 2015 at 10:01 PM, mark maule <mark.ma...@oracle.com> wrote:

> > The usual issue with this kind of behavior is out-of-bound accesses of
> > arrays in a loop
> > or invoking undefined behavior when signed integer operations wrap.
> >
> >
> >     uint32_t                outLun[ BS_CFG_DRIVE_GROUPS ];
> >
> > and
> >
> >        while ( ( dgHandle < ( BS_CFG_DRIVE_GROUPS + 1 ) ) &&
> > ...
> >           dgDestageOut = bs_destageData.outLun[ dgHandle ];
> >
> > looks like this might access outLun[BS_CFG_DRIVE_GROUPS] which is
> > out-of-bounds.
> >
> > Richard.
> 
> You are correct, and when I change outLun[] to be size 
> BS_CFG_DRIVE_GROUPS+1, the generated asm looks like it will account for 
> dgHandle in the while() loop.  I will pass this back to our development 
> team to get a proper fix.
> 
> Now, the followon:  Something in the compiler/optimizer recognized this 
> out of bounds situation - should a warning have been emitted? Or are 
> there ambiguities which make a warning generation here inappropriate?

Yes, ideally a compiler should emit a warning. C compilers traditionally
were not very good at this, but it turns out very recent versions of GCC
can do this:

test.c:14:23: warning: iteration 10u invokes undefined behavior 
[-Waggressive-loop-optimizations]
          dgDestageOut = outLun[ dgHandle ];
                       ^
test.c:11:13: note: containing loop
       while ( ( dgHandle < ( BS_CFG_DRIVE_GROUPS + 1 ) ) )


For this simplified test case:

#include <stdint.h>

#define BS_CFG_DRIVE_GROUPS 10
uint32_t dgDestageLimit = 0;
uint32_t outLun[ BS_CFG_DRIVE_GROUPS ];

void test(void)
{
   int dgHandle = 0;

      while ( ( dgHandle < ( BS_CFG_DRIVE_GROUPS + 1 ) ) )
      {
         uint32_t dgDestageOut;
         dgDestageOut = outLun[ dgHandle ];
         dgHandle++;
      }
}


Martin