I can see nothing obviously wrong with your code. Because the function is inlined there could be a subtle bug near where the function is called. Can you reproduce the problem with a simple test driver?

FWIW, I implemented ffs64() like this.

static inline int ffs64( uint64_t word )
{
    if ( word == 0 ) return 0;
    int bit = 1;
    if ( ( word & 0xFFFFFFFF ) == 0 )
    {
        word >>= 32;
        bit += 32;
    }
    if ( ( word & 0xFFFF ) == 0 )
    {
        word >>= 16;
        bit += 16;
    }
    if ( ( word & 0xFF ) == 0 )
    {
        word >>= 8;
        bit += 8;
    }
    if ( ( word & 0xF ) == 0 )
    {
        word >>= 4;
        bit += 4;
    }
    if ( ( word & 0x3 ) == 0 )
    {
        word >>= 2;
        bit += 2;
    }
    if ( ( word & 0x1 ) == 0 )
    {
        bit += 1;
    }
    return bit;
}


On 22/07/2013 11:57 AM, Charles Mills wrote:
Here is exact cut and paste with zero editing, complete with a typo in the
second comment. The code is unit tested on MS Visual Studio -- hence the two
#ifdef's.

        // Find First Set
        static inline int Ffs64(unsigned long long valueToTest)
        {
                // returns index of first set bit. Uses UNIX convention
where bit 1 is LSB of word for compatibilit with z/OS ffs()

                static const unsigned long long lswMask =
0x00000000ffffffff;

                // note Windows provides a _BitScanForward64() but I did it
this way to make it more z/OS-like for test purposes
                // if we ever needed Windows efficiency, or if IBM provides
an ffs64(), then this should be re-written to take advantage

                if ( valueToTest == 0 ) return 0;

                unsigned int testWord;
                testWord = valueToTest & lswMask;
                if ( testWord != 0 )
                {
                        // _BitScanForward returns base 0
#ifdef WIN32
                        unsigned long index;
                        _BitScanForward(&index, testWord);
                        return index+1;
#else
                        return ffs(testWord);
#endif
                }
                else
                {
                        testWord = valueToTest >> 32;
#ifdef WIN32
                        unsigned long index;
                        _BitScanForward(&index, testWord);
                        return index+33;
#else
                        return ffs(testWord) + 32;
#endif

                }
        }

I have strong -- but not utterly conclusive; you know what debugging a
complex program is like -- that for the value I had in the OP --
0x0034000000000000? -- the method returns 32, implying that the final ffs()
was called with testWord = 0.

Charles

-----Original Message-----
From: IBM Mainframe Discussion List [mailto:IBM-MAIN@LISTSERV.UA.EDU] On
Behalf Of David Crayford
Sent: Sunday, July 21, 2013 8:31 PM
To: IBM-MAIN@LISTSERV.UA.EDU
Subject: Re: Looking for help with an obscure C integer problem

I'm struggling to see what is wrong with testWord = valueToTest >> 32.
There are no side effects and the sequence point is at the end of the full
expression. Can anybody enlighten me?
Charles, is the code snippet you supplied the exact test cast that is
resulting in undefined behaviour? I cannot recreate the problem and I've
tried it on five different C++ compilers, including z/OS v1.13.

On 22/07/2013 2:33 AM, Charles Mills wrote:
Right. There are two things here:

1. Resolving the immediate problem (and understanding exactly why it
fails might be a good first step).

2. The quality of the code. You are right; it is poor code. I try to
write pretty good code. I take no pride in avoiding the use of
unnecessary parentheses. I confess, I (a.) failed to consider that
testWord = valueToTest >> 32 would not reliably operate as intended;
and (b.) I was completely satisfied when the function passed basic
unit tests and though no more of it. Lesson learned, hopefully. Not
certain exactly what the lesson is, but I will be more careful in the
future. I have learned to be cautious about integer type conversions,
but obviously not cautious enough. I guess the lesson is just like for
sequences of logical operators: use parentheses to force what you expect.

Charles

-----Original Message-----
From: IBM Mainframe Discussion List [mailto:IBM-MAIN@LISTSERV.UA.EDU]
On Behalf Of Harry Wahl
Sent: Sunday, July 21, 2013 11:21 AM
To: IBM-MAIN@LISTSERV.UA.EDU
Subject: Re: Looking for help with an obscure C integer problem

Charles,
Hi, here is my opinion (and this definitely falls under the category
of "obscure C"):
You are not considering the implications of "sequence points" in your
C/C++.
"Sequence points" should not be confused with "operator precedence."
Operator precedence is determinate, sequence points are not.
I believe IBM XLC is at the C++11 level of C/C++. The C/C++ level is
relevant here because there are sometimes subtle, and sometimes not so
subtle, differences in how sequence points apply between various
levels of
C++.
While C++11 (the most recent level of C/C++) seems to a have only
tiny, mostly irrelevant and evolutionary changes from prior levels of
C/C++; there are significant differences in how "sequence points" are
defined and effect execution.
Still, C++11 and the level of the C/C++ compiler that is compiling
your program is only tangential to the situation you describe. Your
code will execute with undefined behavior regardless of what compiler
you use. But, knowing the level of the C/C++  compiler may be
important if you wish to reconcile why it behaves one way sometimes and
other ways other times (e.g.
on a different z/OS).
To me, your failure to consider the subtle impact of sequence points
renders your code ambiguous and subject to undefined behavior. This
manifests itself, for example, by executing differently when
optimized. It is at the compiler's and optimizer's discretion to
decide the order of execution for code that the C++ standard does not
specifically define. This includes overlapping execution.
I think the C/C++ compiler and optimizer are working exactly as
specified by applicable ISO/IEC standards.
"The fault, dear Brutus, it not in our stars,But, in ourselves, that
we are underlings"
Cassius in Shakespeare's Julius Caesar

----------------------------------------------------------------------
For IBM-MAIN subscribe / signoff / archive access instructions, send
email to lists...@listserv.ua.edu with the message: INFO IBM-MAIN
----------------------------------------------------------------------
For IBM-MAIN subscribe / signoff / archive access instructions, send email
to lists...@listserv.ua.edu with the message: INFO IBM-MAIN

----------------------------------------------------------------------
For IBM-MAIN subscribe / signoff / archive access instructions,
send email to lists...@listserv.ua.edu with the message: INFO IBM-MAIN

----------------------------------------------------------------------
For IBM-MAIN subscribe / signoff / archive access instructions,
send email to lists...@listserv.ua.edu with the message: INFO IBM-MAIN

Reply via email to