Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined behaviour warning that looks like a false positive? #2786

Open
CiderMan opened this issue Mar 6, 2024 · 4 comments
Open

Undefined behaviour warning that looks like a false positive? #2786

CiderMan opened this issue Mar 6, 2024 · 4 comments
Labels

Comments

@CiderMan
Copy link
Contributor

CiderMan commented Mar 6, 2024

I have a function that calculates a result across each set of 4 lanes. I then write out the value for each set of 4 lanes:

         foreach (Element = 0...16)
         {
            [...]
            if ((Element & 0x03) == 0)
            {
               pOutput[Element >> 2] = Result;
            }
         }

However, when I compile, I get the following warning:

foo.ispc:1170:13: Warning: Undefined behavior: all program instances are writing to the same location!
            pOutput[Element >> 2] = Result;
            ^^^^^^^^^^^^^^^^^^^^^

As I see it, only the lanes where Element is 0, 4, 8 or 12 are active, and they will write to indices 0, 1, 2 and 3 in the output buffer so I'm not sure that I should be getting the warning? Unless I have missed something (which is always possible, of course!)

@dbabokin
Copy link
Collaborator

dbabokin commented Mar 6, 2024

Could you provide more complete example? I can't reproduce it - that's how I'm trying it: here

I suspect that compiler is not wrong, i.e. with 4 wide target all active program instances will be writing to the same location... but where will be only one of them :) But I'm not able to reproduce.

@CiderMan
Copy link
Contributor Author

CiderMan commented Mar 6, 2024

@dbabokin I suspect that it is as you say - the target where we get the warning is 4-wide. A little tweak and I was able to reproduce the warning: https://godbolt.org/z/dbns4c33M

I agree that the statement is technically correct but is it really undefined behaviour to have a single program instance writing to a single location? I suspect that it is not. ;)

@dbabokin
Copy link
Collaborator

dbabokin commented Mar 6, 2024

Thanks for the reproducer!

I agree that the statement is technically correct but is it really undefined behaviour to have a single program instance writing to a single location? I suspect that it is not. ;)

Absolutely, it needs to be fixed, it looks like and corner case at this point.

@biergaizi
Copy link

I also discovered the same false-positive warning in my application.

Consider a 1D array of vector3 in the Array-of-Struct layout, commonly found in legacy code:

{x0, y0, z0, x1, y1, z1, x2, y2, z2}

To access a single vector of (x, y, z), one can write a ISPC kernel like the following:

#define idx(i, n)	(i * 3 + n)

void computeVectorWeight(
	uniform float array[],
	uniform uint32 pos, uniform uint32 len
)
{
	foreach (i = pos ... pos + len) {
		uint32 idxX = idx(i, 0);
		uint32 idxY = idx(i, 1);
		uint32 idxZ = idx(i, 2);

		float x = array[idxX];
		float y = array[idxY];
		float z = array[idxZ];
		float sum = x + y + z;

		array[idxX] = x / sum;
		array[idxY] = y / sum;
		array[idxZ] = z / sum;
	}
}

The compiler thinks it's okay, and only generates gather-scatter warnings.

However, as soon as the compile-time constant 3 is replaced by a runtime variable strideI:

static uint32 strideI;
#define idx(i, n)	(i * strideI + n)

Now false-positive warnings are generated:

Warning: No --target specified on command-line. Using default system target "avx2-i32x8". 
bug2.ispc:19:3: Warning: Undefined behavior: all program instances are writing to the same location! 
  array[idxX] = x / sum;
  ^^^^^^^^^^^

bug2.ispc:20:3: Warning: Undefined behavior: all program instances are writing to the same location! 
  array[idxY] = y / sum;
  ^^^^^^^^^^^

bug2.ispc:21:3: Warning: Undefined behavior: all program instances are writing to the same location! 
  array[idxZ] = z / sum;
  ^^^^^^^^^^^

Clearly, if the value of strideI is correctly set, there's no such violations and the code is perfectly legitimate, albeit relatively inefficient. On the other hand, the compiler obviously cannot prove its runtime behaviors and assumes strideI can take any value, so false-positive warning are generated.

@pbrubaker pbrubaker added the Bugs label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants